Parse markup attributes and raw text the way HTML does by dashorst · Pull Request #1876 · apache/wicket

dashorst · 2026-11-08T23:05:43Z

XmlPullParser now reads attributes the way the HTML tokenizer does, so <a href=foo/bar.html> keeps href="foo/bar.html" instead of turning into href="foo" bar.html="" (#1864). Auditing the parser against the HTML specification, with fuzzing and mutation testing, turned up twelve more defects in the same code: attribute names and quoted values, the quote scanning that finds the end of a tag, raw-text detection for <script> and <style>, CDATA, and two crashes. This PR fixes all of them and documents the parser's contract in its class Javadoc, which until now was one line.

Fixes #1864

Commits

Each commit can be read on its own. The first adds every test, red where the parser was wrong; each later commit fixes one aspect and turns its own tests green. With this order, each test can be seen to fail before its fix.

Document XmlPullParser and test it against the HTML tokenizer. Class Javadoc, plus XmlPullParserAttributeTest, XmlPullParserRawTextTest, and new tests in XmlPullParserTest and FullyBufferedReaderTest.
Find the end of a tag without backslash escapes or recursion. FullyBufferedReader.findOutOfQuotes: title="Next >>" was rejected, about ten thousand > in one quoted value overflowed the stack, and \" counted as an escaped quote although HTML, XML and the attribute parser have no escapes.
Read attribute names and unquoted values the way HTML does (Unquoted attribute values containing / or : are cut off by the markup parser #1864). Unquoted values run to whitespace. Names such as (click), [value], #ref and on:click|once are kept as written. Text that doesn't match is no longer skipped silently, and quoted values are no longer trimmed.
Decide on raw text for script and style by element name and type. Fixes <style> without attributes (off by one), custom elements like <script-loader> swallowing their content, type sniffed by substring (data-type=, unquoted, empty, application/javascript, JSON), raw text ending at </scripts, and a StringIndexOutOfBoundsException on <script type=>.
Accept quotes in CDATA sections and reject </> with a ParseException.

Where to look

Core: wicket-core/.../markup/parser/XmlPullParser.java. Read parseAttributes and isScriptOrDataBlock alongside the class Javadoc, which states the contract they implement. Then wicket-util/.../io/FullyBufferedReader.java#findOutOfQuotes.

Tests (can be skimmed): the four test classes. The expected values come from the HTML specification, not from what the code returned.

MetaPattern.STRING and VariableAssignmentParser are deliberately unchanged. ValueMap and CommaSeparatedVariableParser use them too, and they are public API. The parser now has its own attribute tokenizer instead.

Behaviour changes

No public API changes. Templates that parsed before may now parse differently. migration-to-12.md describes these changes for users:

Unquoted values containing /, :, ? and the like, and framework attribute names (Angular, Vue, Alpine, Svelte, htmx), are now rendered as written in component tags and in tags changed by filters.
Whitespace inside quotes is kept, in Wicket's own attributes too: wicket:id="label " now refers to the id label and fails with "Unable to find component with id" when the page renders. class=" a " used to become class="a".
Text in a tag that isn't an attribute follows HTML recovery: <tag a="1" "junk"> gains an attribute named "junk" instead of junk. A duplicate attribute is still a ParseException, as before.
\" inside a quoted value now ends the value. Before, the bracket finder treated it as an escape (WICKET-3773), but the attribute parser didn't.
A <script> with an empty type, application/javascript or a JSON type is now raw text. It used to be parsed as a template, so a Wicket component inside one is no longer found. Other non-JavaScript types are still templates (WICKET-5288).
A custom element whose name starts with script or style is now parsed as markup instead of raw text.

Testing

mvn clean verify -Pjs-test is green.
Crash fuzzing: 1.5 million random markup strings, with only a ParseException allowed. It found </> and <script type=>. A seeded 20,000-input version is in XmlPullParserTest.anyMarkupEitherParsesOrIsRejectedWithParseException.
Differential fuzzing against jsoup's HTML5 tokenizer, run outside the repository because jsoup is not a dependency: 400,000 generated attribute lists, zero differences after the fix. The only remaining difference was numeric character references without a closing ; (&#9), which Strings.unescapeMarkup does not decode. That is entity decoding rather than tokenizing, so it is left for a separate issue.
PIT on XmlPullParser and FullyBufferedReader with these tests kills 192 of 210 mutants. The survivors are equivalent mutants, getString() of a conditional comment (which nothing reads), a > check in skipUntil that duplicates what next() does, and helpers this PR doesn't touch.

Not changed: for a plain text token, getString() returns text from the position marker rather than the token alone. Nothing in Wicket calls it.

Backporting to wicket-11.x / wicket-10.x is a separate decision. The behaviour changes above argue for master only.

🤖 Generated with Claude Code

XmlPullParser had a one-line class comment, and nothing described what a tag, an attribute or a raw text body may contain. The class documentation now states the contract: where a tag ends, how attribute names and values are read (following the attribute states of the HTML tokenizer), which script and style bodies are raw text, how comments and CDATA end, and that the parser reads trusted templates and is no sanitizer. The new tests assert that contract, with expected values taken from the HTML specification, and fail where the parser departs from it: - XmlPullParserAttributeTest: unquoted values are cut off at the first character outside [\w.-] (GitHub issue #1864); names such as (click), [value], #ref and on:click|once are mangled; text that is not an attribute is dropped silently; quoted values are trimmed; and quoted values with two adjacent '>', a trailing backslash or very many '>' are rejected or overflow the stack. - XmlPullParserRawTextTest: <style> without attributes is not raw text; <script-loader> and <styled-box> swallow their content; the script type is sniffed by substring, so data-type=, an unquoted or empty type, application/javascript and JSON types are parsed as templates; raw text ends at </scripts; a CDATA section containing a quote is rejected. - XmlPullParserTest: </> and <script type=> throw StringIndexOutOfBoundsException instead of ParseException. Both were found by fuzzing, and anyMarkupEitherParsesOrIsRejectedWithParseException keeps that invariant over 20,000 seeded random inputs. The other new tests close gaps found by mutation testing. - FullyBufferedReaderTest: nestedQuotes asserted that a backslash escapes a quote, which HTML and XML do not have. It now asserts the opposite, next to tests for adjacent and very many '>' inside quotes. These tests fail until the commits that follow fix the parser. Generated-by: Claude Code (Claude Opus 5.5)

FullyBufferedReader.findOutOfQuotes, which XmlPullParser uses to find the '>' that ends a tag, had three defects: - It treated a backslash before a quote as an escape. HTML and XML have no such escape, and the attribute parser did not honour it either, so <span title="C:\dir\"> was rejected, and in <a href='b \'" > a' x=1> the two disagreed on where the value ends. - After a '>' inside quotes it resumed the search one character too far, so title="Next >>" was rejected with "Opening/closing quote not found". - It recursed once for every '>' inside quotes, so a value with some ten thousand of them overflowed the stack. It now scans iteratively, and a quoted section ends at the next occurrence of its opening quote. Markup that relied on \" inside a quoted attribute value now has the value end at that quote. The WICKET-3773 test that asserted the escape is rewritten accordingly in the previous commit. Generated-by: Claude Code (Claude Opus 5.5)

XmlPullParser matched attributes with VariableAssignmentParser. Its unquoted value pattern is [\w\-.]+, its name pattern rejects characters such as ( [ # * and |, and the parser looked for the next match with find(), so whatever did not match was skipped silently. <a href=foo/bar.html> came out as href="foo" bar.html="", <button (click)="go()"> as click="" go="", and a component tag, a tag changed by a markup filter or the page's <html> tag was rendered with those mangled attributes. Attributes are now read the way the attribute states of the HTML tokenizer read them: a name runs up to whitespace, '/' or '=', an unquoted value up to whitespace, and parse errors recover as in HTML (a missing space between attributes is accepted, a stray '/' is ignored, "junk" becomes an attribute of that name). Quoted values are no longer trimmed. A duplicate attribute is still rejected, and so is a quoted value whose closing quote lies outside the tag. MetaPattern.STRING and VariableAssignmentParser are left unchanged: ValueMap and CommaSeparatedVariableParser use them too, and they are public API. An application whose markup has unquoted values containing '/' or ':', or attribute names in Angular, Vue, Alpine or Svelte syntax, now gets those attributes rendered as written. One that relied on whitespace being trimmed from quoted values now sees that whitespace in the value. GitHub issue #1864 Generated-by: Claude Code (Claude Opus 5.5)

XmlPullParser decided whether an element's body is raw text before it parsed the tag, by looking at the tag text: - startsWith("script") and startsWith("style") also matched custom elements such as <script-loader> and <styled-box>, whose content, Wicket components included, was swallowed up to the next </script or </style. - An off-by-one length check meant <style> without attributes was not raw text, so a '<' in its CSS was parsed as a tag. - The script type was found with indexOf("type="), which also hit data-type= and a type= inside a URL. It skipped one character to drop the quote, which missed an unquoted type=text/javascript, and it recognised only text/javascript, module and importmap, so an empty type, application/javascript and JSON data blocks were parsed as templates and "if (a<b)" became a <b> tag. <script type=> threw StringIndexOutOfBoundsException. - The raw text ended at the first </script, </scripts included. The decision is now made on the parsed tag. An element named exactly script or style, without a namespace prefix, has a raw text body; for a script only when its type is one the HTML specification treats as a script (absent, empty, a JavaScript MIME type, module, importmap, speculationrules) or is a JSON type. Other script types remain templates (WICKET-5288). The raw text ends at </script or </style followed by whitespace, '/' or '>'. A script with an empty type, application/javascript or a JSON type used to be parsed as a template and is now passed through untouched, so a Wicket component inside one is no longer found. Generated-by: Claude Code (Claude Opus 5.5)

The end of a CDATA section was searched with a quote-aware scan, so <![CDATA[don't]]> was rejected with "No matching close bracket". A CDATA section now ends at the next ]]>, as XML defines it. The quote-aware helper had no other caller and is removed. </> was read as an open-close tag with an empty name and threw StringIndexOutOfBoundsException. It is now rejected with a ParseException, like other malformed tags. Fuzzing XmlPullParser found this crash and the one for <script type=> fixed in the previous commit. Generated-by: Claude Code (Claude Opus 5.5)

codecov-commenter · 2026-11-08T23:14:09Z

Codecov Report

❌ Patch coverage is 94.73684% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 62.78%. Comparing base (8902c1a) to head (8af85af).

Additional details and impacted files

@@             Coverage Diff              @@
##             master    #1876      +/-   ##
============================================
+ Coverage     62.71%   62.78%   +0.07%     
- Complexity    11512    11549      +37     
============================================
  Files          1254     1254              
  Lines         48665    48676      +11     
  Branches       6845     6848       +3     
============================================
+ Hits          30521    30563      +42     
+ Misses        15468    15445      -23     
+ Partials       2676     2668       -8

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

papegaaij · 2026-11-10T16:50:39Z

 			}

-			return true;
+			if (null != tag.getAttributes().put(key, Strings.unescapeMarkup(value).toString()))


Keeping whitespace inside quotes is right, but it has a consequence an upgrading application will notice that the description doesn't name: Wicket's own attributes aren't trimmed any more either. <span wicket:id="label "> used to render and now fails at render time with Unable to find component with id 'label ', and <wicket:message key="foo "> looks up foo . That should never have worked, but templates that relied on it exist, and they only fail when the page is opened.

Could the behaviour changes from the description go into migration-to-12.md as a section, with this case spelled out? For example: "Whitespace inside quotes is now part of an attribute value, including Wicket's own attributes. wicket:id="label " refers to a component with the id label , so a page whose markup has stray spaces in wicket:id, wicket:message or wicket:for fails with 'Unable to find component with id'. Remove the spaces from the markup."

Done in dfa9464: migration-to-12.md has a section "Markup is parsed the way HTML parses it" that opens with this case (wicket:id, wicket:message, wicket:for, failing only at render time) and what to do, then lists the other behaviour changes. I merged master in first, because the branch predated the guide.

papegaaij · 2026-11-10T16:50:39Z

+		});
+	}
+
+	@Test


The fuzz test guards against crashes but not against hangs: if a later change leaves one of the parser's loops without progress for some input, the build hangs instead of failing. A generous timeout turns that into a quick, ordinary failure (it needs import org.junit.jupiter.api.Timeout;):

Suggested change

@Test

@Test

@Timeout(30)

Added @Timeout(30) in 8af85af. The test runs in about 150 ms, so 30 seconds only trips on a real hang.

…uzz-tests

The parser changes for GitHub issue #1864 change what some templates render, and one of them fails only when a page is opened: Wicket's own attributes are no longer trimmed either, so wicket:id="label " looks for a component with the id "label ". The guide now names that case and what to do about it, and lists the other behaviour changes from the pull request. Generated-by: Claude Code (Claude Opus 5.5)

anyMarkupEitherParsesOrIsRejectedWithParseException catches crashes, but if a later change leaves one of the parser's loops without progress for some input, the build would hang. A generous timeout turns that into an ordinary test failure. Generated-by: Claude Code (Claude Opus 5.5)

dashorst added 5 commits November 8, 2026 23:57

papegaaij reviewed Nov 10, 2026

View reviewed changes

dashorst added 3 commits November 10, 2026 17:52

Merge remote-tracking branch 'origin/master' into t3/gh-1864-parser-f…

647b967

…uzz-tests

Navigation Menu

Repository navigation

Parse markup attributes and raw text the way HTML does - #1876

Parse markup attributes and raw text the way HTML does#1876
dashorst wants to merge 8 commits into
masterfrom
t3/gh-1864-parser-fuzz-tests

dashorst commented Nov 8, 2026 •

edited

Loading

Uh oh!

codecov-commenter commented Nov 8, 2026 •

edited

Loading

Uh oh!

papegaaij Nov 10, 2026

Uh oh!

dashorst Nov 10, 2026

Uh oh!

papegaaij Nov 10, 2026

Uh oh!

dashorst Nov 10, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

+              		});
+              	}
+              	@Test

Conversation

dashorst commented Nov 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Commits

Where to look

Behaviour changes

Testing

Uh oh!

codecov-commenter commented Nov 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

papegaaij Nov 10, 2026

Choose a reason for hiding this comment

Uh oh!

dashorst Nov 10, 2026

Choose a reason for hiding this comment

Uh oh!

papegaaij Nov 10, 2026

Choose a reason for hiding this comment

Uh oh!

dashorst Nov 10, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

dashorst commented Nov 8, 2026 •

edited

Loading

codecov-commenter commented Nov 8, 2026 •

edited

Loading