#2092 Bound the meta charset look-ahead and catch charset detection failures in JSoupParserBolt - #2157
Open
dpol1 wants to merge 2 commits into
Open
#2092 Bound the meta charset look-ahead and catch charset detection failures in JSoupParserBolt#2157dpol1 wants to merge 2 commits into
dpol1 wants to merge 2 commits into
Conversation
…dow (#2092) getCharsetFromMeta retried itself with a window ten bytes larger, decoding the whole window again each time, until the closing quote of a <meta charset=" declaration was found. A document which opens the declaration and never closes it recursed once per ten bytes of body and overflowed the stack at a few hundred KB. The window is now extended once, by 64 bytes, room for any registered charset name and its closing quote; a declaration still unterminated after that is treated as broken content, as before. Tests cover the unterminated declaration at 20 KB and 400 KB on a 1 MB stack, with full-content detection, and a declaration cut by the window that is still read (#870). Signed-off-by: Davide Polato <dpol1@apache.org>
…erBolt (#2092) Charset detection ran before the try block whose catch (Throwable) routes parse failures through handleException, so an error thrown there escaped execute() and took the worker down; the URL stayed scheduled and the replay took the restarted worker down again. The detection now runs inside that try, so a failure ends as a Status.ERROR for that URL like any other parse failure. A test injects an Error into the detection and checks the status tuple, the ack and that no document is emitted. Signed-off-by: Davide Polato <dpol1@apache.org>
dpol1
force-pushed
the
meta-charset-overflow
branch
from
September 15, 2026 07:45
bca507f to
701d687
Compare
rzo1
reviewed
Sep 16, 2026
rzo1
left a comment
rzo1
left a comment
Contributor
There was a problem hiding this comment.
The new tests don't check the 64 byte limit. In metaCharsetCutByTheDetectionWindowIsStillRead the page ends 38 bytes after the cut, so Math.min(buffer.length, len + META_CHARSET_LOOKAHEAD) is always buffer.length. The closing quote is 10 bytes after the cut, so the old +10 retry on main finds it too. The test passes with a lookahead of 11 or without any limit.
Please add more than 64 bytes after the tag, plus a case where the closing quote comes after len + 64 and the meta charset is not used.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #2092.
A page that opens a
<meta charset="declaration and never closes it madegetCharsetFromMetaretry with a window ten bytes larger, once per ten bytes of body, until the stack overflowed. The detection ran outside the parser bolt's try block, so the error took the worker down, and the replayed tuple took the restarted worker down again.Two changes:
CharsetIdentification: when the declaration is cut by the detection window, the window is extended once, by 64 bytes, room for any registered charset name. A declaration still open after that is broken content, as before.JSoupParserBolt: charset detection now runs inside the existing try, so a failure there is a parse error for that URL.Tests: the open declaration at 20 KB and 400 KB on a 1 MB stack, the same with full-content detection, a declaration cut by the window that is still read (#870), and an error injected into the detection checked against the status stream.
For all changes
Is there a issue associated with this PR? Is it referenced in the commit message?
Does your PR title start with
#XXXXwhereXXXXis the issue number you are trying to resolve?Has your PR been rebased against the latest commit within the target branch (typically main)?
Is your initial contribution a single, squashed commit? (two commits, one per change)
Is the code properly formatted with
mvn git-code-format:format-code -Dgcf.globPattern="**/*" -Dskip.format.code=false?For code changes
mvn clean verify?