HADOOP-11014. Potential resource leak in JavaKeyStoreProvider due to unclosed stream. (ozawa)

This commit is contained in:
Tsuyoshi Ozawa 2015-03-25 16:59:40 +09:00
parent 5582b0f1d4
commit b351086ff6
3 changed files with 18 additions and 15 deletions

View File

@ -1142,6 +1142,9 @@ Release 2.7.0 - UNRELEASED
HADOOP-11609. Correct credential commands info in HADOOP-11609. Correct credential commands info in
CommandsManual.html#credential. (Varun Saxena via ozawa) CommandsManual.html#credential. (Varun Saxena via ozawa)
HADOOP-11014. Potential resource leak in JavaKeyStoreProvider due to
unclosed stream. (ozawa)
Release 2.6.1 - UNRELEASED Release 2.6.1 - UNRELEASED
INCOMPATIBLE CHANGES INCOMPATIBLE CHANGES

View File

@ -22,6 +22,7 @@
import org.apache.commons.io.IOUtils; import org.apache.commons.io.IOUtils;
import org.apache.hadoop.classification.InterfaceAudience; import org.apache.hadoop.classification.InterfaceAudience;
import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.fs.FSDataInputStream;
import org.apache.hadoop.fs.FSDataOutputStream; import org.apache.hadoop.fs.FSDataOutputStream;
import org.apache.hadoop.fs.FileStatus; import org.apache.hadoop.fs.FileStatus;
import org.apache.hadoop.fs.FileSystem; import org.apache.hadoop.fs.FileSystem;
@ -303,9 +304,11 @@ private boolean isBadorWrongPassword(IOException ioe) {
private FsPermission loadFromPath(Path p, char[] password) private FsPermission loadFromPath(Path p, char[] password)
throws IOException, NoSuchAlgorithmException, CertificateException { throws IOException, NoSuchAlgorithmException, CertificateException {
FileStatus s = fs.getFileStatus(p); try (FSDataInputStream in = fs.open(p)) {
keyStore.load(fs.open(p), password); FileStatus s = fs.getFileStatus(p);
return s.getPermission(); keyStore.load(in, password);
return s.getPermission();
}
} }
private Path constructNewPath(Path path) { private Path constructNewPath(Path path) {
@ -599,9 +602,8 @@ private void cleanupNewAndOld(Path newPath, Path oldPath) throws IOException {
} }
protected void writeToNew(Path newPath) throws IOException { protected void writeToNew(Path newPath) throws IOException {
FSDataOutputStream out = try (FSDataOutputStream out =
FileSystem.create(fs, newPath, permissions); FileSystem.create(fs, newPath, permissions);) {
try {
keyStore.store(out, password); keyStore.store(out, password);
} catch (KeyStoreException e) { } catch (KeyStoreException e) {
throw new IOException("Can't store keystore " + this, e); throw new IOException("Can't store keystore " + this, e);
@ -612,7 +614,6 @@ protected void writeToNew(Path newPath) throws IOException {
throw new IOException( throw new IOException(
"Certificate exception storing keystore " + this, e); "Certificate exception storing keystore " + this, e);
} }
out.close();
} }
protected boolean backupToOld(Path oldPath) protected boolean backupToOld(Path oldPath)

View File

@ -22,6 +22,7 @@
import org.apache.commons.io.IOUtils; import org.apache.commons.io.IOUtils;
import org.apache.hadoop.classification.InterfaceAudience; import org.apache.hadoop.classification.InterfaceAudience;
import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.fs.FSDataInputStream;
import org.apache.hadoop.fs.FSDataOutputStream; import org.apache.hadoop.fs.FSDataOutputStream;
import org.apache.hadoop.fs.FileStatus; import org.apache.hadoop.fs.FileStatus;
import org.apache.hadoop.fs.FileSystem; import org.apache.hadoop.fs.FileSystem;
@ -98,11 +99,8 @@ private JavaKeyStoreProvider(URI uri, Configuration conf) throws IOException {
ClassLoader cl = Thread.currentThread().getContextClassLoader(); ClassLoader cl = Thread.currentThread().getContextClassLoader();
URL pwdFile = cl.getResource(pwFile); URL pwdFile = cl.getResource(pwFile);
if (pwdFile != null) { if (pwdFile != null) {
InputStream is = pwdFile.openStream(); try (InputStream is = pwdFile.openStream()) {
try {
password = IOUtils.toString(is).trim().toCharArray(); password = IOUtils.toString(is).trim().toCharArray();
} finally {
is.close();
} }
} }
} }
@ -110,6 +108,7 @@ private JavaKeyStoreProvider(URI uri, Configuration conf) throws IOException {
if (password == null) { if (password == null) {
password = KEYSTORE_PASSWORD_DEFAULT.toCharArray(); password = KEYSTORE_PASSWORD_DEFAULT.toCharArray();
} }
try { try {
keyStore = KeyStore.getInstance(SCHEME_NAME); keyStore = KeyStore.getInstance(SCHEME_NAME);
if (fs.exists(path)) { if (fs.exists(path)) {
@ -118,7 +117,9 @@ private JavaKeyStoreProvider(URI uri, Configuration conf) throws IOException {
FileStatus s = fs.getFileStatus(path); FileStatus s = fs.getFileStatus(path);
permissions = s.getPermission(); permissions = s.getPermission();
keyStore.load(fs.open(path), password); try (FSDataInputStream in = fs.open(path)) {
keyStore.load(in, password);
}
} else { } else {
permissions = new FsPermission("700"); permissions = new FsPermission("700");
// required to create an empty keystore. *sigh* // required to create an empty keystore. *sigh*
@ -257,8 +258,7 @@ public void flush() throws IOException {
return; return;
} }
// write out the keystore // write out the keystore
FSDataOutputStream out = FileSystem.create(fs, path, permissions); try (FSDataOutputStream out = FileSystem.create(fs, path, permissions)) {
try {
keyStore.store(out, password); keyStore.store(out, password);
} catch (KeyStoreException e) { } catch (KeyStoreException e) {
throw new IOException("Can't store keystore " + this, e); throw new IOException("Can't store keystore " + this, e);
@ -268,7 +268,6 @@ public void flush() throws IOException {
throw new IOException("Certificate exception storing keystore " + this, throw new IOException("Certificate exception storing keystore " + this,
e); e);
} }
out.close();
changed = false; changed = false;
} }
finally { finally {