From e4da6905478a72f1ac9e8a982178de7bf0fb6f23 Mon Sep 17 00:00:00 2001 From: Tsuyoshi Ozawa Date: Wed, 25 Mar 2015 16:59:40 +0900 Subject: [PATCH] HADOOP-11014. Potential resource leak in JavaKeyStoreProvider due to unclosed stream. (ozawa) (cherry picked from commit b351086ff66ca279c0550e078e3a9d110f3f36a5) --- hadoop-common-project/hadoop-common/CHANGES.txt | 3 +++ .../hadoop/crypto/key/JavaKeyStoreProvider.java | 15 ++++++++------- .../security/alias/JavaKeyStoreProvider.java | 15 +++++++-------- 3 files changed, 18 insertions(+), 15 deletions(-) diff --git a/hadoop-common-project/hadoop-common/CHANGES.txt b/hadoop-common-project/hadoop-common/CHANGES.txt index 8724db7e187..89f8292466c 100644 --- a/hadoop-common-project/hadoop-common/CHANGES.txt +++ b/hadoop-common-project/hadoop-common/CHANGES.txt @@ -731,6 +731,9 @@ Release 2.7.0 - UNRELEASED HADOOP-11609. Correct credential commands info in CommandsManual.html#credential. (Varun Saxena via ozawa) + HADOOP-11014. Potential resource leak in JavaKeyStoreProvider due to + unclosed stream. (ozawa) + Release 2.6.1 - UNRELEASED INCOMPATIBLE CHANGES diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/crypto/key/JavaKeyStoreProvider.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/crypto/key/JavaKeyStoreProvider.java index 091cab57f46..c6d60a33b11 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/crypto/key/JavaKeyStoreProvider.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/crypto/key/JavaKeyStoreProvider.java @@ -22,6 +22,7 @@ import com.google.common.base.Preconditions; import org.apache.commons.io.IOUtils; import org.apache.hadoop.classification.InterfaceAudience; import org.apache.hadoop.conf.Configuration; +import org.apache.hadoop.fs.FSDataInputStream; import org.apache.hadoop.fs.FSDataOutputStream; import org.apache.hadoop.fs.FileStatus; import org.apache.hadoop.fs.FileSystem; @@ -303,9 +304,11 @@ public class JavaKeyStoreProvider extends KeyProvider { private FsPermission loadFromPath(Path p, char[] password) throws IOException, NoSuchAlgorithmException, CertificateException { - FileStatus s = fs.getFileStatus(p); - keyStore.load(fs.open(p), password); - return s.getPermission(); + try (FSDataInputStream in = fs.open(p)) { + FileStatus s = fs.getFileStatus(p); + keyStore.load(in, password); + return s.getPermission(); + } } private Path constructNewPath(Path path) { @@ -599,9 +602,8 @@ public class JavaKeyStoreProvider extends KeyProvider { } protected void writeToNew(Path newPath) throws IOException { - FSDataOutputStream out = - FileSystem.create(fs, newPath, permissions); - try { + try (FSDataOutputStream out = + FileSystem.create(fs, newPath, permissions);) { keyStore.store(out, password); } catch (KeyStoreException e) { throw new IOException("Can't store keystore " + this, e); @@ -612,7 +614,6 @@ public class JavaKeyStoreProvider extends KeyProvider { throw new IOException( "Certificate exception storing keystore " + this, e); } - out.close(); } protected boolean backupToOld(Path oldPath) diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/alias/JavaKeyStoreProvider.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/alias/JavaKeyStoreProvider.java index 05958a058a3..5e5cebbc629 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/alias/JavaKeyStoreProvider.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/alias/JavaKeyStoreProvider.java @@ -22,6 +22,7 @@ import org.apache.commons.io.Charsets; import org.apache.commons.io.IOUtils; import org.apache.hadoop.classification.InterfaceAudience; import org.apache.hadoop.conf.Configuration; +import org.apache.hadoop.fs.FSDataInputStream; import org.apache.hadoop.fs.FSDataOutputStream; import org.apache.hadoop.fs.FileStatus; import org.apache.hadoop.fs.FileSystem; @@ -98,11 +99,8 @@ public class JavaKeyStoreProvider extends CredentialProvider { ClassLoader cl = Thread.currentThread().getContextClassLoader(); URL pwdFile = cl.getResource(pwFile); if (pwdFile != null) { - InputStream is = pwdFile.openStream(); - try { + try (InputStream is = pwdFile.openStream()) { password = IOUtils.toString(is).trim().toCharArray(); - } finally { - is.close(); } } } @@ -110,6 +108,7 @@ public class JavaKeyStoreProvider extends CredentialProvider { if (password == null) { password = KEYSTORE_PASSWORD_DEFAULT.toCharArray(); } + try { keyStore = KeyStore.getInstance(SCHEME_NAME); if (fs.exists(path)) { @@ -118,7 +117,9 @@ public class JavaKeyStoreProvider extends CredentialProvider { FileStatus s = fs.getFileStatus(path); permissions = s.getPermission(); - keyStore.load(fs.open(path), password); + try (FSDataInputStream in = fs.open(path)) { + keyStore.load(in, password); + } } else { permissions = new FsPermission("700"); // required to create an empty keystore. *sigh* @@ -257,8 +258,7 @@ public class JavaKeyStoreProvider extends CredentialProvider { return; } // write out the keystore - FSDataOutputStream out = FileSystem.create(fs, path, permissions); - try { + try (FSDataOutputStream out = FileSystem.create(fs, path, permissions)) { keyStore.store(out, password); } catch (KeyStoreException e) { throw new IOException("Can't store keystore " + this, e); @@ -268,7 +268,6 @@ public class JavaKeyStoreProvider extends CredentialProvider { throw new IOException("Certificate exception storing keystore " + this, e); } - out.close(); changed = false; } finally {