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)); + } } }