This is an automated email from the ASF dual-hosted git repository.

janhoy pushed a commit to branch branch_9x
in repository https://gitbox.apache.org/repos/asf/solr.git


The following commit(s) were added to refs/heads/branch_9x by this push:
     new c79011e81da SOLR-16949: Handle inputStream leaks
c79011e81da is described below

commit c79011e81dada2f9bc4b4df32ffb32152ef81152
Author: Jan Høydahl <jan...@apache.org>
AuthorDate: Sun Dec 17 23:19:14 2023 +0100

    SOLR-16949: Handle inputStream leaks
    
    (cherry picked from commit c89813a675663bb82f18236840b2a84cac05e1ee)
---
 .../solr/core/FileSystemConfigSetService.java      |  5 +-
 .../org/apache/solr/util/FileTypeMagicUtil.java    | 93 +++++++++++++++-------
 .../apache/solr/util/FileTypeMagicUtilTest.java    | 58 +++++++-------
 3 files changed, 96 insertions(+), 60 deletions(-)

diff --git 
a/solr/core/src/java/org/apache/solr/core/FileSystemConfigSetService.java 
b/solr/core/src/java/org/apache/solr/core/FileSystemConfigSetService.java
index 4b041252211..c4686195d64 100644
--- a/solr/core/src/java/org/apache/solr/core/FileSystemConfigSetService.java
+++ b/solr/core/src/java/org/apache/solr/core/FileSystemConfigSetService.java
@@ -214,12 +214,11 @@ public class FileSystemConfigSetService extends 
ConfigSetService {
                     "Not including uploading file to config, as it is a 
forbidden type: {}",
                     file.getFileName());
               } else {
-                if 
(!FileTypeMagicUtil.isFileForbiddenInConfigset(Files.newInputStream(file))) {
+                if (!FileTypeMagicUtil.isFileForbiddenInConfigset(file)) {
                   Files.copy(
                       file, 
target.resolve(source.relativize(file).toString()), REPLACE_EXISTING);
                 } else {
-                  String mimeType =
-                      
FileTypeMagicUtil.INSTANCE.guessMimeType(Files.newInputStream(file));
+                  String mimeType = 
FileTypeMagicUtil.INSTANCE.guessMimeType(file);
                   log.warn(
                       "Not copying file {}, as it matched the MAGIC signature 
of a forbidden mime type {}",
                       file.getFileName(),
diff --git a/solr/core/src/java/org/apache/solr/util/FileTypeMagicUtil.java 
b/solr/core/src/java/org/apache/solr/util/FileTypeMagicUtil.java
index cfb6c9fa0af..692cda83bd3 100644
--- a/solr/core/src/java/org/apache/solr/util/FileTypeMagicUtil.java
+++ b/solr/core/src/java/org/apache/solr/util/FileTypeMagicUtil.java
@@ -20,7 +20,6 @@ package org.apache.solr.util;
 import com.j256.simplemagic.ContentInfo;
 import com.j256.simplemagic.ContentInfoUtil;
 import com.j256.simplemagic.ContentType;
-import java.io.ByteArrayInputStream;
 import java.io.IOException;
 import java.io.InputStream;
 import java.nio.file.FileVisitResult;
@@ -58,30 +57,23 @@ public class FileTypeMagicUtil implements 
ContentInfoUtil.ErrorCallBack {
   public static void assertConfigSetFolderLegal(Path confPath) throws 
IOException {
     Files.walkFileTree(
         confPath,
-        new SimpleFileVisitor<Path>() {
+        new SimpleFileVisitor<>() {
           @Override
-          public FileVisitResult visitFile(Path file, BasicFileAttributes 
attrs)
-              throws IOException {
-            // Read first 100 bytes of the file to determine the mime type
-            try (InputStream fileStream = Files.newInputStream(file)) {
-              byte[] bytes = new byte[100];
-              fileStream.read(bytes);
-              if (FileTypeMagicUtil.isFileForbiddenInConfigset(bytes)) {
-                throw new SolrException(
-                    SolrException.ErrorCode.BAD_REQUEST,
-                    String.format(
-                        Locale.ROOT,
-                        "Not uploading file %s to configset, as it matched the 
MAGIC signature of a forbidden mime type %s",
-                        file,
-                        FileTypeMagicUtil.INSTANCE.guessMimeType(bytes)));
-              }
-              return FileVisitResult.CONTINUE;
+          public FileVisitResult visitFile(Path file, BasicFileAttributes 
attrs) {
+            if (FileTypeMagicUtil.isFileForbiddenInConfigset(file)) {
+              throw new SolrException(
+                  SolrException.ErrorCode.BAD_REQUEST,
+                  String.format(
+                      Locale.ROOT,
+                      "Not uploading file %s to configset, as it matched the 
MAGIC signature of a forbidden mime type %s",
+                      file,
+                      FileTypeMagicUtil.INSTANCE.guessMimeType(file)));
             }
+            return FileVisitResult.CONTINUE;
           }
 
           @Override
-          public FileVisitResult preVisitDirectory(Path dir, 
BasicFileAttributes attrs)
-              throws IOException {
+          public FileVisitResult preVisitDirectory(Path dir, 
BasicFileAttributes attrs) {
             if (SKIP_FOLDERS.contains(dir.getFileName().toString()))
               return FileVisitResult.SKIP_SUBTREE;
 
@@ -90,19 +82,29 @@ public class FileTypeMagicUtil implements 
ContentInfoUtil.ErrorCallBack {
         });
   }
 
+  /**
+   * Guess the mime type of file based on its magic number.
+   *
+   * @param file file to check
+   * @return string with content-type or "application/octet-stream" if unknown
+   */
+  public String guessMimeType(Path file) {
+    try {
+      return guessTypeFallbackToOctetStream(util.findMatch(file.toFile()));
+    } catch (IOException e) {
+      throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, e);
+    }
+  }
+
   /**
    * Guess the mime type of file based on its magic number.
    *
    * @param stream input stream of the file
    * @return string with content-type or "application/octet-stream" if unknown
    */
-  public String guessMimeType(InputStream stream) {
+  String guessMimeType(InputStream stream) {
     try {
-      ContentInfo info = util.findMatch(stream);
-      if (info == null) {
-        return ContentType.OTHER.getMimeType();
-      }
-      return info.getContentType().getMimeType();
+      return guessTypeFallbackToOctetStream(util.findMatch(stream));
     } catch (IOException e) {
       throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, e);
     }
@@ -115,7 +117,7 @@ public class FileTypeMagicUtil implements 
ContentInfoUtil.ErrorCallBack {
    * @return string with content-type or "application/octet-stream" if unknown
    */
   public String guessMimeType(byte[] bytes) {
-    return guessMimeType(new ByteArrayInputStream(bytes));
+    return guessTypeFallbackToOctetStream(util.findMatch(bytes));
   }
 
   @Override
@@ -126,6 +128,31 @@ public class FileTypeMagicUtil implements 
ContentInfoUtil.ErrorCallBack {
         e);
   }
 
+  /**
+   * Determine forbidden file type based on magic bytes matching of the file 
itself. Forbidden types
+   * are:
+   *
+   * <ul>
+   *   <li><code>application/x-java-applet</code>: java class file
+   *   <li><code>application/zip</code>: jar or zip archives
+   *   <li><code>application/x-tar</code>: tar archives
+   *   <li><code>text/x-shellscript</code>: shell or bash script
+   * </ul>
+   *
+   * @param file file to check
+   * @return true if file is among the forbidden mime-types
+   */
+  public static boolean isFileForbiddenInConfigset(Path file) {
+    try (InputStream fileStream = Files.newInputStream(file)) {
+      return isFileForbiddenInConfigset(fileStream);
+    } catch (IOException e) {
+      throw new SolrException(
+          SolrException.ErrorCode.SERVER_ERROR,
+          String.format(Locale.ROOT, "Error reading file %s", file),
+          e);
+    }
+  }
+
   /**
    * Determine forbidden file type based on magic bytes matching of the file 
itself. Forbidden types
    * are:
@@ -140,7 +167,7 @@ public class FileTypeMagicUtil implements 
ContentInfoUtil.ErrorCallBack {
    * @param fileStream stream from the file content
    * @return true if file is among the forbidden mime-types
    */
-  public static boolean isFileForbiddenInConfigset(InputStream fileStream) {
+  static boolean isFileForbiddenInConfigset(InputStream fileStream) {
     return 
forbiddenTypes.contains(FileTypeMagicUtil.INSTANCE.guessMimeType(fileStream));
   }
 
@@ -153,7 +180,7 @@ public class FileTypeMagicUtil implements 
ContentInfoUtil.ErrorCallBack {
   public static boolean isFileForbiddenInConfigset(byte[] bytes) {
     if (bytes == null || bytes.length == 0)
       return false; // A ZK znode may be a folder with no content
-    return isFileForbiddenInConfigset(new ByteArrayInputStream(bytes));
+    return 
forbiddenTypes.contains(FileTypeMagicUtil.INSTANCE.guessMimeType(bytes));
   }
 
   private static final Set<String> forbiddenTypes =
@@ -163,4 +190,12 @@ public class FileTypeMagicUtil implements 
ContentInfoUtil.ErrorCallBack {
                       "solr.configset.upload.mimetypes.forbidden",
                       
"application/x-java-applet,application/zip,application/x-tar,text/x-shellscript")
                   .split(",")));
+
+  private String guessTypeFallbackToOctetStream(ContentInfo contentInfo) {
+    if (contentInfo == null) {
+      return ContentType.OTHER.getMimeType();
+    } else {
+      return contentInfo.getContentType().getMimeType();
+    }
+  }
 }
diff --git a/solr/core/src/test/org/apache/solr/util/FileTypeMagicUtilTest.java 
b/solr/core/src/test/org/apache/solr/util/FileTypeMagicUtilTest.java
index b8e9a35a3d9..5c02528a7f4 100644
--- a/solr/core/src/test/org/apache/solr/util/FileTypeMagicUtilTest.java
+++ b/solr/core/src/test/org/apache/solr/util/FileTypeMagicUtilTest.java
@@ -17,38 +17,40 @@
 
 package org.apache.solr.util;
 
+import java.io.IOException;
+import java.io.InputStream;
 import org.apache.solr.SolrTestCaseJ4;
 
 public class FileTypeMagicUtilTest extends SolrTestCaseJ4 {
-  public void testGuessMimeType() {
-    assertEquals(
-        "application/x-java-applet",
-        FileTypeMagicUtil.INSTANCE.guessMimeType(
-            
FileTypeMagicUtil.class.getResourceAsStream("/magic/HelloWorldJavaClass.class.bin")));
-    assertEquals(
-        "application/zip",
-        FileTypeMagicUtil.INSTANCE.guessMimeType(
-            FileTypeMagicUtil.class.getResourceAsStream(
-                "/runtimecode/containerplugin.v.1.jar.bin")));
-    assertEquals(
-        "application/x-tar",
-        FileTypeMagicUtil.INSTANCE.guessMimeType(
-            
FileTypeMagicUtil.class.getResourceAsStream("/magic/hello.tar.bin")));
-    assertEquals(
-        "text/x-shellscript",
-        FileTypeMagicUtil.INSTANCE.guessMimeType(
-            
FileTypeMagicUtil.class.getResourceAsStream("/magic/shell.sh.txt")));
+  public void testGuessMimeType() throws IOException {
+    assertResourceMimeType("application/x-java-applet", 
"/magic/HelloWorldJavaClass.class.bin");
+    assertResourceMimeType("application/zip", 
"/runtimecode/containerplugin.v.1.jar.bin");
+    assertResourceMimeType("application/x-tar", "/magic/hello.tar.bin");
+    assertResourceMimeType("text/x-shellscript", "/magic/shell.sh.txt");
   }
 
-  public void testIsFileForbiddenInConfigset() {
-    assertTrue(
-        FileTypeMagicUtil.isFileForbiddenInConfigset(
-            
FileTypeMagicUtil.class.getResourceAsStream("/magic/HelloWorldJavaClass.class.bin")));
-    assertTrue(
-        FileTypeMagicUtil.isFileForbiddenInConfigset(
-            
FileTypeMagicUtil.class.getResourceAsStream("/magic/shell.sh.txt")));
-    assertFalse(
-        FileTypeMagicUtil.isFileForbiddenInConfigset(
-            FileTypeMagicUtil.class.getResourceAsStream("/magic/plain.txt")));
+  public void testIsFileForbiddenInConfigset() throws IOException {
+    assertResourceForbiddenInConfigset("/magic/HelloWorldJavaClass.class.bin");
+    assertResourceForbiddenInConfigset("/magic/shell.sh.txt");
+    assertResourceAllowedInConfigset("/magic/plain.txt");
+  }
+
+  private void assertResourceMimeType(String mimeType, String resourcePath) 
throws IOException {
+    try (InputStream stream =
+        
FileTypeMagicUtil.class.getResourceAsStream("/magic/HelloWorldJavaClass.class.bin"))
 {
+      assertEquals("application/x-java-applet", 
FileTypeMagicUtil.INSTANCE.guessMimeType(stream));
+    }
+  }
+
+  private void assertResourceForbiddenInConfigset(String resourcePath) throws 
IOException {
+    try (InputStream stream = 
FileTypeMagicUtil.class.getResourceAsStream(resourcePath)) {
+      assertTrue(FileTypeMagicUtil.isFileForbiddenInConfigset(stream));
+    }
+  }
+
+  private void assertResourceAllowedInConfigset(String resourcePath) throws 
IOException {
+    try (InputStream stream = 
FileTypeMagicUtil.class.getResourceAsStream(resourcePath)) {
+      assertFalse(FileTypeMagicUtil.isFileForbiddenInConfigset(stream));
+    }
   }
 }

Reply via email to