This is an automated email from the ASF dual-hosted git repository.
tballison pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/tika.git
The following commit(s) were added to refs/heads/main by this push:
new 65a2f3ab6f TIKA-4796: compile MagicDetector regex once at construction
(#2968)
65a2f3ab6f is described below
commit 65a2f3ab6fb427855d9e09a6954fac52c8bcde76
Author: Grant Ingersoll <[email protected]>
AuthorDate: Tue Jul 28 13:36:13 2026 -0400
TIKA-4796: compile MagicDetector regex once at construction (#2968)
MagicDetector recompiled its Pattern on every regex match even though
the pattern bytes and the case-insensitivity flag are both fixed at
construction time. Compile once in the constructor and reuse the
Pattern; Pattern is immutable and its Matcher is still created per
call, so the regex path stays thread-safe.
Adds direct coverage for the regex branch of matches(byte[]), which
MagicMatch.eval uses for every magic in tika-mimetypes.xml but which
was previously only exercised indirectly, plus repeated-call and
concurrent-use tests over a shared detector instance.
Claude-Session: https://claude.ai/code/session_01Pp6TVDKF7ztw7SS7hwwuFd
Co-authored-by: Claude Opus 5 <[email protected]>
---
CHANGES.txt | 5 +
.../java/org/apache/tika/detect/MagicDetector.java | 35 +++++--
.../org/apache/tika/detect/MagicDetectorTest.java | 102 +++++++++++++++++++++
3 files changed, 134 insertions(+), 8 deletions(-)
diff --git a/CHANGES.txt b/CHANGES.txt
index 0d31e36091..e46059d1a1 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -6,6 +6,11 @@ Release 4.0.0 - ???
opt-in Pkcs7Detector surfaces the subtype at detect() time,
but must be enabled via configuration (TIKA-1997).
+ OTHER CHANGES
+
+ * MagicDetector now compiles its regular expression once, in the
+ constructor, instead of recompiling it on every match (TIKA-4796).
+
Release 4.0.0-beta-1 - 6/29/2026
diff --git a/tika-core/src/main/java/org/apache/tika/detect/MagicDetector.java
b/tika-core/src/main/java/org/apache/tika/detect/MagicDetector.java
index ad59bd4751..80a7725453 100644
--- a/tika-core/src/main/java/org/apache/tika/detect/MagicDetector.java
+++ b/tika-core/src/main/java/org/apache/tika/detect/MagicDetector.java
@@ -41,6 +41,9 @@ import org.apache.tika.parser.ParseContext;
* Because this works on bytes, not characters, by default any string
* matching is done as ISO_8859_1. To use an explicit different
* encoding, supply a type other than "string" / "stringignorecase"
+ * <p>
+ * Instances of this class are immutable and safe for use by multiple
+ * concurrent threads.
*
* @since Apache Tika 0.3
*/
@@ -94,6 +97,12 @@ public class MagicDetector implements Detector {
* starts at this offset.
*/
private final int offsetRangeEnd;
+ /**
+ * The compiled form of {@link #pattern} when {@link #isRegex} is true,
+ * <code>null</code> otherwise. Compiled once here rather than per match,
+ * as every input to it is fixed at construction time.
+ */
+ private final Pattern compiledPattern;
/**
* Creates a detector for input documents that have the exact given byte
@@ -140,6 +149,16 @@ public class MagicDetector implements Detector {
/**
* Creates a detector for input documents that meet the specified
* magic match.
+ * <p>
+ * When <code>isRegex</code> is true the pattern is compiled here rather
+ * than on each match, so a malformed pattern is reported by this
+ * constructor instead of by the first call to
+ * {@link #detect(TikaInputStream, Metadata, ParseContext)} or
+ * {@link #matches(byte[])}.
+ *
+ * @throws java.util.regex.PatternSyntaxException if <code>isRegex</code>
+ * is true and <code>pattern</code> is not a valid regular
+ * expression
*/
public MagicDetector(MediaType type, byte[] pattern, byte[] mask, boolean
isRegex,
boolean isStringIgnoreCase, int offsetRangeBegin, int
offsetRangeEnd) {
@@ -183,6 +202,13 @@ public class MagicDetector implements Detector {
}
}
+ if (this.isRegex) {
+ int flags = this.isStringIgnoreCase ? Pattern.CASE_INSENSITIVE : 0;
+ this.compiledPattern = Pattern.compile(new String(this.pattern,
UTF_8), flags);
+ } else {
+ this.compiledPattern = null;
+ }
+
this.offsetRangeBegin = offsetRangeBegin;
this.offsetRangeEnd = offsetRangeEnd;
}
@@ -444,20 +470,13 @@ public class MagicDetector implements Detector {
*/
private boolean matchesBuffer(byte[] buffer, int startOffset, int
endOffset) {
if (this.isRegex) {
- int flags = 0;
- if (this.isStringIgnoreCase) {
- flags = Pattern.CASE_INSENSITIVE;
- }
-
- Pattern p = Pattern.compile(new String(this.pattern, UTF_8),
flags);
-
int bufferLen = Math.min(buffer.length - startOffset, length +
(endOffset - startOffset));
if (bufferLen <= 0) {
return false;
}
ByteBuffer bb = ByteBuffer.wrap(buffer, startOffset, bufferLen);
CharBuffer result = ISO_8859_1.decode(bb);
- Matcher m = p.matcher(result);
+ Matcher m = compiledPattern.matcher(result);
// Loop until we've covered the entire offset range
for (int i = 0; i <= endOffset - startOffset; i++) {
diff --git
a/tika-core/src/test/java/org/apache/tika/detect/MagicDetectorTest.java
b/tika-core/src/test/java/org/apache/tika/detect/MagicDetectorTest.java
index 4023923065..227029824b 100644
--- a/tika-core/src/test/java/org/apache/tika/detect/MagicDetectorTest.java
+++ b/tika-core/src/test/java/org/apache/tika/detect/MagicDetectorTest.java
@@ -20,11 +20,19 @@ import static java.nio.charset.StandardCharsets.US_ASCII;
import static java.nio.charset.StandardCharsets.UTF_16BE;
import static java.nio.charset.StandardCharsets.UTF_16LE;
import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.junit.jupiter.api.Assertions.fail;
import java.io.ByteArrayInputStream;
import java.io.IOException;
import java.io.InputStream;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.Future;
+import java.util.concurrent.TimeUnit;
import org.apache.commons.io.IOUtils;
import org.junit.jupiter.api.Test;
@@ -207,6 +215,100 @@ public class MagicDetectorTest extends TikaTest {
assertDetect(detector, testMT, data.getBytes(US_ASCII));
}
+ /**
+ * The byte[] path is what MagicMatch.eval uses for every magic in
+ * tika-mimetypes.xml, but it was only ever exercised indirectly. Cover the
+ * regex branch of it directly.
+ */
+ @Test
+ public void testMatchesByteArrayRegEx() {
+ MediaType pdf = new MediaType("application", "pdf");
+ MagicDetector detector =
+ new MagicDetector(pdf,
"(?s)\\A.{0,144}%PDF-".getBytes(US_ASCII), null, true, 0, 0);
+
+ assertTrue(detector.matches("%PDF-1.0".getBytes(US_ASCII)));
+ assertTrue(detector.matches(("0 10 20 30
40 50 6" +
+ "0 70 80 90 100 110 1"
+
+ "20 130 140" + "34%PDF-1.0").getBytes(US_ASCII)));
+ assertFalse(detector.matches(("0 10 20 30
40 50 6" +
+ "0 70 80 90 100 110 1"
+
+ "20 130 140" + "345%PDF-1.0").getBytes(US_ASCII)));
+ assertFalse(detector.matches("".getBytes(US_ASCII)));
+ assertFalse(detector.matches(null));
+
+ // an offset range, mirroring the wider windows used in
tika-mimetypes.xml
+ MediaType xhtml = new MediaType("application", "xhtml+xml");
+ String pattern = "(?s)\\x3chtml
xmlns=\"http://www\\.w3\\.org/1999/xhtml"; +
+ "\".*\\x3ctitle\\x3e.*\\x3c/title\\x3e";
+ MagicDetector ranged =
+ new MagicDetector(xhtml, pattern.getBytes(US_ASCII), null,
true, 0, 8192);
+ assertTrue(ranged.matches(("<html
xmlns=\"http://www.w3.org/1999/xhtml\";>" +
+ "<head><title>XHTML test
document</title></head>").getBytes(US_ASCII)));
+ assertFalse(ranged.matches("<html><head><title>no
namespace</title></head>"
+ .getBytes(US_ASCII)));
+ }
+
+ /**
+ * A MagicDetector is built once and reused for the life of the process, so
+ * repeated calls must be independent of each other. Guards the compiled
+ * Pattern against per-call state leaking in.
+ */
+ @Test
+ public void testRegExDetectorRepeatedCallsStable() throws Exception {
+ MediaType html = new MediaType("text", "html");
+ String pattern = "(?s)\\A.{0,1024}\\x3c\\!(?:DOCTYPE|doctype)
(?:HTML|html) ";
+ MagicDetector detector =
+ new MagicDetector(html, pattern.getBytes(US_ASCII), null,
true, 0, 0);
+
+ byte[] match = "<!DOCTYPE HTML PUBLIC \"-//W3C//DTD HTML
4.01//EN\">".getBytes(US_ASCII);
+ byte[] noMatch =
"<html><head><title>plain</title></head>".getBytes(US_ASCII);
+
+ for (int i = 0; i < 100; i++) {
+ assertTrue(detector.matches(match), "matches() changed on
iteration " + i);
+ assertFalse(detector.matches(noMatch), "matches() changed on
iteration " + i);
+ assertDetect(detector, html, match);
+ assertDetect(detector, MediaType.OCTET_STREAM, noMatch);
+ }
+ }
+
+ /**
+ * MimeTypes shares one MagicDetector instance per magic clause across
every
+ * caller, so the regex path has to be safe to use concurrently.
+ */
+ @Test
+ public void testRegExDetectorConcurrent() throws Exception {
+ MediaType pdf = new MediaType("application", "pdf");
+ MagicDetector detector =
+ new MagicDetector(pdf,
"(?s)\\A.{0,144}%PDF-".getBytes(US_ASCII), null, true, 0, 0);
+
+ byte[] match = "%PDF-1.4\nsome trailing content".getBytes(US_ASCII);
+ byte[] noMatch = "not a pdf at all".getBytes(US_ASCII);
+
+ int threads = 8;
+ int iterations = 200;
+ ExecutorService executor = Executors.newFixedThreadPool(threads);
+ try {
+ List<Future<?>> futures = new ArrayList<>();
+ for (int t = 0; t < threads; t++) {
+ futures.add(executor.submit(() -> {
+ for (int i = 0; i < iterations; i++) {
+ assertTrue(detector.matches(match));
+ assertFalse(detector.matches(noMatch));
+ assertEquals(pdf,
detector.detect(TikaInputStream.get(match), new Metadata(),
+ new ParseContext()));
+ }
+ return null;
+ }));
+ }
+ for (Future<?> future : futures) {
+ // an assertion failure on a worker surfaces here as an
ExecutionException
+ future.get(60, TimeUnit.SECONDS);
+ }
+ } finally {
+ executor.shutdownNow();
+ }
+ }
+
private void assertDetect(Detector detector, MediaType type, String data) {
byte[] bytes = data.getBytes(US_ASCII);
assertDetect(detector, type, bytes);
