From 13e0d6e287475e05e5105abe49c3e6bc4b2057d7 Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Wed, 19 Apr 2017 21:42:33 +0200 Subject: [PATCH] https://github.com/pmd/pmd/issues/337 Modify ResourceLoader to close underlying JarFile if using JarURLConnection to avoid open file leaks. Clarify on JavaDoc that caller must close the returned InputStream. Minor test usage cleanups. (cherry picked from commit effe71ed549dd08b895a444e3fb75b68f48b4641) --- .../sourceforge/pmd/util/ResourceLoader.java | 51 +++++++++++++++---- .../sourceforge/pmd/RuleSetFactoryTest.java | 23 +++++---- .../pmd/AbstractRuleSetFactoryTest.java | 4 +- 3 files changed, 57 insertions(+), 21 deletions(-) diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/util/ResourceLoader.java b/pmd-core/src/main/java/net/sourceforge/pmd/util/ResourceLoader.java index fc1b68fac7..0f6d86ec1f 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/util/ResourceLoader.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/util/ResourceLoader.java @@ -11,7 +11,9 @@ import java.io.FileNotFoundException; import java.io.IOException; import java.io.InputStream; import java.net.HttpURLConnection; +import java.net.JarURLConnection; import java.net.URL; +import java.net.URLConnection; /** */ @@ -36,10 +38,14 @@ public final class ResourceLoader { } /** - * Method to find a file, first by finding it as a file - * (either by the absolute or relative path), then as - * a URL, and then finally seeing if it is on the classpath. - * @param name String + * Method to find a file, first by finding it as a file (either by the + * absolute or relative path), then as a URL, and then finally seeing if it + * is on the classpath. + *

+ * Caller is responsible for closing the {@link InputStream}. + * + * @param name + * String * @return InputStream * @throws RuleSetNotFoundException */ @@ -52,10 +58,15 @@ public final class ResourceLoader { } /** - * Uses the ClassLoader passed in to attempt to load the - * resource if it's not a File or a URL - * @param name String - * @param loader ClassLoader + * Uses the ClassLoader passed in to attempt to load the resource if it's + * not a File or a URL + *

+ * Caller is responsible for closing the {@link InputStream}. + * + * @param name + * String + * @param loader + * ClassLoader * @return InputStream * @throws RuleSetNotFoundException */ @@ -83,8 +94,30 @@ public final class ResourceLoader { if (resource == null) { // Don't throw RuleSetNotFoundException, keep API compatibility return null; + } else { + final URLConnection connection = resource.openConnection(); + final InputStream inputStream = connection.getInputStream(); + if (connection instanceof JarURLConnection) { + // Wrap the InputStream to also close the underlying JarFile if from a JarURLConnection. + // See https://github.com/pmd/pmd/issues/337 + return new InputStream() { + @Override + public int read() throws IOException { + return inputStream.read(); + } + + @Override + public void close() throws IOException { + inputStream.close(); + if (connection instanceof JarURLConnection) { + ((JarURLConnection) connection).getJarFile().close(); + } + } + }; + } else { + return inputStream; + } } - return resource.openStream(); } catch (IOException e1) { // Ignored } diff --git a/pmd-core/src/test/java/net/sourceforge/pmd/RuleSetFactoryTest.java b/pmd-core/src/test/java/net/sourceforge/pmd/RuleSetFactoryTest.java index 412c56dbb0..f200c43a49 100644 --- a/pmd-core/src/test/java/net/sourceforge/pmd/RuleSetFactoryTest.java +++ b/pmd-core/src/test/java/net/sourceforge/pmd/RuleSetFactoryTest.java @@ -50,18 +50,19 @@ public class RuleSetFactoryTest { assertNotNull(rs.getRuleByName("TestRuleRef")); } - @Test - public void testExtendedReferences() throws Exception { - InputStream in = ResourceLoader.loadResourceAsStream("net/sourceforge/pmd/rulesets/reference-ruleset.xml", - this.getClass().getClassLoader()); - Assert.assertNotNull("Test ruleset not found - can't continue with test!", in); + @Test + public void testExtendedReferences() throws Exception { + InputStream in = ResourceLoader.loadResourceAsStream("net/sourceforge/pmd/rulesets/reference-ruleset.xml", + this.getClass().getClassLoader()); + Assert.assertNotNull("Test ruleset not found - can't continue with test!", in); + in.close(); - RuleSetFactory rsf = new RuleSetFactory(); - RuleSets rs = rsf.createRuleSets("net/sourceforge/pmd/rulesets/reference-ruleset.xml"); - // added by referencing a complete ruleset (TestRuleset1.xml) - assertNotNull(rs.getRuleByName("MockRule1")); - assertNotNull(rs.getRuleByName("MockRule2")); - assertNotNull(rs.getRuleByName("MockRule3")); + RuleSetFactory rsf = new RuleSetFactory(); + RuleSets rs = rsf.createRuleSets("net/sourceforge/pmd/rulesets/reference-ruleset.xml"); + // added by referencing a complete ruleset (TestRuleset1.xml) + assertNotNull(rs.getRuleByName("MockRule1")); + assertNotNull(rs.getRuleByName("MockRule2")); + assertNotNull(rs.getRuleByName("MockRule3")); assertNotNull(rs.getRuleByName("TestRuleRef")); // added by specific reference diff --git a/pmd-test/src/main/java/net/sourceforge/pmd/AbstractRuleSetFactoryTest.java b/pmd-test/src/main/java/net/sourceforge/pmd/AbstractRuleSetFactoryTest.java index 7be4d8c617..db48ebd348 100644 --- a/pmd-test/src/main/java/net/sourceforge/pmd/AbstractRuleSetFactoryTest.java +++ b/pmd-test/src/main/java/net/sourceforge/pmd/AbstractRuleSetFactoryTest.java @@ -222,7 +222,9 @@ public abstract class AbstractRuleSetFactoryTest { List ruleSetFileNames = new ArrayList<>(); try { Properties properties = new Properties(); - properties.load(ResourceLoader.loadResourceAsStream("rulesets/" + language + "/rulesets.properties")); + try (InputStream is = ResourceLoader.loadResourceAsStream("rulesets/" + language + "/rulesets.properties")) { + properties.load(is); + } String fileNames = properties.getProperty("rulesets.filenames"); StringTokenizer st = new StringTokenizer(fileNames, ","); while (st.hasMoreTokens()) {