Skip to content

Commit 65a2f3a

Browse files
gsingersclaude
andauthored
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 <noreply@anthropic.com>
1 parent a2275e2 commit 65a2f3a

3 files changed

Lines changed: 134 additions & 8 deletions

File tree

CHANGES.txt

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,11 @@ Release 4.0.0 - ???
66
opt-in Pkcs7Detector surfaces the subtype at detect() time,
77
but must be enabled via configuration (TIKA-1997).
88

9+
OTHER CHANGES
10+
11+
* MagicDetector now compiles its regular expression once, in the
12+
constructor, instead of recompiling it on every match (TIKA-4796).
13+
914

1015
Release 4.0.0-beta-1 - 6/29/2026
1116

tika-core/src/main/java/org/apache/tika/detect/MagicDetector.java

Lines changed: 27 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,9 @@
4141
* Because this works on bytes, not characters, by default any string
4242
* matching is done as ISO_8859_1. To use an explicit different
4343
* encoding, supply a type other than "string" / "stringignorecase"
44+
* <p>
45+
* Instances of this class are immutable and safe for use by multiple
46+
* concurrent threads.
4447
*
4548
* @since Apache Tika 0.3
4649
*/
@@ -94,6 +97,12 @@ public class MagicDetector implements Detector {
9497
* starts at this offset.
9598
*/
9699
private final int offsetRangeEnd;
100+
/**
101+
* The compiled form of {@link #pattern} when {@link #isRegex} is true,
102+
* <code>null</code> otherwise. Compiled once here rather than per match,
103+
* as every input to it is fixed at construction time.
104+
*/
105+
private final Pattern compiledPattern;
97106

98107
/**
99108
* Creates a detector for input documents that have the exact given byte
@@ -140,6 +149,16 @@ public MagicDetector(MediaType type, byte[] pattern, byte[] mask, boolean isRege
140149
/**
141150
* Creates a detector for input documents that meet the specified
142151
* magic match.
152+
* <p>
153+
* When <code>isRegex</code> is true the pattern is compiled here rather
154+
* than on each match, so a malformed pattern is reported by this
155+
* constructor instead of by the first call to
156+
* {@link #detect(TikaInputStream, Metadata, ParseContext)} or
157+
* {@link #matches(byte[])}.
158+
*
159+
* @throws java.util.regex.PatternSyntaxException if <code>isRegex</code>
160+
* is true and <code>pattern</code> is not a valid regular
161+
* expression
143162
*/
144163
public MagicDetector(MediaType type, byte[] pattern, byte[] mask, boolean isRegex,
145164
boolean isStringIgnoreCase, int offsetRangeBegin, int offsetRangeEnd) {
@@ -183,6 +202,13 @@ public MagicDetector(MediaType type, byte[] pattern, byte[] mask, boolean isRege
183202
}
184203
}
185204

205+
if (this.isRegex) {
206+
int flags = this.isStringIgnoreCase ? Pattern.CASE_INSENSITIVE : 0;
207+
this.compiledPattern = Pattern.compile(new String(this.pattern, UTF_8), flags);
208+
} else {
209+
this.compiledPattern = null;
210+
}
211+
186212
this.offsetRangeBegin = offsetRangeBegin;
187213
this.offsetRangeEnd = offsetRangeEnd;
188214
}
@@ -444,20 +470,13 @@ public boolean matches(byte[] data) {
444470
*/
445471
private boolean matchesBuffer(byte[] buffer, int startOffset, int endOffset) {
446472
if (this.isRegex) {
447-
int flags = 0;
448-
if (this.isStringIgnoreCase) {
449-
flags = Pattern.CASE_INSENSITIVE;
450-
}
451-
452-
Pattern p = Pattern.compile(new String(this.pattern, UTF_8), flags);
453-
454473
int bufferLen = Math.min(buffer.length - startOffset, length + (endOffset - startOffset));
455474
if (bufferLen <= 0) {
456475
return false;
457476
}
458477
ByteBuffer bb = ByteBuffer.wrap(buffer, startOffset, bufferLen);
459478
CharBuffer result = ISO_8859_1.decode(bb);
460-
Matcher m = p.matcher(result);
479+
Matcher m = compiledPattern.matcher(result);
461480

462481
// Loop until we've covered the entire offset range
463482
for (int i = 0; i <= endOffset - startOffset; i++) {

tika-core/src/test/java/org/apache/tika/detect/MagicDetectorTest.java

Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,11 +20,19 @@
2020
import static java.nio.charset.StandardCharsets.UTF_16BE;
2121
import static java.nio.charset.StandardCharsets.UTF_16LE;
2222
import static org.junit.jupiter.api.Assertions.assertEquals;
23+
import static org.junit.jupiter.api.Assertions.assertFalse;
24+
import static org.junit.jupiter.api.Assertions.assertTrue;
2325
import static org.junit.jupiter.api.Assertions.fail;
2426

2527
import java.io.ByteArrayInputStream;
2628
import java.io.IOException;
2729
import java.io.InputStream;
30+
import java.util.ArrayList;
31+
import java.util.List;
32+
import java.util.concurrent.ExecutorService;
33+
import java.util.concurrent.Executors;
34+
import java.util.concurrent.Future;
35+
import java.util.concurrent.TimeUnit;
2836

2937
import org.apache.commons.io.IOUtils;
3038
import org.junit.jupiter.api.Test;
@@ -207,6 +215,100 @@ public void testDetectString() throws Exception {
207215
assertDetect(detector, testMT, data.getBytes(US_ASCII));
208216
}
209217

218+
/**
219+
* The byte[] path is what MagicMatch.eval uses for every magic in
220+
* tika-mimetypes.xml, but it was only ever exercised indirectly. Cover the
221+
* regex branch of it directly.
222+
*/
223+
@Test
224+
public void testMatchesByteArrayRegEx() {
225+
MediaType pdf = new MediaType("application", "pdf");
226+
MagicDetector detector =
227+
new MagicDetector(pdf, "(?s)\\A.{0,144}%PDF-".getBytes(US_ASCII), null, true, 0, 0);
228+
229+
assertTrue(detector.matches("%PDF-1.0".getBytes(US_ASCII)));
230+
assertTrue(detector.matches(("0 10 20 30 40 50 6" +
231+
"0 70 80 90 100 110 1" +
232+
"20 130 140" + "34%PDF-1.0").getBytes(US_ASCII)));
233+
assertFalse(detector.matches(("0 10 20 30 40 50 6" +
234+
"0 70 80 90 100 110 1" +
235+
"20 130 140" + "345%PDF-1.0").getBytes(US_ASCII)));
236+
assertFalse(detector.matches("".getBytes(US_ASCII)));
237+
assertFalse(detector.matches(null));
238+
239+
// an offset range, mirroring the wider windows used in tika-mimetypes.xml
240+
MediaType xhtml = new MediaType("application", "xhtml+xml");
241+
String pattern = "(?s)\\x3chtml xmlns=\"http://www\\.w3\\.org/1999/xhtml" +
242+
"\".*\\x3ctitle\\x3e.*\\x3c/title\\x3e";
243+
MagicDetector ranged =
244+
new MagicDetector(xhtml, pattern.getBytes(US_ASCII), null, true, 0, 8192);
245+
assertTrue(ranged.matches(("<html xmlns=\"http://www.w3.org/1999/xhtml\">" +
246+
"<head><title>XHTML test document</title></head>").getBytes(US_ASCII)));
247+
assertFalse(ranged.matches("<html><head><title>no namespace</title></head>"
248+
.getBytes(US_ASCII)));
249+
}
250+
251+
/**
252+
* A MagicDetector is built once and reused for the life of the process, so
253+
* repeated calls must be independent of each other. Guards the compiled
254+
* Pattern against per-call state leaking in.
255+
*/
256+
@Test
257+
public void testRegExDetectorRepeatedCallsStable() throws Exception {
258+
MediaType html = new MediaType("text", "html");
259+
String pattern = "(?s)\\A.{0,1024}\\x3c\\!(?:DOCTYPE|doctype) (?:HTML|html) ";
260+
MagicDetector detector =
261+
new MagicDetector(html, pattern.getBytes(US_ASCII), null, true, 0, 0);
262+
263+
byte[] match = "<!DOCTYPE HTML PUBLIC \"-//W3C//DTD HTML 4.01//EN\">".getBytes(US_ASCII);
264+
byte[] noMatch = "<html><head><title>plain</title></head>".getBytes(US_ASCII);
265+
266+
for (int i = 0; i < 100; i++) {
267+
assertTrue(detector.matches(match), "matches() changed on iteration " + i);
268+
assertFalse(detector.matches(noMatch), "matches() changed on iteration " + i);
269+
assertDetect(detector, html, match);
270+
assertDetect(detector, MediaType.OCTET_STREAM, noMatch);
271+
}
272+
}
273+
274+
/**
275+
* MimeTypes shares one MagicDetector instance per magic clause across every
276+
* caller, so the regex path has to be safe to use concurrently.
277+
*/
278+
@Test
279+
public void testRegExDetectorConcurrent() throws Exception {
280+
MediaType pdf = new MediaType("application", "pdf");
281+
MagicDetector detector =
282+
new MagicDetector(pdf, "(?s)\\A.{0,144}%PDF-".getBytes(US_ASCII), null, true, 0, 0);
283+
284+
byte[] match = "%PDF-1.4\nsome trailing content".getBytes(US_ASCII);
285+
byte[] noMatch = "not a pdf at all".getBytes(US_ASCII);
286+
287+
int threads = 8;
288+
int iterations = 200;
289+
ExecutorService executor = Executors.newFixedThreadPool(threads);
290+
try {
291+
List<Future<?>> futures = new ArrayList<>();
292+
for (int t = 0; t < threads; t++) {
293+
futures.add(executor.submit(() -> {
294+
for (int i = 0; i < iterations; i++) {
295+
assertTrue(detector.matches(match));
296+
assertFalse(detector.matches(noMatch));
297+
assertEquals(pdf, detector.detect(TikaInputStream.get(match), new Metadata(),
298+
new ParseContext()));
299+
}
300+
return null;
301+
}));
302+
}
303+
for (Future<?> future : futures) {
304+
// an assertion failure on a worker surfaces here as an ExecutionException
305+
future.get(60, TimeUnit.SECONDS);
306+
}
307+
} finally {
308+
executor.shutdownNow();
309+
}
310+
}
311+
210312
private void assertDetect(Detector detector, MediaType type, String data) {
211313
byte[] bytes = data.getBytes(US_ASCII);
212314
assertDetect(detector, type, bytes);

0 commit comments

Comments
 (0)