You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@solr.apache.org by "janhoy (via GitHub)" <gi...@apache.org> on 2026/04/25 21:06:29 UTC
[PR] SOLR-18213 Replace simplemagic with built-in magic-byte file type detection [solr]
janhoy opened a new pull request, #4336:
URL: https://github.com/apache/solr/pull/4336
https://issues.apache.org/jira/browse/SOLR-18213
Removes the abandoned `com.j256.simplemagic:simplemagic` dependency (last release 2021) and replaces it with a self-contained byte-pattern detector inside `FileTypeMagicUtil`. The public API is unchanged. This removes a dependency and simplifies code.
The detector covers exactly the four types that were previously configured in the custom `magic/executables` resource file, which is also removed. This PR also tries to improve on some of the detection logic.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-18213 Replace simplemagic with built-in magic-byte file type detection [solr]
Posted by "janhoy (via GitHub)" <gi...@apache.org>.
janhoy commented on code in PR #4336:
URL: https://github.com/apache/solr/pull/4336#discussion_r3215364677
##########
solr/core/src/test/org/apache/solr/util/FileTypeMagicUtilTest.java:
##########
Review Comment:
Added a test for real files, see below.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-18213 Replace simplemagic with built-in magic-byte file type detection [solr]
Posted by "HoustonPutman (via GitHub)" <gi...@apache.org>.
HoustonPutman commented on code in PR #4336:
URL: https://github.com/apache/solr/pull/4336#discussion_r3157939223
##########
solr/core/src/test/org/apache/solr/util/FileTypeMagicUtilTest.java:
##########
Review Comment:
It would be great to get some real files checked into the repo in the test resources directory and test against those...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-18213 Replace simplemagic with built-in magic-byte file type detection [solr]
Posted by "janhoy (via GitHub)" <gi...@apache.org>.
janhoy commented on code in PR #4336:
URL: https://github.com/apache/solr/pull/4336#discussion_r3158058747
##########
solr/core/src/test/org/apache/solr/util/FileTypeMagicUtilTest.java:
##########
Review Comment:
Problem with real binary files in git, is that it is an anti-pattern. We should not have a single binary file in the repo which cannot be reproduced from source or from a script of some kind. Else we risk supply chain attachs with a contributor adding some nice-looking binary in a PR and we merge it without checking what it hides. Perhaps there exists some catalog of well-known binary files with known SHA-512 hashes that we could use for this purpose? Or am I being too cautious?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-18213 Replace simplemagic with built-in magic-byte file type detection [solr]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #4336:
URL: https://github.com/apache/solr/pull/4336#discussion_r3142616733
##########
solr/core/src/java/org/apache/solr/util/FileTypeMagicUtil.java:
##########
@@ -196,14 +201,110 @@ public static boolean isFileForbiddenInConfigset(byte[] bytes) {
Arrays.asList(
System.getProperty(
"solr.configset.upload.mimetypes.forbidden",
- "application/x-java-applet,application/zip,application/x-tar,text/x-shellscript")
+ "application/x-java-applet,application/zip,application/x-tar,"
+ + "application/gzip,application/x-bzip2,application/x-xz,"
+ + "text/x-shellscript,application/x-dosexec,application/x-executable,"
+ + "application/x-java-serialized-object,application/x-mach-binary")
.split(",")));
Review Comment:
`forbiddenTypes` is built by splitting the system property on commas without trimming whitespace. If an operator sets `solr.configset.upload.mimetypes.forbidden` with spaces after commas (a common pattern), entries like " application/zip" won't match and forbidden types could be unintentionally allowed. Consider trimming each token (and filtering empties) when building the set.
##########
solr/core/src/java/org/apache/solr/util/FileTypeMagicUtil.java:
##########
@@ -196,14 +201,110 @@ public static boolean isFileForbiddenInConfigset(byte[] bytes) {
Arrays.asList(
System.getProperty(
"solr.configset.upload.mimetypes.forbidden",
- "application/x-java-applet,application/zip,application/x-tar,text/x-shellscript")
+ "application/x-java-applet,application/zip,application/x-tar,"
+ + "application/gzip,application/x-bzip2,application/x-xz,"
+ + "text/x-shellscript,application/x-dosexec,application/x-executable,"
+ + "application/x-java-serialized-object,application/x-mach-binary")
.split(",")));
- private String guessTypeFallbackToOctetStream(ContentInfo contentInfo) {
- if (contentInfo == null) {
- return ContentType.OTHER.getMimeType();
- } else {
- return contentInfo.getContentType().getMimeType();
- }
+ /**
+ * Detects JVM class files by the 0xCAFEBABE magic. Kotlin, Scala and Groovy use the same bytes.
+ */
+ private static boolean isJavaClass(byte[] b) {
+ return b.length >= 4
+ && (b[0] & 0xFF) == 0xCA
+ && (b[1] & 0xFF) == 0xFE
+ && (b[2] & 0xFF) == 0xBA
+ && (b[3] & 0xFF) == 0xBE;
+ }
+
+ /**
+ * Detects ZIP/JAR archives by the PK magic at offset 0. Handles three signatures: PK\x03\x04
+ * (local file header), PK\x05\x06 (empty archive), PK\x07\x08 (data descriptor).
+ *
+ * <p>Polyglot files (e.g. JPEG+ZIP) are not detected: their ZIP content is appended after the
+ * outer format's end marker and does not appear at offset 0.
+ */
+ private static boolean isZip(byte[] b) {
+ return b.length >= 4
+ && b[0] == 'P'
+ && b[1] == 'K'
+ && ((b[2] == 0x03 && b[3] == 0x04)
+ || (b[2] == 0x05 && b[3] == 0x06)
+ || (b[2] == 0x07 && b[3] == 0x08));
+ }
+
+ /**
+ * Detects POSIX/GNU ustar TAR archives via the magic at offset 257. V7-format tars have no magic
+ * and are not detected, but they are essentially extinct.
+ */
+ private static boolean isTar(byte[] b) {
+ return b.length >= 262
+ && b[257] == 'u'
+ && b[258] == 's'
+ && b[259] == 't'
+ && b[260] == 'a'
+ && b[261] == 'r';
+ }
+
+ /** Detects gzip compressed files (magic: 1F 8B). */
+ private static boolean isGzip(byte[] b) {
+ return b.length >= 2 && (b[0] & 0xFF) == 0x1F && (b[1] & 0xFF) == 0x8B;
+ }
+
+ /** Detects bzip2 compressed files (magic: "BZh"). */
+ private static boolean isBzip2(byte[] b) {
+ return b.length >= 3 && b[0] == 'B' && b[1] == 'Z' && b[2] == 'h';
+ }
+
+ /** Detects xz compressed files (magic: FD 37 7A 58 5A 00). */
+ private static boolean isXz(byte[] b) {
+ return b.length >= 6
+ && (b[0] & 0xFF) == 0xFD
+ && b[1] == '7'
+ && b[2] == 'z'
+ && b[3] == 'X'
+ && b[4] == 'Z'
+ && b[5] == 0x00;
+ }
+
+ /** Detects shell scripts by the shebang (#!) at the start of the file. */
Review Comment:
`isShellScript` currently classifies any file starting with a shebang (`#!`) as `text/x-shellscript`, but the Javadoc says “shell or bash script”. Since this will also match e.g. Python/Ruby/Perl scripts (as your tests demonstrate), either tighten detection to shell-specific interpreters or update the documentation/type description to reflect “shebang script” rather than “shell script”.
```suggestion
/** Detects shebang-based scripts by the {@code #!} marker at the start of the file. */
```
##########
solr/core/src/test/org/apache/solr/util/FileTypeMagicUtilTest.java:
##########
@@ -29,27 +30,119 @@ public void testGuessMimeType() throws IOException {
assertResourceMimeType("text/x-shellscript", "/magic/shell.sh.txt");
}
+ public void testGuessMimeTypeBytes() {
+ // Empty / null
+ assertEquals("application/octet-stream", FileTypeMagicUtil.INSTANCE.guessMimeType(new byte[0]));
+ assertFalse(FileTypeMagicUtil.isFileForbiddenInConfigset(new byte[0]));
+
+ // Java class: 0xCAFEBABE + version 52 (Java 8)
+ byte[] javaClass = {(byte) 0xCA, (byte) 0xFE, (byte) 0xBA, (byte) 0xBE, 0, 0, 0, 52};
+ assertEquals("application/x-java-applet", FileTypeMagicUtil.INSTANCE.guessMimeType(javaClass));
+
+ // Java class: preview-compiled (minor=0xFFFF, major=61 / Java 17).
+ // A previous version had a signed-integer overflow that allowed these through.
+ byte[] previewClass = {
+ (byte) 0xCA, (byte) 0xFE, (byte) 0xBA, (byte) 0xBE, (byte) 0xFF, (byte) 0xFF, 0, 61
+ };
+ assertEquals(
+ "application/x-java-applet", FileTypeMagicUtil.INSTANCE.guessMimeType(previewClass));
+
+ // ZIP: PK\x03\x04
+ byte[] zip = {'P', 'K', 0x03, 0x04, 0, 0, 0, 0};
+ assertEquals("application/zip", FileTypeMagicUtil.INSTANCE.guessMimeType(zip));
+
+ // ZIP: PK\x05\x06 (empty archive)
+ byte[] emptyZip = {'P', 'K', 0x05, 0x06, 0, 0, 0, 0};
+ assertEquals("application/zip", FileTypeMagicUtil.INSTANCE.guessMimeType(emptyZip));
+
+ // ZIP: PK\x07\x08 (data-descriptor signature)
+ byte[] ddZip = {'P', 'K', 0x07, 0x08, 0, 0, 0, 0};
+ assertEquals("application/zip", FileTypeMagicUtil.INSTANCE.guessMimeType(ddZip));
+
+ // gzip compressed file
+ byte[] gzip = {(byte) 0x1F, (byte) 0x8B, 0x08, 0x00};
+ assertEquals("application/gzip", FileTypeMagicUtil.INSTANCE.guessMimeType(gzip));
+
+ // bzip2 compressed file
+ byte[] bzip2 = {'B', 'Z', 'h', '9'};
+ assertEquals("application/x-bzip2", FileTypeMagicUtil.INSTANCE.guessMimeType(bzip2));
+
+ // xz compressed file
+ byte[] xz = {(byte) 0xFD, '7', 'z', 'X', 'Z', 0x00};
+ assertEquals("application/x-xz", FileTypeMagicUtil.INSTANCE.guessMimeType(xz));
+
+ // Shell scripts — various interpreter paths
+ assertShellScript("#!/bin/sh\necho hello\n");
+ assertShellScript("#!/usr/bin/env python3\nprint('hi')\n");
+ assertShellScript("#!/opt/homebrew/bin/python3\nprint('hi')\n");
+ assertShellScript("#! /bin/bash\necho hi\n");
+ assertShellScript("#!/nix/store/xxx-bash/bin/bash\necho hi\n");
+
+ // MZ: Windows EXE / self-extracting ZIP
+ byte[] mz = {'M', 'Z', 0, 0};
+ assertEquals("application/x-dosexec", FileTypeMagicUtil.INSTANCE.guessMimeType(mz));
+
+ // ELF: Linux native binary
+ byte[] elf = {0x7F, 'E', 'L', 'F', 0x02, 0x01};
+ assertEquals("application/x-executable", FileTypeMagicUtil.INSTANCE.guessMimeType(elf));
+
Review Comment:
The new detector adds many additional forbidden MIME types (gzip/bzip2/xz, MZ/ELF/Mach-O, Java serialized, etc.), but the tests here only assert `guessMimeType(...)` for them. Consider also asserting `FileTypeMagicUtil.isFileForbiddenInConfigset(bytes)` is `true` for each of these signatures so the default forbidden list and the detector logic can’t drift apart unnoticed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-18213 Replace simplemagic with built-in magic-byte file type detection [solr]
Posted by "janhoy (via GitHub)" <gi...@apache.org>.
janhoy commented on code in PR #4336:
URL: https://github.com/apache/solr/pull/4336#discussion_r3215363266
##########
solr/core/src/test/org/apache/solr/util/FileTypeMagicUtilTest.java:
##########
@@ -17,40 +17,251 @@
package org.apache.solr.util;
+import java.io.ByteArrayInputStream;
import java.io.IOException;
import java.io.InputStream;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Set;
import org.apache.solr.SolrTestCaseJ4;
+import org.apache.solr.common.SolrException;
public class FileTypeMagicUtilTest extends SolrTestCaseJ4 {
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");
+ // Tests the InputStream code path for each format using inline magic bytes, avoiding binary
+ // blobs in the repository. Text files are still tested via classpath resources.
+ byte[] javaClass = {(byte) 0xCA, (byte) 0xFE, (byte) 0xBA, (byte) 0xBE, 0, 0, 0, 52};
+ assertStreamMimeType("application/x-java-applet", javaClass);
+
+ byte[] jar = {'P', 'K', 0x03, 0x04, 0, 0, 0, 0};
+ assertStreamMimeType("application/zip", jar);
+
+ byte[] tar = new byte[512];
+ tar[257] = 'u';
+ tar[258] = 's';
+ tar[259] = 't';
+ tar[260] = 'a';
+ tar[261] = 'r';
+ assertStreamMimeType("application/x-tar", tar);
+
+ // Shell scripts are plain text — safe to keep as a classpath resource.
assertResourceMimeType("text/x-shellscript", "/magic/shell.sh.txt");
}
+ public void testGuessMimeTypeBytes() {
+ // Empty / null
+ assertEquals("application/octet-stream", FileTypeMagicUtil.INSTANCE.guessMimeType(new byte[0]));
+ assertFalse(FileTypeMagicUtil.isFileForbiddenInConfigset(new byte[0]));
+ assertFalse(FileTypeMagicUtil.isFileForbiddenInConfigset((byte[]) null));
+
+ // Java class: 0xCAFEBABE + version 52 (Java 8)
+ byte[] javaClass = {(byte) 0xCA, (byte) 0xFE, (byte) 0xBA, (byte) 0xBE, 0, 0, 0, 52};
+ assertEquals("application/x-java-applet", FileTypeMagicUtil.INSTANCE.guessMimeType(javaClass));
+
+ // Java class: preview-compiled (minor=0xFFFF, major=61 / Java 17).
+ // A previous version had a signed-integer overflow that allowed these through.
+ byte[] previewClass = {
+ (byte) 0xCA, (byte) 0xFE, (byte) 0xBA, (byte) 0xBE, (byte) 0xFF, (byte) 0xFF, 0, 61
+ };
+ assertEquals(
+ "application/x-java-applet", FileTypeMagicUtil.INSTANCE.guessMimeType(previewClass));
+
+ // ZIP: PK\x03\x04
+ byte[] zip = {'P', 'K', 0x03, 0x04, 0, 0, 0, 0};
+ assertEquals("application/zip", FileTypeMagicUtil.INSTANCE.guessMimeType(zip));
+
+ // ZIP: PK\x05\x06 (empty archive)
+ byte[] emptyZip = {'P', 'K', 0x05, 0x06, 0, 0, 0, 0};
+ assertEquals("application/zip", FileTypeMagicUtil.INSTANCE.guessMimeType(emptyZip));
+
+ // ZIP: PK\x07\x08 (data-descriptor signature)
+ byte[] ddZip = {'P', 'K', 0x07, 0x08, 0, 0, 0, 0};
+ assertEquals("application/zip", FileTypeMagicUtil.INSTANCE.guessMimeType(ddZip));
+
+ // gzip compressed file
+ byte[] gzip = {(byte) 0x1F, (byte) 0x8B, 0x08, 0x00};
+ assertEquals("application/gzip", FileTypeMagicUtil.INSTANCE.guessMimeType(gzip));
+ assertTrue(FileTypeMagicUtil.isFileForbiddenInConfigset(gzip));
+
+ // bzip2 compressed file
+ byte[] bzip2 = {'B', 'Z', 'h', '9'};
+ assertEquals("application/x-bzip2", FileTypeMagicUtil.INSTANCE.guessMimeType(bzip2));
+ assertTrue(FileTypeMagicUtil.isFileForbiddenInConfigset(bzip2));
+
+ // xz compressed file
+ byte[] xz = {(byte) 0xFD, '7', 'z', 'X', 'Z', 0x00};
+ assertEquals("application/x-xz", FileTypeMagicUtil.INSTANCE.guessMimeType(xz));
+ assertTrue(FileTypeMagicUtil.isFileForbiddenInConfigset(xz));
+
+ // Shell scripts — various interpreter paths
+ assertShellScript("#!/bin/sh\necho hello\n");
+ assertShellScript("#!/usr/bin/env python3\nprint('hi')\n");
+ assertShellScript("#!/opt/homebrew/bin/python3\nprint('hi')\n");
+ assertShellScript("#! /bin/bash\necho hi\n");
+ assertShellScript("#!/nix/store/xxx-bash/bin/bash\necho hi\n");
+
+ // MZ: Windows EXE / self-extracting ZIP
+ byte[] mz = {'M', 'Z', 0, 0};
+ assertEquals("application/x-dosexec", FileTypeMagicUtil.INSTANCE.guessMimeType(mz));
+ assertTrue(FileTypeMagicUtil.isFileForbiddenInConfigset(mz));
+
+ // ELF: Linux native binary
+ byte[] elf = {0x7F, 'E', 'L', 'F', 0x02, 0x01};
+ assertEquals("application/x-executable", FileTypeMagicUtil.INSTANCE.guessMimeType(elf));
+ assertTrue(FileTypeMagicUtil.isFileForbiddenInConfigset(elf));
+
+ // Java serialized object
+ byte[] ser = {(byte) 0xAC, (byte) 0xED, 0x00, 0x05};
+ assertEquals(
+ "application/x-java-serialized-object", FileTypeMagicUtil.INSTANCE.guessMimeType(ser));
+ assertTrue(FileTypeMagicUtil.isFileForbiddenInConfigset(ser));
+
+ // Mach-O: all four variants (32/64-bit, big/little-endian)
+ byte[] macho32be = {(byte) 0xFE, (byte) 0xED, (byte) 0xFA, (byte) 0xCE};
+ byte[] macho64be = {(byte) 0xFE, (byte) 0xED, (byte) 0xFA, (byte) 0xCF};
+ byte[] macho32le = {(byte) 0xCE, (byte) 0xFA, (byte) 0xED, (byte) 0xFE};
+ byte[] macho64le = {(byte) 0xCF, (byte) 0xFA, (byte) 0xED, (byte) 0xFE};
+ for (byte[] m : new byte[][] {macho32be, macho64be, macho32le, macho64le}) {
+ assertEquals("application/x-mach-binary", FileTypeMagicUtil.INSTANCE.guessMimeType(m));
+ assertTrue(FileTypeMagicUtil.isFileForbiddenInConfigset(m));
+ }
+
+ // Plain text: not forbidden
+ assertEquals(
+ "application/octet-stream",
+ FileTypeMagicUtil.INSTANCE.guessMimeType("hello world".getBytes(StandardCharsets.UTF_8)));
+ }
+
public void testIsFileForbiddenInConfigset() throws IOException {
- assertResourceForbiddenInConfigset("/magic/HelloWorldJavaClass.class.bin");
+ byte[] javaClass = {(byte) 0xCA, (byte) 0xFE, (byte) 0xBA, (byte) 0xBE, 0, 0, 0, 52};
+ assertTrue(FileTypeMagicUtil.isFileForbiddenInConfigset(new ByteArrayInputStream(javaClass)));
+
+ // Text files are safe to keep as classpath resources.
assertResourceForbiddenInConfigset("/magic/shell.sh.txt");
assertResourceAllowedInConfigset("/magic/plain.txt");
}
+ public void testPolyglotZipNotDetectedFromHeaderOnly() {
+ // When only a leading chunk of bytes is available (no EOCD in the window), a JPEG+ZIP polyglot
+ // is not detected — the JPEG magic at offset 0 wins and the result is octet-stream.
+ // This is an accepted limitation for callers that supply only a file header excerpt.
+ byte[] jpegMagic = {(byte) 0xFF, (byte) 0xD8, (byte) 0xFF, (byte) 0xE0};
+ assertEquals("application/octet-stream", FileTypeMagicUtil.INSTANCE.guessMimeType(jpegMagic));
+ assertFalse(FileTypeMagicUtil.isFileForbiddenInConfigset(jpegMagic));
+ }
+
+ public void testPolyglotZipDetectedByTailScan() throws IOException {
+ // Build a JPEG+ZIP polyglot: JPEG magic at offset 0, minimal ZIP EOCD appended at the tail.
+ // ZIP readers locate the archive by scanning backwards for the EOCD signature (PK\x05\x06),
+ // regardless of what appears at offset 0, making such files genuine ZIP archives.
+ byte[] jpegMagic = {(byte) 0xFF, (byte) 0xD8, (byte) 0xFF, (byte) 0xE0, 0, 0x10};
+ // Minimal valid EOCD: PK\x05\x06 + 18 zero bytes (no entries, no comment).
+ byte[] eocd = {'P', 'K', 0x05, 0x06, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0};
+ byte[] polyglot = concat(jpegMagic, eocd);
+
+ // Complete byte[] contains the EOCD — polyglot is detected and blocked in both paths.
+ assertEquals("application/zip", FileTypeMagicUtil.INSTANCE.guessMimeType(polyglot));
+ assertTrue(FileTypeMagicUtil.isFileForbiddenInConfigset(polyglot));
Review Comment:
I also added a test for polyglot ZIP file. I.e. a valid jpg file which is also a valid zip (or JAR file) at the same time.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-18213 Replace simplemagic with built-in magic-byte file type detection [solr]
Posted by "janhoy (via GitHub)" <gi...@apache.org>.
janhoy commented on code in PR #4336:
URL: https://github.com/apache/solr/pull/4336#discussion_r3215361871
##########
solr/core/src/test/org/apache/solr/util/FileTypeMagicUtilTikaCorpusTest.java:
##########
@@ -0,0 +1,224 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.solr.util;
+
+import java.io.ByteArrayOutputStream;
+import java.io.InputStream;
+import java.io.ObjectOutputStream;
+import java.net.URI;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.ArrayList;
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Locale;
+import java.util.Map;
+import org.apache.solr.SolrTestCaseJ4;
+import org.apache.solr.common.util.SuppressForbidden;
+
+/**
+ * Manual regression test that downloads small binary sample files from Apache Tika's test corpus
+ * (Apache License 2.0) and verifies that {@link FileTypeMagicUtil} correctly identifies each MIME
+ * type it claims to handle.
+ *
+ * <p><b>This test is disabled by default.</b> It requires outbound network access and downloads
+ * several binary files at runtime. It is intended to be run manually in two situations:
+ *
+ * <ol>
+ * <li><b>Adding a new format:</b> after extending {@link FileTypeMagicUtil} with a new magic-byte
+ * detector, add a corresponding entry to the URL map below so future runs validate that real
+ * files of that format are detected correctly.
+ * <li><b>Regression validation:</b> when refactoring the detector, run this test to confirm that
+ * real-world binary samples are still detected as expected.
+ * </ol>
+ *
+ * <p>To run this test, temporarily remove the {@code @Ignore} annotation and run:
+ *
+ * <pre>{@code
+ * ./gradlew :solr:core:test \
+ * --tests "org.apache.solr.util.FileTypeMagicUtilTikaCorpusTest"
+ * }</pre>
+ *
+ * <p>Sample sources:
+ *
+ * <ul>
+ * <li>Apache Tika test corpus (Apache 2.0): <a
+ * href="https://github.com/apache/tika/tree/main/tika-parsers">tika-parsers</a>
+ * <li>XZ Utils test files (public domain): <a
+ * href="https://github.com/tukaani-project/xz/tree/master/tests/files">xz tests</a>
+ * <li>TAR, shell script, and Java serialised object: generated in-test via JDK APIs (no public
+ * corpus provides a reliably uncompressed TAR or a {@code .ser} file).
+ * </ul>
+ */
+// @Ignore("Corpus validation test — downloads external files at runtime. Remove @Ignore to run
+// manually.")
+public class FileTypeMagicUtilTikaCorpusTest extends SolrTestCaseJ4 {
+
+ private static final String TIKA_PKG =
+ "https://raw.githubusercontent.com/apache/tika/main/tika-parsers/"
+ + "tika-parsers-standard/tika-parsers-standard-modules/"
+ + "tika-parser-pkg-module/src/test/resources/test-documents/";
+
+ private static final String TIKA_CODE =
+ "https://raw.githubusercontent.com/apache/tika/main/tika-parsers/"
+ + "tika-parsers-standard/tika-parsers-standard-modules/"
+ + "tika-parser-code-module/src/test/resources/test-documents/";
Review Comment:
Added this new test case which is `@Ignore`'d by default but can be run manually. It will download real binaries from the tika project and other locations and then test our detection on those.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-18213 Replace simplemagic with built-in magic-byte file type detection [solr]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #4336:
URL: https://github.com/apache/solr/pull/4336#discussion_r3142573315
##########
solr/core/src/java/org/apache/solr/util/FileTypeMagicUtil.java:
##########
@@ -97,22 +97,28 @@ public FileVisitResult preVisitDirectory(Path dir, BasicFileAttributes attrs) {
* @return string with content-type or "application/octet-stream" if unknown
*/
public String guessMimeType(Path file) {
- try {
- return guessTypeFallbackToOctetStream(util.findMatch(file.toFile()));
+ try (InputStream in = Files.newInputStream(file)) {
+ return guessMimeType(in);
} catch (IOException e) {
throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, e);
Review Comment:
`guessMimeType(Path)` wraps `IOException` in a `SolrException` without including the file path, which makes failures during configset validation harder to diagnose (previously the thrown error included the filename). Consider including `file` in the exception message (or reusing `isFileForbiddenInConfigset(Path)` in `assertConfigSetFolderLegal`).
```suggestion
throw new SolrException(
SolrException.ErrorCode.SERVER_ERROR,
"Failed to guess mime type for file " + file,
e);
```
##########
solr/core/src/java/org/apache/solr/util/FileTypeMagicUtil.java:
##########
@@ -199,11 +202,63 @@ public static boolean isFileForbiddenInConfigset(byte[] bytes) {
"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();
+ /**
+ * Detects JVM class files by the 0xCAFEBABE magic. Kotlin, Scala and Groovy use the same bytes.
+ */
+ private static boolean isJavaClass(byte[] b) {
+ return b.length >= 4
+ && (b[0] & 0xFF) == 0xCA
+ && (b[1] & 0xFF) == 0xFE
+ && (b[2] & 0xFF) == 0xBA
+ && (b[3] & 0xFF) == 0xBE;
+ }
+
+ /**
+ * Detects ZIP/JAR archives by the PK magic at offset 0. Handles three signatures: PK\x03\x04
+ * (local file header), PK\x05\x06 (empty archive), PK\x07\x08 (data descriptor).
+ *
+ * <p>Polyglot files (e.g. JPEG+ZIP) are not detected: their ZIP content is appended after the
+ * outer format's end marker and does not appear at offset 0.
+ */
+ private static boolean isZip(byte[] b) {
+ return b.length >= 4
+ && b[0] == 'P'
+ && b[1] == 'K'
+ && ((b[2] == 0x03 && b[3] == 0x04)
+ || (b[2] == 0x05 && b[3] == 0x06)
+ || (b[2] == 0x07 && b[3] == 0x08));
+ }
+
+ /**
+ * Detects TAR archives via the POSIX/GNU ustar magic at offset 257, and compressed archives
+ * (gzip, bzip2, xz) which commonly wrap tarballs. V7-format tars have no magic and are not
+ * detected, but they are essentially extinct. Plain compressed files without a tar payload are
+ * also blocked — they have no legitimate use in a Solr configset.
+ */
+ private static boolean isTar(byte[] b) {
+ if (b.length >= 262
+ && b[257] == 'u'
+ && b[258] == 's'
+ && b[259] == 't'
+ && b[260] == 'a'
+ && b[261] == 'r') {
+ return true;
}
+ if (b.length >= 2 && (b[0] & 0xFF) == 0x1F && (b[1] & 0xFF) == 0x8B) return true; // gzip
+ if (b.length >= 3 && b[0] == 'B' && b[1] == 'Z' && b[2] == 'h') return true; // bzip2
+ return b.length >= 6 // xz: FD 37 7A 58 5A 00
+ && (b[0] & 0xFF) == 0xFD
+ && b[1] == '7'
+ && b[2] == 'z'
+ && b[3] == 'X'
+ && b[4] == 'Z'
+ && b[5] == 0x00;
Review Comment:
`isTar` treats any gzip/bzip2/xz-compressed file as `application/x-tar`, even if it’s not a tarball. This is both an inaccurate MIME classification and a behavior change vs the previous magic rules (which only detected actual tar headers). Consider either (a) detecting only real tar (ustar) here, or (b) returning distinct mime types for gzip/bzip2/xz and adding those to the forbidden list explicitly if you intend to block them.
```suggestion
* Detects TAR archives via the POSIX/GNU ustar magic at offset 257. V7-format tars have no
* magic and are not detected, but they are essentially extinct.
*/
private static boolean isTar(byte[] b) {
return b.length >= 262
&& b[257] == 'u'
&& b[258] == 's'
&& b[259] == 't'
&& b[260] == 'a'
&& b[261] == 'r';
```
##########
solr/core/src/java/org/apache/solr/util/FileTypeMagicUtil.java:
##########
@@ -199,11 +202,63 @@ public static boolean isFileForbiddenInConfigset(byte[] bytes) {
"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();
+ /**
+ * Detects JVM class files by the 0xCAFEBABE magic. Kotlin, Scala and Groovy use the same bytes.
+ */
+ private static boolean isJavaClass(byte[] b) {
+ return b.length >= 4
+ && (b[0] & 0xFF) == 0xCA
+ && (b[1] & 0xFF) == 0xFE
+ && (b[2] & 0xFF) == 0xBA
+ && (b[3] & 0xFF) == 0xBE;
+ }
+
+ /**
+ * Detects ZIP/JAR archives by the PK magic at offset 0. Handles three signatures: PK\x03\x04
+ * (local file header), PK\x05\x06 (empty archive), PK\x07\x08 (data descriptor).
+ *
+ * <p>Polyglot files (e.g. JPEG+ZIP) are not detected: their ZIP content is appended after the
+ * outer format's end marker and does not appear at offset 0.
+ */
+ private static boolean isZip(byte[] b) {
+ return b.length >= 4
+ && b[0] == 'P'
+ && b[1] == 'K'
+ && ((b[2] == 0x03 && b[3] == 0x04)
+ || (b[2] == 0x05 && b[3] == 0x06)
+ || (b[2] == 0x07 && b[3] == 0x08));
+ }
+
+ /**
+ * Detects TAR archives via the POSIX/GNU ustar magic at offset 257, and compressed archives
+ * (gzip, bzip2, xz) which commonly wrap tarballs. V7-format tars have no magic and are not
+ * detected, but they are essentially extinct. Plain compressed files without a tar payload are
+ * also blocked — they have no legitimate use in a Solr configset.
+ */
+ private static boolean isTar(byte[] b) {
+ if (b.length >= 262
+ && b[257] == 'u'
+ && b[258] == 's'
+ && b[259] == 't'
+ && b[260] == 'a'
+ && b[261] == 'r') {
+ return true;
}
+ if (b.length >= 2 && (b[0] & 0xFF) == 0x1F && (b[1] & 0xFF) == 0x8B) return true; // gzip
+ if (b.length >= 3 && b[0] == 'B' && b[1] == 'Z' && b[2] == 'h') return true; // bzip2
+ return b.length >= 6 // xz: FD 37 7A 58 5A 00
+ && (b[0] & 0xFF) == 0xFD
+ && b[1] == '7'
+ && b[2] == 'z'
+ && b[3] == 'X'
+ && b[4] == 'Z'
+ && b[5] == 0x00;
+ }
+
+ private static boolean isShellScript(byte[] b) {
+ if (b.length < 2) return false;
+ // lookingAt() matches from the start without requiring a full-string match.
+ String prefix = new String(b, 0, Math.min(b.length, 64), StandardCharsets.ISO_8859_1);
+ return SHELL_SHEBANG.matcher(prefix).lookingAt();
Review Comment:
Shell-script detection currently builds a `String` and runs a regex just to check for `#!`. This can be simplified to a direct byte-prefix check (`b[0]=='#' && b[1]=='!'`) to avoid allocations and regex overhead, and would better match the intent of the method (since the regex is effectively constant).
```suggestion
return b.length >= 2 && b[0] == '#' && b[1] == '!';
```
##########
solr/core/src/java/org/apache/solr/util/FileTypeMagicUtil.java:
##########
@@ -199,11 +202,63 @@ public static boolean isFileForbiddenInConfigset(byte[] bytes) {
"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();
+ /**
+ * Detects JVM class files by the 0xCAFEBABE magic. Kotlin, Scala and Groovy use the same bytes.
+ */
+ private static boolean isJavaClass(byte[] b) {
+ return b.length >= 4
+ && (b[0] & 0xFF) == 0xCA
+ && (b[1] & 0xFF) == 0xFE
+ && (b[2] & 0xFF) == 0xBA
+ && (b[3] & 0xFF) == 0xBE;
Review Comment:
`isJavaClass` now flags any file starting with `0xCAFEBABE` as a class file. The previous implementation also validated the version field, which reduces accidental false-positives. Consider additionally checking that `b.length >= 8` and that the major/minor version bytes represent a plausible classfile version (while still allowing preview classfiles with minor `0xFFFF`).
```suggestion
if (b.length < 8
|| (b[0] & 0xFF) != 0xCA
|| (b[1] & 0xFF) != 0xFE
|| (b[2] & 0xFF) != 0xBA
|| (b[3] & 0xFF) != 0xBE) {
return false;
}
final int minorVersion = ((b[4] & 0xFF) << 8) | (b[5] & 0xFF);
final int majorVersion = ((b[6] & 0xFF) << 8) | (b[7] & 0xFF);
return majorVersion >= 45 && (minorVersion == 0 || minorVersion == 0xFFFF);
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-18213 Replace simplemagic with built-in magic-byte file type detection [solr]
Posted by "janhoy (via GitHub)" <gi...@apache.org>.
janhoy commented on code in PR #4336:
URL: https://github.com/apache/solr/pull/4336#discussion_r3142577325
##########
solr/core/src/java/org/apache/solr/util/FileTypeMagicUtil.java:
##########
@@ -199,11 +202,63 @@ public static boolean isFileForbiddenInConfigset(byte[] bytes) {
"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();
+ /**
+ * Detects JVM class files by the 0xCAFEBABE magic. Kotlin, Scala and Groovy use the same bytes.
+ */
+ private static boolean isJavaClass(byte[] b) {
+ return b.length >= 4
+ && (b[0] & 0xFF) == 0xCA
+ && (b[1] & 0xFF) == 0xFE
+ && (b[2] & 0xFF) == 0xBA
+ && (b[3] & 0xFF) == 0xBE;
+ }
+
+ /**
+ * Detects ZIP/JAR archives by the PK magic at offset 0. Handles three signatures: PK\x03\x04
+ * (local file header), PK\x05\x06 (empty archive), PK\x07\x08 (data descriptor).
+ *
+ * <p>Polyglot files (e.g. JPEG+ZIP) are not detected: their ZIP content is appended after the
+ * outer format's end marker and does not appear at offset 0.
+ */
+ private static boolean isZip(byte[] b) {
+ return b.length >= 4
+ && b[0] == 'P'
+ && b[1] == 'K'
+ && ((b[2] == 0x03 && b[3] == 0x04)
+ || (b[2] == 0x05 && b[3] == 0x06)
+ || (b[2] == 0x07 && b[3] == 0x08));
+ }
+
+ /**
+ * Detects TAR archives via the POSIX/GNU ustar magic at offset 257, and compressed archives
+ * (gzip, bzip2, xz) which commonly wrap tarballs. V7-format tars have no magic and are not
+ * detected, but they are essentially extinct. Plain compressed files without a tar payload are
+ * also blocked — they have no legitimate use in a Solr configset.
+ */
+ private static boolean isTar(byte[] b) {
+ if (b.length >= 262
+ && b[257] == 'u'
+ && b[258] == 's'
+ && b[259] == 't'
+ && b[260] == 'a'
+ && b[261] == 'r') {
+ return true;
}
+ if (b.length >= 2 && (b[0] & 0xFF) == 0x1F && (b[1] & 0xFF) == 0x8B) return true; // gzip
+ if (b.length >= 3 && b[0] == 'B' && b[1] == 'Z' && b[2] == 'h') return true; // bzip2
+ return b.length >= 6 // xz: FD 37 7A 58 5A 00
+ && (b[0] & 0xFF) == 0xFD
+ && b[1] == '7'
+ && b[2] == 'z'
+ && b[3] == 'X'
+ && b[4] == 'Z'
+ && b[5] == 0x00;
Review Comment:
I think it is more correct to detect gzip/bzip2/xz as individual types and add them to the forbidden list
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-18213 Replace simplemagic with built-in magic-byte file type detection [solr]
Posted by "janhoy (via GitHub)" <gi...@apache.org>.
janhoy commented on code in PR #4336:
URL: https://github.com/apache/solr/pull/4336#discussion_r3142575681
##########
solr/core/src/java/org/apache/solr/util/FileTypeMagicUtil.java:
##########
@@ -199,11 +202,63 @@ public static boolean isFileForbiddenInConfigset(byte[] bytes) {
"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();
+ /**
+ * Detects JVM class files by the 0xCAFEBABE magic. Kotlin, Scala and Groovy use the same bytes.
+ */
+ private static boolean isJavaClass(byte[] b) {
+ return b.length >= 4
+ && (b[0] & 0xFF) == 0xCA
+ && (b[1] & 0xFF) == 0xFE
+ && (b[2] & 0xFF) == 0xBA
+ && (b[3] & 0xFF) == 0xBE;
Review Comment:
We accept the risk
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org