rzo1 opened a new issue, #2092:
URL: https://github.com/apache/stormcrawler/issues/2092
## What happens
`getCharsetFromMeta` looks for `<meta charset="` inside the first
`detect.charset.maxlength` bytes. If it finds the opening but no closing quote
before the cutoff, it calls itself with `maxlength + 10`, decoding the whole
window into a new String each time. For a document whose body starts with
`<meta charset="` and never contains another double quote, the recursion depth
is `(content.length - 10000) / 10`, and each frame holds a String of the
growing window. A few hundred KB of body is enough to exhaust the stack.
`JSoupParserBolt` calls charset detection at lines 274 to 280, before the `try`
at line 299, so nothing in the bolt catches it and the worker dies.
## Where
`core/src/main/java/org/apache/stormcrawler/util/CharsetIdentification.java:189-196`,
called from
`core/src/main/java/org/apache/stormcrawler/bolt/JSoupParserBolt.java:274-280`.
Config keys: `detect.charset.maxlength` (10000 in `crawler-default.yaml:251`),
`detect.charset.fast` (false by default, so the full `getCharset` path runs),
`http.content.limit` (`-1` in `crawler-default.yaml:125`, `65536` in the
archetype).
```java
int end = html.indexOf('"', start + 15);
// https://github.com/apache/stormcrawler/issues/870
// try on a slightly larger section of text if it is trimmed
if (end == -1 && ((maxlength + 10) < buffer.length)) {
return getCharsetFromMeta(buffer, maxlength + 10);
}
```
```java
charset =
CharsetIdentification.getCharset(metadata, content,
maxLengthCharsetDetection);
```
## Why it matters
One fetched page ends the worker JVM rather than the parse of that one URL,
which is what the surrounding `catch (Throwable)` and `handleException` in
`JSoupParserBolt` are there to guarantee. The URL stays scheduled, so the
replayed tuple takes the restarted worker down again. This needs a content
limit that lets a few hundred KB through, which is the library default of `-1`;
with the archetype's `http.content.limit: 65536` the depth is about 5.5k
frames, which is CPU churn and repeated copying rather than a crash. Depending
on heap and stack settings the worker may die of memory exhaustion before the
stack overflows, since every frame retains its own copy of the window.
## Reproduction
Save as
`core/src/test/java/org/apache/stormcrawler/util/CharsetIdentificationUnterminatedMetaTest.java`:
```java
/*
* 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.stormcrawler.util;
import java.nio.charset.StandardCharsets;
import java.util.Arrays;
import java.util.concurrent.atomic.AtomicReference;
import org.apache.stormcrawler.Metadata;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;
/**
* A document with an unterminated meta charset declaration must not take
the JVM down: charset
* detection is expected to give up and let the caller fall back to a
default.
*/
class CharsetIdentificationUnterminatedMetaTest {
/** detect.charset.maxlength as set in crawler-default.yaml. */
private static final int MAXLENGTH = 10000;
private static byte[] unterminatedMetaCharset(int size) {
byte[] content = new byte[size];
Arrays.fill(content, (byte) 'A');
byte[] prefix = "<meta charset=\"".getBytes(StandardCharsets.UTF_8);
System.arraycopy(prefix, 0, content, 0, prefix.length);
return content;
}
private static Throwable runOnStack(byte[] content, long stackSize)
throws InterruptedException {
AtomicReference<Throwable> thrown = new AtomicReference<>();
AtomicReference<String> 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: <null> but was:
<java.lang.StackOverflowError>
```
## 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.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
