Merge pull request #698 from jborgers/pmd7-issue697

pmd7-issue697 Extend AvoidLoadingAllFromFile with all major load‑all‑into‑memory patterns

Author: stokpop

docs/JavaCodePerformance.md

@@ -2080,6 +2080,10 @@ This especially applies to direct file streaming. Access through ClassLoader.get
 
 **Observation: All bytes of a large file are loaded into memory.** E.g. for uploading or downloading documents, for instance to determine the mime type or create a digest.    
 **Problem:** Large objects are allocated on the heap, up to e.g. 50 MB. We also observed 300 MB and even 1 GB in back-end systems. This likely triggers long gc pauses for compaction or may trigger out of memory crashes.
+APIs like `Files.readAllBytes`, `Files.readAllLines`, `Files.readString`, `InputStream.readAllBytes`, `IOUtils.toByteArray`, `ByteArrayInputStream`,
+`ByteArrayOutputStream.toByteArray`, and `ResizableByteArrayOutputStream.toByteArray` all load entire content into heap memory. 
+This risks OutOfMemoryError, long GC pauses, and slow responses when processing large files.   
+**Note:** Not a problem for small files.
 
 In the next example, there are two large byte arrays in memory: one in baos and the other is the returned byte array since a copy is made in toByteArray().
 
@@ -2102,7 +2106,8 @@ class Bad1 {
 }
 ```
 
-**Solution:** Stream-through: use streaming all the way, don't store the whole thing in memory, don't use byte arrays. A mime type is determined from the first few bytes of the file, don't read in all 50 MB - 1 GB for that. Often, functionality can be achieved in a streaming way, i.e. [a digest can be computed in a streaming way](http://www.mkyong.com/java/java-sha-hashing-example/).
+**Solution:** Use streaming APIs end-to-end, do not buffer whole files in memory.   
+A mime type is determined from the first few bytes of the file, don't read in all 50 MB - 1 GB for that. Usually, functionality can be achieved in a streaming way, i.e. [a digest can be computed in a streaming way](http://www.mkyong.com/java/java-sha-hashing-example/).
 
 Example 1 improved:
 

rulesets/java/jpinpoint-java-rules.xml

@@ -482,47 +482,60 @@ class IterationsBetter {
 
     <rule name="AvoidLoadingAllFromFile"
           language="java"
-          message="Files.readAll methods load all bytes from a file into memory: a risk of memory problems."
+          message="Files.readAll methods and similar APIs load all bytes into memory: a risk of memory problems."
           class="net.sourceforge.pmd.lang.rule.xpath.XPathRule"
-
           externalInfoUrl="https://github.com/jborgers/PMD-jPinpoint-rules/tree/pmd7/docs/JavaCodePerformance.md#isio03">
-        <description>Problem: Files.readAllBytes and Files.readAllLines load all bytes from a file into the heap memory.
-            This may result in an OutOfMemoryError crash, or long gc pauses and slow responses.
-            Solution: Stream-through: use streaming all the way, don't store the whole thing in memory, don't use byte arrays.
-            Often, functionality can be achieved in a streaming way.
-            Note: not a problem for small files.
+        <description>
+            Problem: APIs like Files.readAllBytes, Files.readAllLines, Files.readString,
+            InputStream.readAllBytes, IOUtils.toByteArray, ByteArrayInputStream,
+            ByteArrayOutputStream.toByteArray, and ResizableByteArrayOutputStream.toByteArray
+            load entire content into heap memory. This risks OutOfMemoryError, long GC pauses,
+            and slow responses when processing large files.
+            Solution: Use streaming APIs end-to-end, do not buffer whole files in memory.
+            Note: Not a problem for small files. In that case use e.g.:
+            @SuppressWarnings("PMD.AvoidLoadingAllFromFile") // only small files: max 1 kB.
+            (jpinpoint-rules)
         (jpinpoint-rules)</description>
         <priority>2</priority>
         <properties>
             <property name="tags" value="jpinpoint-rule,memory,performance,sustainability-medium" type="String" description="classification"/>
             <property name="xpath">
                 <value><![CDATA[
-//MethodDeclaration//MethodCall[pmd-java:matchesSig('java.nio.file.Files#readAllBytes(java.nio.file.Path)')
-    or pmd-java:matchesSig('java.nio.file.Files#readAllLines(java.nio.file.Path)')
-    or pmd-java:matchesSig('java.nio.file.Files#readAllLines(java.nio.file.Path,_)')
+//MethodCall[
+    (TypeExpression[pmd-java:typeIs('java.nio.file.Files')] and @MethodName=('readAllBytes', 'readAllLines', 'readString'))
+    or pmd-java:matchesSig('java.io.InputStream#readAllBytes()')
+    or (TypeExpression[pmd-java:typeIs('org.apache.commons.io.IOUtils')] and @MethodName='toByteArray')
+    or (VariableAccess[pmd-java:typeIs('java.io.ByteArrayOutputStream')] and @MethodName='toByteArray')
+    or (VariableAccess[pmd-java:typeIs('org.springframework.util.ResizableByteArrayOutputStream')] and @MethodName='toByteArray')
 ]
-                ]]></value>
+|
+//ConstructorCall[pmd-java:typeIs('java.io.ByteArrayInputStream')]
+    ]]></value>
             </property>
         </properties>
-        <example>
-            <![CDATA[
+        <example><![CDATA[
 class Bad {
-    void bad(Path path) {
-        byte[] fileBytes = Files.readAllBytes(path); // bad
-        List<String> fileLines = Files.readAllLines(path); // bad
-        // process bytes / lines
+    void bad(Path path, InputStream in) throws Exception {
+        byte[] fileBytes = Files.readAllBytes(path);                        // bad
+        List<String> fileLines = Files.readAllLines(path);                  // bad
+        String s = Files.readString(path);                                  // bad
+        byte[] full = IOUtils.toByteArray(in);                              // bad
+        ByteArrayInputStream bis = new ByteArrayInputStream(full);          // bad
+        ByteArrayOutputStream baos = new ByteArrayOutputStream();
+        byte[] arr = baos.toByteArray();                                    // bad
     }
 }
-
 class Good {
-    void good(Path in) throws IOException {
-        try (BufferedReader reader = Files.newBufferedReader(in)) {
-            String line = reader.readLine();
-            // process line by line
+    void good(Path path, InputStream in) throws Exception {
+        try (InputStream s = Files.newInputStream(path)) {
+            IOUtils.copy(s, System.out);                                    // streaming OK
+        }
+        try (BufferedReader reader = Files.newBufferedReader(path)) {
+            String line = reader.readLine();                                // streaming OK
         }
     }
 }
-            ]]>
+]]>
         </example>
     </rule>
 
@@ -6124,10 +6137,9 @@ return HttpClientBuilder.create() // good, setConnectionManager called, pool con
           language="java"
           message="Apache HttpClient builder is used and not all three timeouts are configured."
           externalInfoUrl="https://github.com/jborgers/PMD-jPinpoint-rules/tree/pmd7/docs/JavaCodePerformance.md#IBI10">
-        <description>Problem: for connectionRequestTimeout, connectTimeout, socketTimeout (using
-            HttpComponentsClientHttpRequestFactory/RequestConfig) or readTimeout/responseTimeout (using RequestConfig v4/v5) and connectTimeout (using ConnectionConfig v5)
-            the default timeout settings are not optimal in most cases. &#13;
-            Solution: Set the timeouts explicitly to proper reasoned values. See best practice values via the link. Use
+        <description>Problem: default timeout values for Apache HttpClient are often sub optimal (connectionRequestTimeout, connectTimeout, socketTimeout in HttpComponentsClientHttpRequestFactory;
+            readTimeout/responseTimeout in RequestConfig v4/v5; and connectTimeout in ConnectionConfig v5) &#13;
+            Solution: Set all these timeouts explicitly to proper reasoned values. See best practice values via the link. Use
             the setDefaultRequestConfig with a method with a RequestConfig object on HttpClient builders to set the
             timeouts. (jpinpoint-rules)
         (jpinpoint-rules)</description>

rulesets/java/jpinpoint-rules.xml

@@ -482,47 +482,60 @@ class IterationsBetter {
 
     <rule name="AvoidLoadingAllFromFile"
           language="java"
-          message="Files.readAll methods load all bytes from a file into memory: a risk of memory problems."
+          message="Files.readAll methods and similar APIs load all bytes into memory: a risk of memory problems."
           class="net.sourceforge.pmd.lang.rule.xpath.XPathRule"
-
           externalInfoUrl="https://github.com/jborgers/PMD-jPinpoint-rules/tree/pmd7/docs/JavaCodePerformance.md#isio03">
-        <description>Problem: Files.readAllBytes and Files.readAllLines load all bytes from a file into the heap memory.
-            This may result in an OutOfMemoryError crash, or long gc pauses and slow responses.
-            Solution: Stream-through: use streaming all the way, don't store the whole thing in memory, don't use byte arrays.
-            Often, functionality can be achieved in a streaming way.
-            Note: not a problem for small files.
+        <description>
+            Problem: APIs like Files.readAllBytes, Files.readAllLines, Files.readString,
+            InputStream.readAllBytes, IOUtils.toByteArray, ByteArrayInputStream,
+            ByteArrayOutputStream.toByteArray, and ResizableByteArrayOutputStream.toByteArray
+            load entire content into heap memory. This risks OutOfMemoryError, long GC pauses,
+            and slow responses when processing large files.
+            Solution: Use streaming APIs end-to-end, do not buffer whole files in memory.
+            Note: Not a problem for small files. In that case use e.g.:
+            @SuppressWarnings("PMD.AvoidLoadingAllFromFile") // only small files: max 1 kB.
+            (jpinpoint-rules)
         (jpinpoint-rules)</description>
         <priority>2</priority>
         <properties>
             <property name="tags" value="jpinpoint-rule,memory,performance,sustainability-medium" type="String" description="classification"/>
             <property name="xpath">
                 <value><![CDATA[
-//MethodDeclaration//MethodCall[pmd-java:matchesSig('java.nio.file.Files#readAllBytes(java.nio.file.Path)')
-    or pmd-java:matchesSig('java.nio.file.Files#readAllLines(java.nio.file.Path)')
-    or pmd-java:matchesSig('java.nio.file.Files#readAllLines(java.nio.file.Path,_)')
+//MethodCall[
+    (TypeExpression[pmd-java:typeIs('java.nio.file.Files')] and @MethodName=('readAllBytes', 'readAllLines', 'readString'))
+    or pmd-java:matchesSig('java.io.InputStream#readAllBytes()')
+    or (TypeExpression[pmd-java:typeIs('org.apache.commons.io.IOUtils')] and @MethodName='toByteArray')
+    or (VariableAccess[pmd-java:typeIs('java.io.ByteArrayOutputStream')] and @MethodName='toByteArray')
+    or (VariableAccess[pmd-java:typeIs('org.springframework.util.ResizableByteArrayOutputStream')] and @MethodName='toByteArray')
 ]
-                ]]></value>
+|
+//ConstructorCall[pmd-java:typeIs('java.io.ByteArrayInputStream')]
+    ]]></value>
             </property>
         </properties>
-        <example>
-            <![CDATA[
+        <example><![CDATA[
 class Bad {
-    void bad(Path path) {
-        byte[] fileBytes = Files.readAllBytes(path); // bad
-        List<String> fileLines = Files.readAllLines(path); // bad
-        // process bytes / lines
+    void bad(Path path, InputStream in) throws Exception {
+        byte[] fileBytes = Files.readAllBytes(path);                        // bad
+        List<String> fileLines = Files.readAllLines(path);                  // bad
+        String s = Files.readString(path);                                  // bad
+        byte[] full = IOUtils.toByteArray(in);                              // bad
+        ByteArrayInputStream bis = new ByteArrayInputStream(full);          // bad
+        ByteArrayOutputStream baos = new ByteArrayOutputStream();
+        byte[] arr = baos.toByteArray();                                    // bad
     }
 }
-
 class Good {
-    void good(Path in) throws IOException {
-        try (BufferedReader reader = Files.newBufferedReader(in)) {
-            String line = reader.readLine();
-            // process line by line
+    void good(Path path, InputStream in) throws Exception {
+        try (InputStream s = Files.newInputStream(path)) {
+            IOUtils.copy(s, System.out);                                    // streaming OK
+        }
+        try (BufferedReader reader = Files.newBufferedReader(path)) {
+            String line = reader.readLine();                                // streaming OK
         }
     }
 }
-            ]]>
+]]>
         </example>
     </rule>
 
@@ -6124,10 +6137,9 @@ return HttpClientBuilder.create() // good, setConnectionManager called, pool con
           language="java"
           message="Apache HttpClient builder is used and not all three timeouts are configured."
           externalInfoUrl="https://github.com/jborgers/PMD-jPinpoint-rules/tree/pmd7/docs/JavaCodePerformance.md#IBI10">
-        <description>Problem: for connectionRequestTimeout, connectTimeout, socketTimeout (using
-            HttpComponentsClientHttpRequestFactory/RequestConfig) or readTimeout/responseTimeout (using RequestConfig v4/v5) and connectTimeout (using ConnectionConfig v5)
-            the default timeout settings are not optimal in most cases. &#13;
-            Solution: Set the timeouts explicitly to proper reasoned values. See best practice values via the link. Use
+        <description>Problem: default timeout values for Apache HttpClient are often sub optimal (connectionRequestTimeout, connectTimeout, socketTimeout in HttpComponentsClientHttpRequestFactory;
+            readTimeout/responseTimeout in RequestConfig v4/v5; and connectTimeout in ConnectionConfig v5) &#13;
+            Solution: Set all these timeouts explicitly to proper reasoned values. See best practice values via the link. Use
             the setDefaultRequestConfig with a method with a RequestConfig object on HttpClient builders to set the
             timeouts. (jpinpoint-rules)
         (jpinpoint-rules)</description>

src/main/resources/category/java/common.xml

@@ -2141,47 +2141,60 @@ class Foo {
 
     <rule name="AvoidLoadingAllFromFile"
           language="java"
-          message="Files.readAll methods load all bytes from a file into memory: a risk of memory problems."
+          message="Files.readAll methods and similar APIs load all bytes into memory: a risk of memory problems."
           class="net.sourceforge.pmd.lang.rule.xpath.XPathRule"
-
           externalInfoUrl="${doc_root}/JavaCodePerformance.md#isio03">
-        <description>Problem: Files.readAllBytes and Files.readAllLines load all bytes from a file into the heap memory.
-            This may result in an OutOfMemoryError crash, or long gc pauses and slow responses.
-            Solution: Stream-through: use streaming all the way, don't store the whole thing in memory, don't use byte arrays.
-            Often, functionality can be achieved in a streaming way.
-            Note: not a problem for small files.
+        <description>
+            Problem: APIs like Files.readAllBytes, Files.readAllLines, Files.readString,
+            InputStream.readAllBytes, IOUtils.toByteArray, ByteArrayInputStream,
+            ByteArrayOutputStream.toByteArray, and ResizableByteArrayOutputStream.toByteArray
+            load entire content into heap memory. This risks OutOfMemoryError, long GC pauses,
+            and slow responses when processing large files.
+            Solution: Use streaming APIs end-to-end, do not buffer whole files in memory.
+            Note: Not a problem for small files. In that case use e.g.:
+            @SuppressWarnings("PMD.AvoidLoadingAllFromFile") // only small files: max 1 kB.
+            (jpinpoint-rules)
         </description>
         <priority>2</priority>
         <properties>
             <property name="tags" value="jpinpoint-rule,memory,performance,sustainability-medium" type="String" description="classification"/>
             <property name="xpath">
                 <value><![CDATA[
-//MethodDeclaration//MethodCall[pmd-java:matchesSig('java.nio.file.Files#readAllBytes(java.nio.file.Path)')
-    or pmd-java:matchesSig('java.nio.file.Files#readAllLines(java.nio.file.Path)')
-    or pmd-java:matchesSig('java.nio.file.Files#readAllLines(java.nio.file.Path,_)')
+//MethodCall[
+    (TypeExpression[pmd-java:typeIs('java.nio.file.Files')] and @MethodName=('readAllBytes', 'readAllLines', 'readString'))
+    or pmd-java:matchesSig('java.io.InputStream#readAllBytes()')
+    or (TypeExpression[pmd-java:typeIs('org.apache.commons.io.IOUtils')] and @MethodName='toByteArray')
+    or (VariableAccess[pmd-java:typeIs('java.io.ByteArrayOutputStream')] and @MethodName='toByteArray')
+    or (VariableAccess[pmd-java:typeIs('org.springframework.util.ResizableByteArrayOutputStream')] and @MethodName='toByteArray')
 ]
-                ]]></value>
+|
+//ConstructorCall[pmd-java:typeIs('java.io.ByteArrayInputStream')]
+    ]]></value>
             </property>
         </properties>
-        <example>
-            <![CDATA[
+        <example><![CDATA[
 class Bad {
-    void bad(Path path) {
-        byte[] fileBytes = Files.readAllBytes(path); // bad
-        List<String> fileLines = Files.readAllLines(path); // bad
-        // process bytes / lines
+    void bad(Path path, InputStream in) throws Exception {
+        byte[] fileBytes = Files.readAllBytes(path);                        // bad
+        List<String> fileLines = Files.readAllLines(path);                  // bad
+        String s = Files.readString(path);                                  // bad
+        byte[] full = IOUtils.toByteArray(in);                              // bad
+        ByteArrayInputStream bis = new ByteArrayInputStream(full);          // bad
+        ByteArrayOutputStream baos = new ByteArrayOutputStream();
+        byte[] arr = baos.toByteArray();                                    // bad
     }
 }
-
 class Good {
-    void good(Path in) throws IOException {
-        try (BufferedReader reader = Files.newBufferedReader(in)) {
-            String line = reader.readLine();
-            // process line by line
+    void good(Path path, InputStream in) throws Exception {
+        try (InputStream s = Files.newInputStream(path)) {
+            IOUtils.copy(s, System.out);                                    // streaming OK
+        }
+        try (BufferedReader reader = Files.newBufferedReader(path)) {
+            String line = reader.readLine();                                // streaming OK
         }
     }
 }
-            ]]>
+]]>
         </example>
     </rule>