apache / apache/stormcrawler

CharsetIdentification retries the meta charset lookup ten bytes at a time by recursing, and the call sits outside JSoupParserBolt's catch block

Open
#2,092 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Java
Stars
995
Forks
292
Avg merge
2d 49m
Merged PRs (30d)
62

Description

## What happens
`getCharsetFromMeta` looks for ` thrown = new AtomicReference<>();
AtomicReference charset = new AtomicReference<>();
Runnable body =
() -> {
try {
charset.set(
CharsetIdentification.getCharset(new Metadata(), content, MAXLENGTH));
} catch (Throwable t) {
thrown.set(t);
}
};
Thread t = new Thread(null, body, "charset-detection", stackSize);
t.start();
t.join();
return thrown.get();
}

@Test
void smallDocumentWithUnterminatedMetaIsHandled() throws Exception {
Throwable thrown = runOnStack(unterminatedMetaCharset(20000), 1024 * 1024);
Assertions.assertNull(thrown, "charset detection threw " + thrown);
}

@Test
void largeDocumentWithUnterminatedMetaIsHandled() throws Exception {
// 400 KB is an ordinary page size and is fetched whole under the library
// default http.content.limit of -1
Throwable thrown = runOnStack(unterminatedMetaCharset(400_000), 1024 * 1024);
Assertions.assertNull(thrown, "charset detection threw " + thrown);
}
}
```

Run it:

```
mvn -pl core test -Dtest=CharsetIdentificationUnterminatedMetaTest
```

It runs the call on a thread with a pinned 1 MB stack so the result does not depend on the surefire defaults, and asserts the correct behaviour, so it fails on main. The 20 KB case passes, the 400 KB case fails.

```
[ERROR] Tests run: 2, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 0.133 s <<< FAILURE! -- in org.apache.stormcrawler.util.CharsetIdentificationUnterminatedMetaTest
[ERROR] org.apache.stormcrawler.util.CharsetIdentificationUnterminatedMetaTest.largeDocumentWithUnterminatedMetaIsHandled -- Time elapsed: 0.091 s <<< FAILURE!
org.opentest4j.AssertionFailedError: charset detection threw java.lang.StackOverflowError ==> expected: but was:
```

## Suggested fix
Make the retry iterative and bounded in `CharsetIdentification.getCharsetFromMeta`: search for the closing quote once over `min(buffer.length, someCap)` instead of re-decoding a window that grows by ten bytes per call, and give up and return null if it is not found. Decoding the window once also removes the quadratic copying. Separately, move the `getCharset` and `getCharsetFast` calls in `JSoupParserBolt.execute` inside the existing `try` that ends at the `catch (Throwable)`, so a failure there routes through `handleException` like every other parse failure. Both changes are behaviour preserving for well formed pages.

Contributor guide

Open the contributing guide

Research direction

Start with core/src/main/java/org/apache/stormcrawler/util/CharsetIdentification.java:189-196 and core/src/main/java/org/apache/stormcrawler/bolt/JSoupParserBolt.java:274-299, then run CharsetIdentificationUnterminatedMetaTest with the provided Maven command. Check the unterminated-meta cases and the existing bolt error path; done means both tests pass without stack exhaustion and failures are handled by the bolt.

Written by the indexing model from the issue text.

Assessment

Tech stack
java
Domain
backend, distributed-systems
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
76/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.