From b93f7b674a1014bfc6edcec4468655009051eb39 Mon Sep 17 00:00:00 2001 From: Jim Manico Date: Thu, 10 Sep 2026 20:24:35 -1000 Subject: [PATCH 1/5] Keep a table open across content a browser would put in front of it A browser puts content that cannot go inside a table, such as a div between its rows, in front of the table and keeps the table open, so that the next row pops the content and carries on in the same table. The tag balancer closed the table to make room for the content, and when the row came it opened a new table inside the content, which then stayed open until its parent closed and swallowed every row and everything after the table (#342). The balancer now keeps the table, and any row group and row the content was pushed out of, on its stack while closing them in the output, marked as pushed out. They still bound end tags for the elements below them, as in a browser, and when a part of the table arrives they take it back: the pushed-out content is closed, the entries that cannot hold the part are dropped, and the rest are written again as a new table for the part to go in. A table arriving instead pops the open one, as in a browser. The output cannot put anything in front of a tag already written, so the table is written twice, once empty and once with the later rows, and text pushed out of a table follows it rather than preceding it. A link pushed out of a table and closed when the table resumes is not written again around another link or where one is open, since nested links do not survive a browser's parse; and a link that ends the link open before it does so through a table between them rather than pushing the table out. Fixes #342 Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_014Ydng7gBm6Ax5zfwt4vZip --- change_log.md | 14 ++ .../TagBalancingHtmlStreamEventReceiver.java | 165 +++++++++++++- .../org/owasp/html/HtmlSanitizerTest.java | 210 ++++++++++++++++++ .../TagBalancingHtmlStreamRendererTest.java | 89 ++++++++ 4 files changed, 472 insertions(+), 6 deletions(-) diff --git a/change_log.md b/change_log.md index 2561171e..9770f0cc 100644 --- a/change_log.md +++ b/change_log.md @@ -2,6 +2,20 @@ Most recent at top. * Next release + * Content that cannot go inside a table, such as a `div` between its + rows, no longer stays open until the end of the document, taking the + rows and everything after the table with it (#342). A browser puts + such content in front of the table and keeps the table open, so that + the next row pops the content and carries on in the same table. The + tag balancer now keeps the table, and any row group and row, on its + stack while closing them in the output, closes the content when a part + of the table arrives, and writes the table again for that part. The + output cannot put anything in front of a tag already written, so the + table is written twice, once empty and once with the later rows, and + text pushed out of a table follows it rather than preceding it as in a + browser; a browser reads the rest as it reads the input. A link + pushed out of a table is not written again around or inside another + link. * What `HtmlStreamRenderer` leaves out now reaches an `HtmlChangeListener` as well as the renderer's bad-HTML handler: a start tag whose name is not one HTML allows, which an `ElementPolicy` can produce by renaming, diff --git a/owasp-java-html-sanitizer/src/main/java/org/owasp/html/TagBalancingHtmlStreamEventReceiver.java b/owasp-java-html-sanitizer/src/main/java/org/owasp/html/TagBalancingHtmlStreamEventReceiver.java index 4b948eae..de15d307 100644 --- a/owasp-java-html-sanitizer/src/main/java/org/owasp/html/TagBalancingHtmlStreamEventReceiver.java +++ b/owasp-java-html-sanitizer/src/main/java/org/owasp/html/TagBalancingHtmlStreamEventReceiver.java @@ -56,12 +56,55 @@ public class TagBalancingHtmlStreamEventReceiver */ private final IntVector outputElements = new IntVector(); private final IntVector toResumeInReverse = new IntVector(); + /** + * Bit {@code i} is set while the element at {@code i} of + * {@link #openElements} is closed in the output but still open here. + *

+ * A browser keeps a table, and any row group and row open in it, on its + * stack when content arrives that cannot go inside a table: it puts the + * content in front of the table instead, foster parenting, and later table + * content pops that content and carries on in the same table. The output + * cannot put anything in front of a tag already written, so the table is + * closed there and the content written after it, and the table is written + * again, as a new table, when its content resumes (#342). Meanwhile its + * entries stay here, so that table content finds them and comes back to + * them, and so that they bound end tags for elements below them as they do + * in a browser. Such entries are contiguous, from a {@code table} up, and + * are the only pushed-out entries below the content pushed out of them. + */ + private final BitSet pushedOut = new BitSet(); private static final HtmlElementTables METADATA = HtmlElementTables.get(); private static final int UNRECOGNIZED_TAG = METADATA.indexForName(HtmlElementNames.CUSTOM_ELEMENT_NAME); private static final int A_TAG = METADATA.indexForName("a"); private static final int BODY_TAG = METADATA.indexForName("body"); + private static final int TABLE_TAG = METADATA.indexForName("table"); private static final int NO_OUTPUT_ELEMENT = -1; + /** + * The elements a browser keeps open, and later clears its stack back to, + * when it foster-parents content that arrives inside them: a table and its + * row groups and rows. Not the cell, caption or template, inside which + * content nests normally. + */ + private static final BitSet TABLE_CONTEXT = new BitSet(); + /** + * The elements whose arrival, in a browser, ends foster-parented content + * and returns to the table: its parts, which clear the stack back to the + * table, and a table itself, which pops the open table and takes its + * place. + */ + private static final BitSet TABLE_PARTS = new BitSet(); + static { + for (String name : new String[] { "table", "tbody", "tfoot", "thead", "tr" }) { + TABLE_CONTEXT.set(METADATA.indexForName(name)); + } + for (String name : new String[] { + "caption", "col", "colgroup", "table", "tbody", "td", "tfoot", + "th", "thead", "tr", + }) { + TABLE_PARTS.set(METADATA.indexForName(name)); + } + } /** * Elements on entering which a browser puts a marker on its list of * active formatting elements, so that an {@code a} opened inside one of @@ -184,12 +227,14 @@ public void openDocument() { public void closeDocument() { for (int i = Math.min(nestingLimit, openElements.size()); --i >= 0;) { + if (pushedOut.get(i)) { continue; } // Already closed in the output. int elIndex = openElements.get(i); String elname = METADATA.canonNameForIndex(elIndex); underlying.closeTag(elname); } openElements.clear(); outputElements.clear(); + pushedOut.clear(); toResumeInReverse.clear(); underlying.closeDocument(); } @@ -263,6 +308,11 @@ private int outputElementIndexForLastOpenTag(int inputElementIndex) { } private void prepareForContent(int elIndex) { + if (!pushedOut.isEmpty() + && elIndex != HtmlElementTables.TEXT_NODE + && TABLE_PARTS.get(elIndex)) { + returnToPushedOutTable(elIndex); + } int nOpen = openElements.size(); { int top = nOpen != 0 ? openElements.get(nOpen - 1) : BODY_TAG; @@ -299,16 +349,30 @@ private void prepareForContent(int elIndex) { int top = openElements.get(nOpen - 1); // Close all the elements that cannot contain the content to open. while (true) { + // A link ends the link open before it, wherever that is: nested + // links do not survive a browser's parse, so a table between them + // cannot stay open either. + boolean linkEndsLink = + elIndex == A_TAG && hasOpenLinkInFormattingScope(); boolean canContain = canContain(elIndex, top, nOpen - 1) - && !(elIndex == A_TAG && hasOpenLinkInFormattingScope()); + && !linkEndsLink; if (canContain) { break; } - if (openElements.size() < nestingLimit) { + if (!linkEndsLink + && TABLE_CONTEXT.get(top) && isFosterParented(elIndex)) { + // A browser puts the content in front of the table and keeps the + // table open. Close the table in the output, keep it here, and + // open the content beside it. + pushOutTable(nOpen - 1); + break; + } + if (openElements.size() < nestingLimit && !pushedOut.get(nOpen - 1)) { underlying.closeTag(METADATA.canonNameForIndex(top)); } openElements.remove(--nOpen); outputElements.remove(nOpen); + pushedOut.clear(nOpen); if (METADATA.resumable(top) && top != elIndex) { toResumeInReverse.add(top); } @@ -320,11 +384,16 @@ private void prepareForContent(int elIndex) { while (!toResumeInReverse.isEmpty()) { int toResume = toResumeInReverse.getLast(); // If toResume can contain elInfo AND the top of the stack can contain - // toResume, then we push toResume. + // toResume, then we push toResume. A link is not resumed around + // another link, or where one is open: a browser ends a link when the + // next begins, and nested links do not survive a browser's parse, so + // the output would not read back as written. nOpen = openElements.size(); if ((nOpen == 0 || canContain(toResume, openElements.get(nOpen - 1), nOpen)) - && canContain(elIndex, toResume, nOpen)) { + && canContain(elIndex, toResume, nOpen) + && !(toResume == A_TAG + && (elIndex == A_TAG || hasOpenLinkInFormattingScope()))) { toResumeInReverse.removeLast(); int outputElementIndex = NO_OUTPUT_ELEMENT; if (openElements.size() < nestingLimit) { @@ -341,6 +410,88 @@ && canContain(elIndex, toResume, nOpen)) { } } + /** + * True if a browser puts content of this kind that arrives inside a table, + * outside a cell or caption, in front of the table rather than in it: text, + * and any element that is not one of a table's own parts. + */ + private static boolean isFosterParented(int elIndex) { + return elIndex == HtmlElementTables.TEXT_NODE || !TABLE_PARTS.get(elIndex); + } + + /** + * Closes in the output, innermost first, the row, row group and table that + * the top of the stack is in, and marks them pushed out, keeping them here. + * Entries already pushed out, by earlier content beside the same table, are + * left as they are. + */ + private void pushOutTable(int topIndex) { + for (int i = topIndex; i >= 0; --i) { + int elIndex = openElements.get(i); + if (!TABLE_CONTEXT.get(elIndex)) { break; } + if (!pushedOut.get(i)) { + if (i < nestingLimit) { + underlying.closeTag(METADATA.canonNameForIndex(elIndex)); + } + pushedOut.set(i); + } + if (elIndex == TABLE_TAG) { break; } + } + } + + /** + * Handles a table part, or a table, arriving while a table is pushed out, + * as a browser does: closes the content that was put in front of the + * nearest pushed-out table, pops the pushed-out entries that cannot hold + * the part, even by implying elements between, which for a table is all of + * them, and writes the rest again as a new table for the part to go in. + */ + private void returnToPushedOutTable(int elIndex) { + int top = pushedOut.length() - 1; // The nearest pushed-out entry. + for (int i = openElements.size(); --i > top;) { + int unclosed = openElements.remove(i); + outputElements.remove(i); + if (i < nestingLimit) { + underlying.closeTag(METADATA.canonNameForIndex(unclosed)); + } + if (METADATA.resumable(unclosed)) { + toResumeInReverse.add(unclosed); + } + } + while (top >= 0 && pushedOut.get(top)) { + int entry = openElements.get(top); + if (canContain(elIndex, entry, top) + || METADATA.impliedElements(entry, elIndex).length != 0) { + break; + } + openElements.remove(top); + outputElements.remove(top); + pushedOut.clear(top); + --top; + } + if (top < 0 || !pushedOut.get(top)) { return; } + int start = top; + while (start > 0 && pushedOut.get(start - 1)) { --start; } + // Pop the run and push it back, outermost first, opening each again. + int n = top - start + 1; + int[] run = new int[n]; + for (int i = n; --i >= 0;) { + run[i] = openElements.remove(start + i); + outputElements.remove(start + i); + pushedOut.clear(start + i); + } + for (int i = 0; i < n; ++i) { + int outputElementIndex = NO_OUTPUT_ELEMENT; + if (openElements.size() < nestingLimit) { + underlying.openTag( + METADATA.canonNameForIndex(run[i]), new ArrayList<>()); + outputElementIndex = outputElementIndexForLastOpenTag(run[i]); + } + openElements.add(run[i]); + outputElements.add(outputElementIndex); + } + } + private static final BitSet TRANSPARENT = new BitSet(); static { for (String transparentElement @@ -459,16 +610,18 @@ public void closeTag(String elementName) { while (--last > index) { int unclosed = openElements.remove(last); outputElements.remove(last); - if (last + 1 < nestingLimit) { + if (last + 1 < nestingLimit && !pushedOut.get(last)) { underlying.closeTag(METADATA.canonNameForIndex(unclosed)); } + pushedOut.clear(last); if (METADATA.resumable(unclosed)) { toResumeInReverse.add(unclosed); } } - if (openElements.size() < nestingLimit) { + if (openElements.size() < nestingLimit && !pushedOut.get(index)) { underlying.closeTag(METADATA.canonNameForIndex(elIndex)); } + pushedOut.clear(index); openElements.remove(index); outputElements.remove(index); } diff --git a/owasp-java-html-sanitizer/src/test/java/org/owasp/html/HtmlSanitizerTest.java b/owasp-java-html-sanitizer/src/test/java/org/owasp/html/HtmlSanitizerTest.java index f816608c..b1d44ba6 100644 --- a/owasp-java-html-sanitizer/src/test/java/org/owasp/html/HtmlSanitizerTest.java +++ b/owasp-java-html-sanitizer/src/test/java/org/owasp/html/HtmlSanitizerTest.java @@ -1779,6 +1779,216 @@ void testRenamedElementsDetermineFormattingMarkerScope() { assertEquals(renamedInto, intoMarker.sanitize(renamedInto)); } + /** + * A browser puts content that cannot go inside a table in front of the + * table and keeps the table open, so that the next row pops the content + * and carries on in the same table (#342). The output cannot put anything + * in front of a tag already written, so the table is closed there, the + * content written after it, and the table written again for its rows. + * The content used to stay open until the end of the document, taking the + * rows and everything after the table with it. A browser now reads the + * output as it reads the input, but for the empty table in front. + */ + @Test + void testContentPushedOutOfATableIsClosedWhenTheTableResumes() + throws Exception { + PolicyFactory p = tablePolicy(); + String input = "
x
y
tail"; + String out = p.sanitize(input); + + assertEquals( + "

x
" + + "
y
tail", + out); + assertEquals(out, p.sanitize(out)); + assertEquals(parseAsBrowser("
" + input), parseAsBrowser(out)); + } + + /** + * The shape reported in #342: the table had rows before the content and + * attributes of its own. The rows before stay in the first table, and the + * table written again for the rows after has no attributes, as a formatting + * element written again after being closed has none. + */ + @Test + void testRowsAfterPushedOutContentContinueInANewTable() throws Exception { + PolicyFactory p = tablePolicy(); + String input = "" + + "
x
a
y
tail"; + String out = p.sanitize(input); + + assertEquals( + "
a
" + + "
x
" + + "
y
tail", + out); + assertEquals(out, p.sanitize(out)); + } + + /** + * Content pushed out of a table nests as usual until a part of the table + * arrives, which closes all of it. End tags for that content arriving + * later find nothing to close, as in a browser, and the text after them + * is pushed out as well. A browser puts that text in front of the table; + * the output, written in order, can only put it after. + */ + @Test + void testPushedOutContentNestsUntilATablePartArrives() { + PolicyFactory p = tablePolicy(); + String input = "

xs

tu" + + "
y
v"; + String out = p.sanitize(input); + + assertEquals( + "

xs

" + + "
y
tuv", + out); + assertEquals(out, p.sanitize(out)); + } + + /** + * A browser keeps the table open below the content it pushed out, so an + * end tag for an element enclosing the table is ignored meanwhile, and the + * text after it lands in the pushed-out content. The balancer used to + * close the table and let the end tag through. + */ + @Test + void testEndTagBelowAPushedOutTableIsIgnoredWhileItIsOpen() + throws Exception { + PolicyFactory p = tablePolicy(); + String input = "
xy
z"; + String out = p.sanitize(input); + + assertEquals("
xyz
", out); + assertEquals(out, p.sanitize(out)); + assertEquals( + parseAsBrowser("
xyz
"), + parseAsBrowser(out)); + } + + /** + * The table's own end tag ends the content pushed out of it. A browser + * has the content in front of the table and the text after both; the + * output has the same nodes with the table first. + */ + @Test + void testTableEndTagEndsPushedOutContent() { + PolicyFactory p = tablePolicy(); + String out = p.sanitize("
x
tail"); + + assertEquals("
x
tail", out); + assertEquals(out, p.sanitize(out)); + } + + /** + * A table arriving inside pushed-out content pops the open table in a + * browser and takes its place, rather than nesting in the content; and + * with tables pushed out at two levels, a row returns to the nearest. + */ + @Test + void testTableInsidePushedOutContentReplacesTheOpenTable() + throws Exception { + PolicyFactory p = tablePolicy(); + String input = "
x
y
tail"; + String out = p.sanitize(input); + assertEquals( + "
x
" + + "
y
tail", + out); + assertEquals(out, p.sanitize(out)); + + String nested = "
x
y
z
" + + "
wv"; + out = p.sanitize(nested); + assertEquals( + "
x
y" + + "
z
wv", + out); + assertEquals(out, p.sanitize(out)); + assertEquals(parseAsBrowser("
" + nested), parseAsBrowser(out)); + } + + /** Every part of a table brings the pushed-out table back. */ + @Test + void testEveryTablePartReturnsToThePushedOutTable() throws Exception { + PolicyFactory p = tablePolicy(); + String[][] cases = { + { "c", "c" }, + { "h", "h" }, + { "y", "y" }, + { "y", "y" }, + { "y", "y" }, + { "h", "h" }, + }; + for (String[] c : cases) { + String input = "
x" + c[0] + "
"; + String out = p.sanitize(input); + assertEquals( + "
x
" + c[1] + "
", out, c[0]); + assertEquals(out, p.sanitize(out), c[0]); + assertEquals( + parseAsBrowser("
" + input), parseAsBrowser(out), c[0]); + } + } + + /** + * A link pushed out of a table is closed when the table resumes, and is + * not written again around another link or inside one: nested links do + * not survive a browser's parse. Text is pushed out likewise, and needs + * nothing closed. + */ + @Test + void testPushedOutLinkIsNotResumedAroundAnotherLink() throws Exception { + PolicyFactory p = new HtmlPolicyBuilder() + .allowElements("table", "tbody", "tr", "td", "a") + .allowAttributes("href").onElements("a") + .allowWithoutAttributes("a") + .toFactory(); + String out = p.sanitize( + "x
yz
w"); + + assertEquals( + "
x" + + "" + + "
yz
w", + out); + assertEquals(out, p.sanitize(out)); + assertEquals( + "
x
y
", + p.sanitize("x
y
")); + } + + /** Pushed-out content is bounded by the nesting limit like any other. */ + @Test + void testPushedOutContentRespectsTheNestingLimit() { + PolicyFactory p = tablePolicy(); + StringBuilder sb = new StringBuilder(""); + for (int i = 0; i < 300; ++i) { sb.append("
"); } + sb.append("
y
z"); + String out = p.sanitize(sb.toString()); + assertTrue( + out.endsWith("
y
z"), + out.substring(out.length() - 80)); + assertEquals(out, p.sanitize(out)); + + sb = new StringBuilder(); + for (int i = 0; i < 300; ++i) { sb.append("
"); } + sb.append("
y
z"); + out = p.sanitize(sb.toString()); + assertTrue(out.startsWith("
"), out); + assertEquals(out, p.sanitize(out)); + } + + private static PolicyFactory tablePolicy() { + return new HtmlPolicyBuilder() + .allowElements( + "table", "caption", "thead", "tbody", "tfoot", "tr", "td", "th", + "div", "p", "span") + .allowAttributes("class").onElements("table", "div") + .allowWithoutAttributes("span") + .toFactory(); + } + /** The tree a browser builds from html, one node per line. */ private static String parseAsBrowser(String html) throws Exception { Node fragment = new HtmlDocumentBuilder().parseFragment( diff --git a/owasp-java-html-sanitizer/src/test/java/org/owasp/html/TagBalancingHtmlStreamRendererTest.java b/owasp-java-html-sanitizer/src/test/java/org/owasp/html/TagBalancingHtmlStreamRendererTest.java index 88302e7d..0560035f 100644 --- a/owasp-java-html-sanitizer/src/test/java/org/owasp/html/TagBalancingHtmlStreamRendererTest.java +++ b/owasp-java-html-sanitizer/src/test/java/org/owasp/html/TagBalancingHtmlStreamRendererTest.java @@ -246,6 +246,95 @@ void testTableNesting() { htmlOutputBuffer.toString()); } + /** + * A browser puts content that cannot go inside a table in front of the + * table and keeps the table open, so that the next row pops the content + * and carries on in the same table (#342). The output cannot put anything + * in front of a tag already written, so the table is closed there and + * written again for the row; the content is closed when the row comes, + * where it used to stay open and swallow the row and everything after. + */ + @Test + void testContentPushedOutOfATableIsClosedWhenTheTableResumes() { + balancer.openDocument(); + balancer.openTag("table", j8().listOf()); + balancer.openTag("div", j8().listOf()); + balancer.text("x"); + balancer.openTag("tr", j8().listOf()); + balancer.openTag("td", j8().listOf()); + balancer.text("y"); + balancer.closeTag("td"); + balancer.closeTag("tr"); + balancer.closeTag("div"); // Ignored: no div in table scope. + balancer.closeTag("table"); + balancer.text("tail"); + balancer.closeDocument(); + + assertEquals( + "
x
" + + "
y
tail", + htmlOutputBuffer.toString()); + } + + /** The row group and row the content was pushed out of return as well. */ + @Test + void testPushedOutRowAndRowGroupReturnWithTheTable() { + balancer.openDocument(); + balancer.openTag("table", j8().listOf()); + balancer.openTag("tbody", j8().listOf()); + balancer.openTag("tr", j8().listOf()); + balancer.openTag("td", j8().listOf()); + balancer.text("a"); + balancer.closeTag("td"); + balancer.openTag("div", j8().listOf()); + balancer.text("x"); + balancer.openTag("td", j8().listOf()); + balancer.text("b"); + balancer.closeTag("td"); + balancer.closeTag("tr"); + balancer.closeTag("table"); + balancer.closeDocument(); + + assertEquals( + "
a
x
" + + "
b
", + htmlOutputBuffer.toString()); + } + + /** The table's own end tag ends the content pushed out of it. */ + @Test + void testPushedOutTableEndsWithItsEndTag() { + balancer.openDocument(); + balancer.openTag("table", j8().listOf()); + balancer.openTag("div", j8().listOf()); + balancer.text("x"); + balancer.closeTag("table"); + balancer.text("tail"); + balancer.closeDocument(); + + assertEquals( + "
x
tail", htmlOutputBuffer.toString()); + } + + /** + * A pushed-out table is closed in the output but still counts toward the + * nesting limit, which is a conservative reading of the limit. + */ + @Test + void testPushedOutTableCountsTowardTheNestingLimit() { + balancer.setNestingLimit(3); + balancer.openDocument(); + balancer.openTag("table", j8().listOf()); + balancer.openTag("div", j8().listOf()); + balancer.openTag("p", j8().listOf()); + balancer.openTag("span", j8().listOf()); // Past the limit. + balancer.text("x"); + balancer.closeDocument(); + + assertEquals( + "

x

", htmlOutputBuffer.toString()); + } + @Test void testNestingLimits() { // Some browsers can be DoSed by deeply nested structures. From 6025077584e7a67095e17763ae5920b7b6d6797e Mon Sep 17 00:00:00 2001 From: Jim Manico Date: Thu, 10 Sep 2026 20:29:36 -1000 Subject: [PATCH 2/5] Stop the return to a pushed-out table at a table-scope boundary A browser looks for the table to return to within table scope only, so a table or part arriving inside a template in the pushed-out content stays there, and the pushed-out table waits for a part arriving outside it. The return used to close the template and jump back. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_014Ydng7gBm6Ax5zfwt4vZip --- .../TagBalancingHtmlStreamEventReceiver.java | 11 +++++++++ .../org/owasp/html/HtmlSanitizerTest.java | 23 +++++++++++++++++++ 2 files changed, 34 insertions(+) diff --git a/owasp-java-html-sanitizer/src/main/java/org/owasp/html/TagBalancingHtmlStreamEventReceiver.java b/owasp-java-html-sanitizer/src/main/java/org/owasp/html/TagBalancingHtmlStreamEventReceiver.java index de15d307..3b0e0d62 100644 --- a/owasp-java-html-sanitizer/src/main/java/org/owasp/html/TagBalancingHtmlStreamEventReceiver.java +++ b/owasp-java-html-sanitizer/src/main/java/org/owasp/html/TagBalancingHtmlStreamEventReceiver.java @@ -445,9 +445,20 @@ private void pushOutTable(int topIndex) { * nearest pushed-out table, pops the pushed-out entries that cannot hold * the part, even by implying elements between, which for a table is all of * them, and writes the rest again as a new table for the part to go in. + *

+ * Not across a boundary of table scope, such as a {@code template} in the + * pushed-out content: a browser looks for the table within that scope + * only, so the part is handled where it arrived, as it would be with no + * table pushed out. */ private void returnToPushedOutTable(int elIndex) { int top = pushedOut.length() - 1; // The nearest pushed-out entry. + byte tableScope = SCOPE_FOR_END_TAG[TABLE_TAG]; + for (int i = openElements.size(); --i > top;) { + if ((SCOPES_BY_ELEMENT[openElements.get(i)] & tableScope) != 0) { + return; + } + } for (int i = openElements.size(); --i > top;) { int unclosed = openElements.remove(i); outputElements.remove(i); diff --git a/owasp-java-html-sanitizer/src/test/java/org/owasp/html/HtmlSanitizerTest.java b/owasp-java-html-sanitizer/src/test/java/org/owasp/html/HtmlSanitizerTest.java index b1d44ba6..1527fbf4 100644 --- a/owasp-java-html-sanitizer/src/test/java/org/owasp/html/HtmlSanitizerTest.java +++ b/owasp-java-html-sanitizer/src/test/java/org/owasp/html/HtmlSanitizerTest.java @@ -1958,6 +1958,29 @@ void testPushedOutLinkIsNotResumedAroundAnotherLink() throws Exception { p.sanitize("x
y
")); } + /** + * A browser looks for the table to return to within table scope only, so + * a table or row inside a {@code template} in the pushed-out content + * stays in the template, and the pushed-out table waits for a part + * arriving outside it. + */ + @Test + void testReturnToAPushedOutTableStopsAtATableScopeBoundary() { + PolicyFactory p = new HtmlPolicyBuilder() + .allowElements("table", "tbody", "tr", "td", "div", "template") + .toFactory(); + String out = p.sanitize( + "
" + + "z
w
v
u"); + + assertEquals( + "

z
w" + + "
v
u", + out); + assertEquals(out, p.sanitize(out)); + } + /** Pushed-out content is bounded by the nesting limit like any other. */ @Test void testPushedOutContentRespectsTheNestingLimit() { From 77fc91036553a875706875882850891e28453fee Mon Sep 17 00:00:00 2001 From: Jim Manico Date: Fri, 11 Sep 2026 03:51:50 -1000 Subject: [PATCH 3/5] Judge pushed-out content by the element that holds the table A review of the push-out found four shapes where content beside a pushed-out table was handled as if the table still contained it, three of them regressions against the behaviour before the push-out landed. A browser puts content a table cannot hold in front of the table, so the element that holds the table is the one that holds the content, and it is what decides which elements are implied around the content and what has to close. The pushed-out entries stay on the stack, so every step after the push-out was reading a table as the container: * Implied elements were computed against the pushed-out table, so an option beside one lost the select the tables never leave to chance, and the output was not idempotent. * Containment was not re-checked below the pushed-out run, so an element that cannot hold the content nested it instead of closing: a heading inside a heading, a button inside a button, neither of which survives a browser's parse. * Text whose container could not hold text was written into it and dropped by the policy's text gate rather than closing it, losing the text after a table inside a colgroup. * A cell returning to a pushed-out row group was given a fresh table inside the re-opened group, since a non-empty implied path was read as the group being able to hold the cell even when that path leads through a table. The push-out now happens before anything asks what contains the content, and only when the table cannot hold it with implied elements between; a container index skips a pushed-out run, so the implied elements and the close loop see the real container and close it when it cannot hold the content; and the return to a pushed-out table keeps an entry only when the implied path to the part runs through it. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_014Ydng7gBm6Ax5zfwt4vZip --- change_log.md | 11 +- .../TagBalancingHtmlStreamEventReceiver.java | 132 +++++++++++----- .../org/owasp/html/HtmlSanitizerTest.java | 146 +++++++++++++++++- 3 files changed, 242 insertions(+), 47 deletions(-) diff --git a/change_log.md b/change_log.md index 9770f0cc..504ce42b 100644 --- a/change_log.md +++ b/change_log.md @@ -13,9 +13,14 @@ Most recent at top. output cannot put anything in front of a tag already written, so the table is written twice, once empty and once with the later rows, and text pushed out of a table follows it rather than preceding it as in a - browser; a browser reads the rest as it reads the input. A link - pushed out of a table is not written again around or inside another - link. + browser; a browser reads the rest as it reads the input. Such content + is judged by the element that holds the table, which is where a + browser puts it: that element closes if it cannot hold the content, + and supplies the elements a browser would imply around it, such as the + `select` around an `option`. A link is no longer written again around + or inside another link, with or without a table involved, since a + browser's parse unnests links and the output would read back as a + different tree. * What `HtmlStreamRenderer` leaves out now reaches an `HtmlChangeListener` as well as the renderer's bad-HTML handler: a start tag whose name is not one HTML allows, which an `ElementPolicy` can produce by renaming, diff --git a/owasp-java-html-sanitizer/src/main/java/org/owasp/html/TagBalancingHtmlStreamEventReceiver.java b/owasp-java-html-sanitizer/src/main/java/org/owasp/html/TagBalancingHtmlStreamEventReceiver.java index 3b0e0d62..c73eee99 100644 --- a/owasp-java-html-sanitizer/src/main/java/org/owasp/html/TagBalancingHtmlStreamEventReceiver.java +++ b/owasp-java-html-sanitizer/src/main/java/org/owasp/html/TagBalancingHtmlStreamEventReceiver.java @@ -92,6 +92,10 @@ public class TagBalancingHtmlStreamEventReceiver * and returns to the table: its parts, which clear the stack back to the * table, and a table itself, which pops the open table and takes its * place. + *

+ * These are the same elements whose end tags are scoped to a table in + * {@link #SCOPE_FOR_END_TAG}, which is built far below; the two lists are + * written out separately because that one is not initialized yet here. */ private static final BitSet TABLE_PARTS = new BitSet(); static { @@ -313,9 +317,22 @@ private void prepareForContent(int elIndex) { && TABLE_PARTS.get(elIndex)) { returnToPushedOutTable(elIndex); } - int nOpen = openElements.size(); + // Push an open table out of the way before anything below asks what + // contains the content: a browser puts content a table cannot hold in + // front of the table, so what contains the table contains the content, + // and it is what decides which elements are implied and what must close. + if (isFosterParented(elIndex) && !endsAnOpenLink(elIndex)) { + int tableIndex = containerIndex(); + if (tableIndex >= 0 + && TABLE_CONTEXT.get(openElements.get(tableIndex)) + && !canHold(elIndex, openElements.get(tableIndex), tableIndex)) { + pushOutTable(tableIndex); + } + } + { - int top = nOpen != 0 ? openElements.get(nOpen - 1) : BODY_TAG; + int container = containerIndex(); + int top = container >= 0 ? openElements.get(container) : BODY_TAG; // Open implied elements, such as list-items and table cells & rows. int[] impliedElIndices = METADATA.impliedElements(top, elIndex); if (impliedElIndices.length != 0) { @@ -339,50 +356,47 @@ private void prepareForContent(int elIndex) { openElements.add(impliedElIndex); outputElements.add( outputElementIndexForLastOpenTag(impliedElIndex)); - top = impliedElIndex; - ++nOpen; } } } - if (nOpen != 0) { - int top = openElements.get(nOpen - 1); - // Close all the elements that cannot contain the content to open. - while (true) { - // A link ends the link open before it, wherever that is: nested - // links do not survive a browser's parse, so a table between them - // cannot stay open either. - boolean linkEndsLink = - elIndex == A_TAG && hasOpenLinkInFormattingScope(); - boolean canContain = canContain(elIndex, top, nOpen - 1) - && !linkEndsLink; - if (canContain) { - break; - } - if (!linkEndsLink - && TABLE_CONTEXT.get(top) && isFosterParented(elIndex)) { - // A browser puts the content in front of the table and keeps the - // table open. Close the table in the output, keep it here, and - // open the content beside it. - pushOutTable(nOpen - 1); - break; - } - if (openElements.size() < nestingLimit && !pushedOut.get(nOpen - 1)) { - underlying.closeTag(METADATA.canonNameForIndex(top)); + // Close all the elements that cannot contain the content to open. + while (true) { + int container = containerIndex(); + if (container < 0) { break; } + int top = openElements.get(container); + // A link ends the link open before it, wherever that is: nested links + // do not survive a browser's parse, so a table between them cannot + // stay open either. + boolean endsLink = endsAnOpenLink(elIndex); + if (!endsLink && canContain(elIndex, top, container)) { + break; + } + if (!endsLink + && TABLE_CONTEXT.get(top) && isFosterParented(elIndex)) { + // As above, for a table uncovered by closing what held it. + pushOutTable(container); + continue; + } + // Close the container, and with it anything put in front of a table + // it holds, from the top down. + for (int i = openElements.size(); --i >= container;) { + int unclosed = openElements.get(i); + if (i + 1 < nestingLimit && !pushedOut.get(i)) { + underlying.closeTag(METADATA.canonNameForIndex(unclosed)); } - openElements.remove(--nOpen); - outputElements.remove(nOpen); - pushedOut.clear(nOpen); - if (METADATA.resumable(top) && top != elIndex) { - toResumeInReverse.add(top); + openElements.remove(i); + outputElements.remove(i); + pushedOut.clear(i); + if (METADATA.resumable(unclosed) && unclosed != elIndex) { + toResumeInReverse.add(unclosed); } - if (nOpen == 0) { break; } - top = openElements.get(nOpen - 1); } } while (!toResumeInReverse.isEmpty()) { int toResume = toResumeInReverse.getLast(); + int nOpen; // If toResume can contain elInfo AND the top of the stack can contain // toResume, then we push toResume. A link is not resumed around // another link, or where one is open: a browser ends a link when the @@ -413,12 +427,52 @@ && canContain(elIndex, toResume, nOpen) /** * True if a browser puts content of this kind that arrives inside a table, * outside a cell or caption, in front of the table rather than in it: text, - * and any element that is not one of a table's own parts. + * and any element that is not one of a table's own parts. What a table + * may hold directly, such as a {@code script} or a {@code form}, never + * reaches this: the containment tables say the table can contain it. */ private static boolean isFosterParented(int elIndex) { return elIndex == HtmlElementTables.TEXT_NODE || !TABLE_PARTS.get(elIndex); } + /** True if a link is open that a browser ends before opening this one. */ + private boolean endsAnOpenLink(int elIndex) { + return elIndex == A_TAG && hasOpenLinkInFormattingScope(); + } + + /** + * The stack index of the element that content arriving now goes into: the + * top, unless a pushed-out table is at the top, in which case the content + * goes beside the table, so the element that contains the table is the one + * that contains the content. + */ + private int containerIndex() { + int i = openElements.size(); + while (--i >= 0 && pushedOut.get(i)) { + // Skip a table closed in the output but still open here. + } + return i; + } + + /** + * True if {@code container} can hold {@code elIndex}, with elements + * implied between them where it needs them. The implied path has to run + * through the container: one that does not is a fresh table, or list, to + * open inside it rather than a way into the one that is already open. + */ + private boolean canHold( + int elIndex, int container, int containerIndexOnStack) { + int[] implied = METADATA.impliedElements(container, elIndex); + for (int i = 0, n = implied.length; i < n; ++i) { + if (implied[i] == container) { + return i + 1 < n + || canContain(elIndex, container, containerIndexOnStack); + } + } + return implied.length == 0 + && canContain(elIndex, container, containerIndexOnStack); + } + /** * Closes in the output, innermost first, the row, row group and table that * the top of the stack is in, and marks them pushed out, keeping them here. @@ -470,11 +524,7 @@ private void returnToPushedOutTable(int elIndex) { } } while (top >= 0 && pushedOut.get(top)) { - int entry = openElements.get(top); - if (canContain(elIndex, entry, top) - || METADATA.impliedElements(entry, elIndex).length != 0) { - break; - } + if (canHold(elIndex, openElements.get(top), top)) { break; } openElements.remove(top); outputElements.remove(top); pushedOut.clear(top); diff --git a/owasp-java-html-sanitizer/src/test/java/org/owasp/html/HtmlSanitizerTest.java b/owasp-java-html-sanitizer/src/test/java/org/owasp/html/HtmlSanitizerTest.java index 1527fbf4..b0a405b2 100644 --- a/owasp-java-html-sanitizer/src/test/java/org/owasp/html/HtmlSanitizerTest.java +++ b/owasp-java-html-sanitizer/src/test/java/org/owasp/html/HtmlSanitizerTest.java @@ -1981,6 +1981,131 @@ void testReturnToAPushedOutTableStopsAtATableScopeBoundary() { assertEquals(out, p.sanitize(out)); } + /** + * Content is judged by what holds the table it was pushed out of, since + * that is where a browser puts it. An element that cannot hold the + * content closes instead of nesting it: a heading inside a heading does + * not survive a browser's parse, so the output would read back as a + * different tree. + */ + @Test + void testPushedOutContentIsJudgedByWhatHoldsTheTable() { + PolicyFactory p = new HtmlPolicyBuilder() + .allowElements("table", "tbody", "tr", "td", "h1", "button") + .toFactory(); + String heading = p.sanitize( + "

b

a
"); + assertEquals( + "

a

" + + "

b

", + heading); + assertEquals(heading, p.sanitize(heading)); + + String button = p.sanitize( + "" + + "", + button); + assertEquals(button, p.sanitize(button)); + } + + /** + * And it gets the elements a browser would imply around it there: an + * option needs its select whether or not a table was pushed out of the + * way, which the tables never leave to chance, since the sanitizer does + * not know what the output will be embedded in. + */ + @Test + void testContentBesideAPushedOutTableStillGetsItsImpliedWrapper() { + PolicyFactory p = new HtmlPolicyBuilder() + .allowElements("table", "tbody", "tr", "td", "select", "option") + .toFactory(); + String out = p.sanitize("x
z"); + + assertEquals("
xz", out); + assertEquals(out, p.sanitize(out)); + } + + /** + * Text that follows a pushed-out table reaches the output even where the + * element holding the table cannot hold text: that element closes, as it + * would for text arriving anywhere else, rather than the text going into + * it and being dropped. The table inside the {@code colgroup} here is a + * separate, older mis-implication (#483); what this pins is that + * {@code tail} survives. + */ + @Test + void testTextAfterAPushedOutTableSurvivesAContainerThatCannotHoldIt() { + PolicyFactory p = new HtmlPolicyBuilder() + .allowElements("table", "colgroup", "col", "tbody", "tr", "td") + .toFactory(); + String out = p.sanitize("tail
a
"); + + assertEquals( + "" + + "
a
" + + "tail", + out); + } + + /** + * A cell returning to a pushed-out table lands in the table, not in a + * fresh one: the row group it arrives in cannot hold a cell without a row + * between, and the path to that row runs through a table, so the row group + * goes and the table takes the cell. + */ + @Test + void testACellReturningToAPushedOutSectionDoesNotOpenANewTable() { + PolicyFactory p = new HtmlPolicyBuilder() + .allowElements("table", "thead", "tfoot", "tbody", "tr", "td", "th") + .toFactory(); + String head = p.sanitize("x
y
tail"); + assertEquals( + "
x" + + "
y
tail", + head); + assertEquals(head, p.sanitize(head)); + + String foot = p.sanitize("x
h
tail"); + assertEquals( + "
x" + + "
h
tail", + foot); + assertEquals(foot, p.sanitize(foot)); + } + + /** + * The rule that a link is not written again around or inside another link + * is not about tables: an end tag that closes the element a link was + * misnested in queues the link for resumption, and resuming it around the + * next link used to produce nested links, which a browser's parse + * unnests, so the output read back as a different tree. + */ + @Test + void testLinkIsNotResumedAroundAnotherLinkWithoutATable() { + PolicyFactory p = new HtmlPolicyBuilder() + .allowElements("div", "p", "a") + .allowAttributes("href").onElements("a") + .allowUrlProtocols("http") + .allowWithoutAttributes("a") + .toFactory(); + String out = p.sanitize( + "y"); + assertEquals( + "y", + out); + assertEquals(out, p.sanitize(out)); + + String nested = p.sanitize( + "
x

y

"); + assertEquals( + "
x

" + + "y
", + nested); + assertEquals(nested, p.sanitize(nested)); + } + /** Pushed-out content is bounded by the nesting limit like any other. */ @Test void testPushedOutContentRespectsTheNestingLimit() { @@ -1993,12 +2118,27 @@ void testPushedOutContentRespectsTheNestingLimit() { out.endsWith("
y
z"), out.substring(out.length() - 80)); assertEquals(out, p.sanitize(out)); + } - sb = new StringBuilder(); + /** + * A table arriving beside a pushed-out table replaces it, so a long run + * of them leaves nothing behind: the stack stays flat however many there + * are, where the content used to nest one run inside the next. + */ + @Test + void testRepeatedPushOutAndReturnDoesNotAccumulate() { + PolicyFactory p = tablePolicy(); + StringBuilder sb = new StringBuilder(); for (int i = 0; i < 300; ++i) { sb.append("
"); } sb.append("
y
z"); - out = p.sanitize(sb.toString()); - assertTrue(out.startsWith("
"), out); + String out = p.sanitize(sb.toString()); + + StringBuilder expected = new StringBuilder(); + for (int i = 0; i < 300; ++i) { expected.append("
"); } + expected.setLength(expected.length() - "
".length()); + expected.append("
") + .append("
y
z"); + assertEquals(expected.toString(), out); assertEquals(out, p.sanitize(out)); } From 3240a05477d5ba2a9efa7f805719ae363df9e0da Mon Sep 17 00:00:00 2001 From: Jim Manico Date: Fri, 11 Sep 2026 06:46:49 -1000 Subject: [PATCH 4/5] Fix table foster-parenting edge cases --- change_log.md | 7 +- ...ndAttributePolicyBasedSanitizerPolicy.java | 67 ++++- .../org/owasp/html/HtmlChangeReporter.java | 71 ++++- .../java/org/owasp/html/HtmlSanitizer.java | 57 +++- .../TagBalancingHtmlStreamEventReceiver.java | 262 ++++++++++++++++-- .../org/owasp/html/HtmlSanitizerTest.java | 139 ++++++++++ .../TagBalancingHtmlStreamRendererTest.java | 35 +++ 7 files changed, 608 insertions(+), 30 deletions(-) diff --git a/change_log.md b/change_log.md index 504ce42b..2e537b8f 100644 --- a/change_log.md +++ b/change_log.md @@ -20,7 +20,12 @@ Most recent at top. `select` around an `option`. A link is no longer written again around or inside another link, with or without a table involved, since a browser's parse unnests links and the output would read back as a - different tree. + different tree. Table-part names in SVG and MathML stay in foreign + content, including across stray end tags, while names at HTML integration + points still return to the table. Implied table structure observes the + nesting limit. If an element policy drops the table written again for + later rows, or renames it, the synthetic replacement and its row + structure are suppressed while allowed cell text survives. * What `HtmlStreamRenderer` leaves out now reaches an `HtmlChangeListener` as well as the renderer's bad-HTML handler: a start tag whose name is not one HTML allows, which an `ElementPolicy` can produce by renaming, diff --git a/owasp-java-html-sanitizer/src/main/java/org/owasp/html/ElementAndAttributePolicyBasedSanitizerPolicy.java b/owasp-java-html-sanitizer/src/main/java/org/owasp/html/ElementAndAttributePolicyBasedSanitizerPolicy.java index 2d0d64fc..f2666110 100644 --- a/owasp-java-html-sanitizer/src/main/java/org/owasp/html/ElementAndAttributePolicyBasedSanitizerPolicy.java +++ b/owasp-java-html-sanitizer/src/main/java/org/owasp/html/ElementAndAttributePolicyBasedSanitizerPolicy.java @@ -49,6 +49,9 @@ class ElementAndAttributePolicyBasedSanitizerPolicy implements HtmlSanitizer.Policy, TagBalancingHtmlStreamEventReceiver.TextSuppressionPolicy, TagBalancingHtmlStreamEventReceiver.OpenTagOutputPolicy, + TagBalancingHtmlStreamEventReceiver.OpenTagSuppressionPolicy, + TagBalancingHtmlStreamEventReceiver.ReopenedTablePolicy, + TagBalancingHtmlStreamEventReceiver.OutputContextPolicy, HtmlChangeReporter.AttributelessSkipPolicy, HtmlChangeReporter.DroppedTextSource, HtmlChangeReporter.DiscardedAttributeSource { @@ -105,6 +108,9 @@ class ElementAndAttributePolicyBasedSanitizerPolicy * emits. {@link #isLiteralContentElement} says why it matters. */ private boolean inForeignContent; + /** Browser tree-construction context for the tags actually emitted. */ + private HtmlSanitizer.ForeignContentContext outputForeignContent + = new HtmlSanitizer.ForeignContentContext(); /** * The last few characters emitted for the kept literal-content element that * is open. Text arrives in chunks, and {@link #stripTags} needs them to see @@ -187,6 +193,7 @@ public void openDocument() { inKeptCdataElement = false; keptCdataElementName = null; inForeignContent = false; + outputForeignContent = new HtmlSanitizer.ForeignContentContext(); literalTextTail = ""; droppedTextListener = null; discardedAttributeListener = null; @@ -204,6 +211,7 @@ public void closeDocument() { for (int i = openElementStack.size() - 1; i >= 0; i -= 2) { String tagNameToClose = openElementStack.get(i); if (tagNameToClose != null) { + outputForeignContent.processEndTag(tagNameToClose); out.closeTag(tagNameToClose); } } @@ -234,6 +242,20 @@ public void reportDiscardedAttributesTo( return outputElementNameForLastOpenTag; } + public boolean isOutputInForeignContent() { + return outputForeignContent.isInForeignContent(); + } + + public @Nullable String outputForeignContentRootName() { + return outputForeignContent.outermostForeignElementName(); + } + + public boolean outputStartTagUsesForeignContentRules( + String elementName, List attrs) { + return outputForeignContent.startTagUsesForeignContentRules( + elementName, attrs); + } + public void text(String textChunk) { if (!skipText) { // The renderer emits the text of a kept literal-content element as it @@ -762,13 +784,40 @@ private boolean isLiteralContentElement(String adjustedElementName) { } public void openTag(String elementName, List attrs) { + openTag(elementName, attrs, OpenTagMode.NORMAL); + } + + public void openTagWithoutOutput(String elementName, List attrs) { + openTag(elementName, attrs, OpenTagMode.SUPPRESS); + } + + public void openReopenedTable(List attrs) { + openTag("table", attrs, OpenTagMode.REOPENED_TABLE); + } + + private void openTag( + String elementName, List attrs, OpenTagMode mode) { outputElementNameForLastOpenTag = null; ElementAndAttributePolicies policies = elAndAttrPolicies.get(elementName); String adjustedElementName = applyPolicies(elementName, attrs, policies); skippedLastTagAsAttributeless = false; if (adjustedElementName != null) { if (!(attrs.isEmpty() && policies.htmlTagSkipType.skipAvailability())) { - writeOpenTag(policies, adjustedElementName, attrs); + if (mode == OpenTagMode.NORMAL + || (mode == OpenTagMode.REOPENED_TABLE + && "table".equals(adjustedElementName))) { + writeOpenTag(policies, adjustedElementName, attrs); + } else if (!HtmlTextEscapingMode.isVoidElement(elementName)) { + push(elementName, null); + skipText = !allowedTextContainers.contains(elementName) + || disallowedTextContainers.contains(elementName) + // An emitted HTML breakout can leave the renderer's lexical + // SVG/Math nesting open after the browser context has left it. + // Text from a suppressed table part cannot be placed safely in + // that stale lexical context, so fail closed for that text. + || (inForeignContent + && !outputForeignContent.isInForeignContent()); + } return; } // The element was allowed; it goes only because no attribute survived. @@ -777,6 +826,12 @@ public void openTag(String elementName, List attrs) { deferOpenTag(elementName); } + private enum OpenTagMode { + NORMAL, + REOPENED_TABLE, + SUPPRESS, + } + public boolean skippedLastTagAsAttributeless() { return skippedLastTagAsAttributeless; } @@ -841,6 +896,7 @@ public void closeTag(String elementName) { for (int j = n - 1; j > i; j -= 2) { String tagNameToClose = openElementStack.get(j); if (tagNameToClose != null) { + outputForeignContent.processEndTag(tagNameToClose); out.closeTag(tagNameToClose); } } @@ -866,10 +922,15 @@ void writeOpenTag( // on the name the policy emitted it under. The stack follows suit, so // that every entry on it is one a close tag can pop. if (HtmlTextEscapingMode.isVoidElement(elementName)) { + boolean adjustedIsVoid = + HtmlTextEscapingMode.isVoidElement(adjustedElementName); + outputForeignContent.processStartTag( + adjustedElementName, attrs, adjustedIsVoid); out.openTag(adjustedElementName, attrs); - if (!HtmlTextEscapingMode.isVoidElement(adjustedElementName)) { + if (!adjustedIsVoid) { // Renamed to an element that needs closing, which nothing upstream // will do: closed at once, so it does not swallow what follows. + outputForeignContent.processEndTag(adjustedElementName); out.closeTag(adjustedElementName); } return; @@ -881,6 +942,7 @@ void writeOpenTag( // a dropped element. push(elementName, null); skipText = skipText || suppressesTextWhenDropped(elementName); + outputForeignContent.processStartTag(adjustedElementName, attrs, true); out.openTag(adjustedElementName, attrs); return; } @@ -907,6 +969,7 @@ void writeOpenTag( inForeignContent = inForeignContent || HtmlStreamRenderer.FOREIGN_CONTENT_ROOT_ELEMENT_NAMES.contains( adjustedElementName); + outputForeignContent.processStartTag(adjustedElementName, attrs, false); out.openTag(adjustedElementName, attrs); } diff --git a/owasp-java-html-sanitizer/src/main/java/org/owasp/html/HtmlChangeReporter.java b/owasp-java-html-sanitizer/src/main/java/org/owasp/html/HtmlChangeReporter.java index 073065e0..5fa5ce9f 100644 --- a/owasp-java-html-sanitizer/src/main/java/org/owasp/html/HtmlChangeReporter.java +++ b/owasp-java-html-sanitizer/src/main/java/org/owasp/html/HtmlChangeReporter.java @@ -33,8 +33,11 @@ import javax.annotation.Nullable; -import org.owasp.html.TagBalancingHtmlStreamEventReceiver.TextSuppressionPolicy; import org.owasp.html.TagBalancingHtmlStreamEventReceiver.OpenTagOutputPolicy; +import org.owasp.html.TagBalancingHtmlStreamEventReceiver.OpenTagSuppressionPolicy; +import org.owasp.html.TagBalancingHtmlStreamEventReceiver.OutputContextPolicy; +import org.owasp.html.TagBalancingHtmlStreamEventReceiver.ReopenedTablePolicy; +import org.owasp.html.TagBalancingHtmlStreamEventReceiver.TextSuppressionPolicy; /** * Sits between the HTML parser, the policy, and the renderer so that it @@ -127,8 +130,11 @@ void reportDiscardedAttributesTo( private static final class InputChannel implements HtmlSanitizer.Policy, TagBalancingHtmlStreamEventReceiver.NestingLimitListener, - TextSuppressionPolicy, OpenTagOutputPolicy, + OpenTagSuppressionPolicy, + OutputContextPolicy, + ReopenedTablePolicy, + TextSuppressionPolicy, HtmlStreamRenderer.DropListener { HtmlStreamEventReceiver policy; final OutputChannel output; @@ -139,6 +145,12 @@ private static final class InputChannel /** Output name produced in response to the most recent input start tag. */ private @Nullable String outputElementNameForLastOpenTag; + private enum OpenTagMode { + NORMAL, + REOPENED_TABLE, + SUPPRESS, + } + InputChannel( OutputChannel output, HtmlChangeListener listener, @Nullable T context) { @@ -201,6 +213,24 @@ public void droppedAttribute( return outputElementNameForLastOpenTag; } + public boolean isOutputInForeignContent() { + return policy instanceof OutputContextPolicy + && ((OutputContextPolicy) policy).isOutputInForeignContent(); + } + + public @Nullable String outputForeignContentRootName() { + return policy instanceof OutputContextPolicy + ? ((OutputContextPolicy) policy).outputForeignContentRootName() + : null; + } + + public boolean outputStartTagUsesForeignContentRules( + String elementName, List attrs) { + return policy instanceof OutputContextPolicy + && ((OutputContextPolicy) policy) + .outputStartTagUsesForeignContentRules(elementName, attrs); + } + public void openDocument() { pendingDroppedText.clear(); outputElementNameForLastOpenTag = null; @@ -237,11 +267,40 @@ public void closeDocument() { } public void openTag(String elementName, List attrs) { + openTag(elementName, attrs, OpenTagMode.NORMAL); + } + + public void openTagWithoutOutput( + String elementName, List attrs) { + openTag(elementName, attrs, OpenTagMode.SUPPRESS); + } + + public void openReopenedTable(List attrs) { + openTag("table", attrs, OpenTagMode.REOPENED_TABLE); + } + + private void openTag( + String elementName, List attrs, OpenTagMode mode) { output.openedElementName = null; // Copied before the policy runs: it removes rejected attributes from // attrs in place, and their values are wanted for the report. output.expectAttributes(attrs); - policy.openTag(elementName, attrs); + if (mode == OpenTagMode.REOPENED_TABLE) { + if (!(policy instanceof ReopenedTablePolicy)) { + throw new IllegalStateException( + "Policy cannot safely reopen a table"); + } + ((ReopenedTablePolicy) policy).openReopenedTable(attrs); + } else if (mode == OpenTagMode.SUPPRESS) { + if (!(policy instanceof OpenTagSuppressionPolicy)) { + throw new IllegalStateException( + "Policy cannot suppress a table-structure tag"); + } + ((OpenTagSuppressionPolicy) policy) + .openTagWithoutOutput(elementName, attrs); + } else { + policy.openTag(elementName, attrs); + } { // Gather the notification details to avoid any problems with the // listener re-entering the stream event receiver. This shouldn't @@ -249,8 +308,10 @@ public void openTag(String elementName, List attrs) { // // The tag survived if the policy opened anything in response and // the renderer wrote it. Its name is not compared with the input - // name: an ElementPolicy may rename the element, and a renamed - // element was kept, not dropped. + // name: an ElementPolicy may rename an ordinary element, and that + // renamed element was kept. A synthetic table reopen is the exception: + // its policy result is deliberately suppressed unless it remains a + // table, so it is reported as discarded here. boolean discarded = output.openedElementName == null; outputElementNameForLastOpenTag = output.openedElementName; output.openedElementName = null; diff --git a/owasp-java-html-sanitizer/src/main/java/org/owasp/html/HtmlSanitizer.java b/owasp-java-html-sanitizer/src/main/java/org/owasp/html/HtmlSanitizer.java index ce81370e..0e9fa523 100644 --- a/owasp-java-html-sanitizer/src/main/java/org/owasp/html/HtmlSanitizer.java +++ b/owasp-java-html-sanitizer/src/main/java/org/owasp/html/HtmlSanitizer.java @@ -251,7 +251,7 @@ public static void sanitize( } /** Tracks the tree-construction context needed for self-closing flags. */ - private static final class ForeignContentContext { + static final class ForeignContentContext { /** Match the sanitizer's output nesting limit without growing unchecked. */ private static final int MAX_DEPTH = 256; @@ -292,12 +292,64 @@ private static final class ForeignContentContext { */ private boolean unknown; + /** Whether the most recently processed tag used foreign-content rules. */ + private boolean lastTagUsedForeignContentRules; + + /** + * True when the most recent start or end tag was processed in SVG or + * MathML rather than under the HTML tree-building rules. + */ + boolean lastTagUsedForeignContentRules() { + return lastTagUsedForeignContentRules; + } + + /** Whether a known current node is in SVG or MathML. */ + boolean isInForeignContent() { + OpenElement current = currentElement(); + return !unknown && current != null && current.namespace != Namespace.HTML; + } + + /** Whether this start tag would use foreign-content tree-building rules. */ + boolean startTagUsesForeignContentRules( + String elementName, List attrs) { + if (unknown) { return false; } + OpenElement current = currentElement(); + return !usesHtmlRulesForStartTag(current, elementName) + && !breaksOutOfForeignContent(elementName, attrs); + } + + /** The outermost SVG or MathML element in the tracked foreign region. */ + @Nullable String outermostForeignElementName() { + if (unknown) { return null; } + for (OpenElement open : openElements) { + if (open.namespace != Namespace.HTML) { return open.elementName; } + } + return null; + } + + /** Whether the foreign end-tag walk would find this local name. */ + boolean hasForeignElementNamed(String elementName) { + if (!isInForeignContent()) { return false; } + for (int i = openElements.size(); --i >= 0;) { + OpenElement open = openElements.get(i); + if (open.namespace == Namespace.HTML) { return false; } + if (asciiEqualsIgnoreCase(open.elementName, elementName)) { return true; } + } + return false; + } + + /** Records an HTML end tag known to be ignored without changing context. */ + void ignoreEndTagUnderHtmlRules() { + lastTagUsedForeignContentRules = false; + } + /** * Updates the context for a start tag and returns whether its self-closing * flag is honored by tree construction. */ boolean processStartTag( String elementName, List attrs, boolean selfClosing) { + lastTagUsedForeignContentRules = false; if (unknown) { return selfClosing && isForeignContentRoot(elementName); } @@ -316,6 +368,7 @@ && breaksOutOfForeignContent(elementName, attrs)) { // Any other start tag in foreign content inherits the current // namespace, even one named "svg" or "math". + lastTagUsedForeignContentRules = true; if (!selfClosing) { push(new OpenElement(elementName, current.namespace, attrs)); } @@ -324,6 +377,7 @@ && breaksOutOfForeignContent(elementName, attrs)) { /** Updates the context using the foreign-content or HTML end-tag rules. */ void processEndTag(String elementName) { + lastTagUsedForeignContentRules = false; if (unknown) { return; } if (openElements.isEmpty()) { if ("form".equals(elementName)) { @@ -353,6 +407,7 @@ void processEndTag(String elementName) { OpenElement open = openElements.get(i); if (open.namespace == Namespace.HTML) { break; } if (asciiEqualsIgnoreCase(open.elementName, elementName)) { + lastTagUsedForeignContentRules = true; openElements.subList(i, openElements.size()).clear(); return; } diff --git a/owasp-java-html-sanitizer/src/main/java/org/owasp/html/TagBalancingHtmlStreamEventReceiver.java b/owasp-java-html-sanitizer/src/main/java/org/owasp/html/TagBalancingHtmlStreamEventReceiver.java index c73eee99..efca5e44 100644 --- a/owasp-java-html-sanitizer/src/main/java/org/owasp/html/TagBalancingHtmlStreamEventReceiver.java +++ b/owasp-java-html-sanitizer/src/main/java/org/owasp/html/TagBalancingHtmlStreamEventReceiver.java @@ -48,6 +48,10 @@ public class TagBalancingHtmlStreamEventReceiver implements HtmlStreamEventReceiver { private final HtmlStreamEventReceiver underlying; private int nestingLimit = Integer.MAX_VALUE; + private HtmlSanitizer.ForeignContentContext foreignContent + = new HtmlSanitizer.ForeignContentContext(); + /** Input name whose rendered foreign root closes before the table reopens. */ + private @Nullable String foreignRootPendingTableReturn; private final IntVector openElements = new IntVector(); /** * The element each entry in {@link #openElements} became after policy @@ -73,6 +77,13 @@ public class TagBalancingHtmlStreamEventReceiver * are the only pushed-out entries below the content pushed out of them. */ private final BitSet pushedOut = new BitSet(); + /** + * Bit {@code i} is set when the policy did not emit a synthetic table while + * returning from {@link #pushedOut}. Its table-structure descendants cannot + * be emitted in that output context without changing when a browser reparses + * them. + */ + private final BitSet reopenedWithoutTable = new BitSet(); private static final HtmlElementTables METADATA = HtmlElementTables.get(); private static final int UNRECOGNIZED_TAG = METADATA.indexForName(HtmlElementNames.CUSTOM_ELEMENT_NAME); @@ -175,6 +186,32 @@ interface OpenTagOutputPolicy { @Nullable String outputElementNameForLastOpenTag(); } + /** + * Implemented by a policy that can apply its element and text decisions to + * a start tag while deliberately emitting no tag for it. + */ + interface OpenTagSuppressionPolicy { + void openTagWithoutOutput(String elementName, List attrs); + } + + /** + * Implemented by a policy that emits a synthetic table only when its element + * policy keeps it as a table, and otherwise retains it as a virtual element. + */ + interface ReopenedTablePolicy { + void openReopenedTable(List attrs); + } + + /** Reports the browser context of the elements the policy actually emits. */ + interface OutputContextPolicy { + boolean isOutputInForeignContent(); + + @Nullable String outputForeignContentRootName(); + + boolean outputStartTagUsesForeignContentRules( + String elementName, List attrs); + } + /** * How many elements whose content the policy would suppress -- {@code * " + + "" + + "ytail"; + String out = p.sanitize(input); + + assertEquals( + "
" + + "" + + "
" + + "
y
tail", + out); + assertEquals(out, p.sanitize(out)); + } + + /** + * A policy may drop or rename the attribute-free table written when table + * content resumes. A renamed synthetic table and its row structure cannot + * safely be emitted, but allowed cell text still survives. This includes + * replacements with table, void, select, raw-text and foreign parsing rules. + */ + @Test + void testTableStructureIsSuppressedUnderRenamedReopenedTable() { + String input = "
x
" + + "a</script><svg onload=x>b" + + "
tail"; + String[] replacements = { + "", "div", "tbody", "tr", "td", "caption", "colgroup", "col", + "br", "select", "option", "script", "style", "textarea", + "noscript", "xmp", "plaintext", "iframe", "svg", "math", + }; + HtmlChangeListener ignore = new HtmlChangeListener() { + public void discardedTag(Object context, String elementName) { + // Output is under test, notifications are not. + } + + public void discardedAttributes( + Object context, String tagName, String... attributeNames) { + // Output is under test, notifications are not. + } + }; + for (String c : replacements) { + final String replacement = c.isEmpty() ? null : c; + PolicyFactory p = new HtmlPolicyBuilder() + .allowElements( + (elementName, attrs) -> attrs.isEmpty() + ? replacement : elementName, + "table") + .allowElements("tbody", "tr", "td", "div") + .allowAttributes("class").onElements("table") + .allowTextIn("table") + .toFactory(); + String out = p.sanitize(input); + + assertEquals( + "
x
" + + "a</script><svg onload=x>btail", + out, c); + assertEquals(out, p.sanitize(out), c); + assertEquals(out, p.sanitize(input, ignore, null), c); + } + } + /** * A link pushed out of a table is closed when the table resumes, and is * not written again around another link or inside one: nested links do diff --git a/owasp-java-html-sanitizer/src/test/java/org/owasp/html/TagBalancingHtmlStreamRendererTest.java b/owasp-java-html-sanitizer/src/test/java/org/owasp/html/TagBalancingHtmlStreamRendererTest.java index 0560035f..06b54624 100644 --- a/owasp-java-html-sanitizer/src/test/java/org/owasp/html/TagBalancingHtmlStreamRendererTest.java +++ b/owasp-java-html-sanitizer/src/test/java/org/owasp/html/TagBalancingHtmlStreamRendererTest.java @@ -335,6 +335,41 @@ void testPushedOutTableCountsTowardTheNestingLimit() { "

x

", htmlOutputBuffer.toString()); } + /** Implied table structure never opens past a small nesting limit. */ + @Test + void testPushedOutTableImpliedElementsRespectSmallNestingLimits() { + String[] expected = { + "xytail", + "
x
ytail", + "
x
" + + "
ytail", + "
x
" + + "
ytail", + }; + for (int limit = 0; limit < expected.length; ++limit) { + StringBuilder out = new StringBuilder(); + TagBalancingHtmlStreamEventReceiver limited = + new TagBalancingHtmlStreamEventReceiver( + HtmlStreamRenderer.create( + out, x -> fail("Unexpected renderer error: " + x))); + limited.setNestingLimit(limit); + limited.openDocument(); + limited.openTag("table", j8().listOf()); + limited.openTag("div", j8().listOf()); + limited.text("x"); + limited.openTag("tr", j8().listOf()); + limited.openTag("td", j8().listOf()); + limited.text("y"); + limited.closeTag("td"); + limited.closeTag("tr"); + limited.closeTag("table"); + limited.text("tail"); + limited.closeDocument(); + + assertEquals(expected[limit], out.toString(), "limit " + limit); + } + } + @Test void testNestingLimits() { // Some browsers can be DoSed by deeply nested structures. From 705b33184c0e8839332a851b859a68ae35a64e62 Mon Sep 17 00:00:00 2001 From: Jim Manico Date: Fri, 11 Sep 2026 09:22:54 -1000 Subject: [PATCH 5/5] Preserve custom policy reporter compatibility --- ...ndAttributePolicyBasedSanitizerPolicy.java | 6 +- .../org/owasp/html/HtmlChangeReporter.java | 49 +++++++++------ .../TagBalancingHtmlStreamEventReceiver.java | 63 ++++++++++--------- .../owasp/html/HtmlChangeReporterTest.java | 41 ++++++++++++ 4 files changed, 108 insertions(+), 51 deletions(-) diff --git a/owasp-java-html-sanitizer/src/main/java/org/owasp/html/ElementAndAttributePolicyBasedSanitizerPolicy.java b/owasp-java-html-sanitizer/src/main/java/org/owasp/html/ElementAndAttributePolicyBasedSanitizerPolicy.java index f2666110..7322263f 100644 --- a/owasp-java-html-sanitizer/src/main/java/org/owasp/html/ElementAndAttributePolicyBasedSanitizerPolicy.java +++ b/owasp-java-html-sanitizer/src/main/java/org/owasp/html/ElementAndAttributePolicyBasedSanitizerPolicy.java @@ -49,9 +49,7 @@ class ElementAndAttributePolicyBasedSanitizerPolicy implements HtmlSanitizer.Policy, TagBalancingHtmlStreamEventReceiver.TextSuppressionPolicy, TagBalancingHtmlStreamEventReceiver.OpenTagOutputPolicy, - TagBalancingHtmlStreamEventReceiver.OpenTagSuppressionPolicy, - TagBalancingHtmlStreamEventReceiver.ReopenedTablePolicy, - TagBalancingHtmlStreamEventReceiver.OutputContextPolicy, + TagBalancingHtmlStreamEventReceiver.PushedOutTablePolicy, HtmlChangeReporter.AttributelessSkipPolicy, HtmlChangeReporter.DroppedTextSource, HtmlChangeReporter.DiscardedAttributeSource { @@ -242,6 +240,8 @@ public void reportDiscardedAttributesTo( return outputElementNameForLastOpenTag; } + public boolean supportsPushedOutTableOperations() { return true; } + public boolean isOutputInForeignContent() { return outputForeignContent.isInForeignContent(); } diff --git a/owasp-java-html-sanitizer/src/main/java/org/owasp/html/HtmlChangeReporter.java b/owasp-java-html-sanitizer/src/main/java/org/owasp/html/HtmlChangeReporter.java index 5fa5ce9f..91d8e4b0 100644 --- a/owasp-java-html-sanitizer/src/main/java/org/owasp/html/HtmlChangeReporter.java +++ b/owasp-java-html-sanitizer/src/main/java/org/owasp/html/HtmlChangeReporter.java @@ -34,9 +34,7 @@ import javax.annotation.Nullable; import org.owasp.html.TagBalancingHtmlStreamEventReceiver.OpenTagOutputPolicy; -import org.owasp.html.TagBalancingHtmlStreamEventReceiver.OpenTagSuppressionPolicy; -import org.owasp.html.TagBalancingHtmlStreamEventReceiver.OutputContextPolicy; -import org.owasp.html.TagBalancingHtmlStreamEventReceiver.ReopenedTablePolicy; +import org.owasp.html.TagBalancingHtmlStreamEventReceiver.PushedOutTablePolicy; import org.owasp.html.TagBalancingHtmlStreamEventReceiver.TextSuppressionPolicy; /** @@ -131,9 +129,7 @@ private static final class InputChannel implements HtmlSanitizer.Policy, TagBalancingHtmlStreamEventReceiver.NestingLimitListener, OpenTagOutputPolicy, - OpenTagSuppressionPolicy, - OutputContextPolicy, - ReopenedTablePolicy, + PushedOutTablePolicy, TextSuppressionPolicy, HtmlStreamRenderer.DropListener { HtmlStreamEventReceiver policy; @@ -213,22 +209,34 @@ public void droppedAttribute( return outputElementNameForLastOpenTag; } + public boolean supportsPushedOutTableOperations() { + PushedOutTablePolicy tablePolicy = pushedOutTablePolicy(); + return tablePolicy != null + && tablePolicy.supportsPushedOutTableOperations(); + } + public boolean isOutputInForeignContent() { - return policy instanceof OutputContextPolicy - && ((OutputContextPolicy) policy).isOutputInForeignContent(); + PushedOutTablePolicy tablePolicy = pushedOutTablePolicy(); + return tablePolicy != null && tablePolicy.isOutputInForeignContent(); } public @Nullable String outputForeignContentRootName() { - return policy instanceof OutputContextPolicy - ? ((OutputContextPolicy) policy).outputForeignContentRootName() - : null; + PushedOutTablePolicy tablePolicy = pushedOutTablePolicy(); + return tablePolicy != null + ? tablePolicy.outputForeignContentRootName() : null; } public boolean outputStartTagUsesForeignContentRules( String elementName, List attrs) { - return policy instanceof OutputContextPolicy - && ((OutputContextPolicy) policy) - .outputStartTagUsesForeignContentRules(elementName, attrs); + PushedOutTablePolicy tablePolicy = pushedOutTablePolicy(); + return tablePolicy != null + && tablePolicy.outputStartTagUsesForeignContentRules( + elementName, attrs); + } + + private @Nullable PushedOutTablePolicy pushedOutTablePolicy() { + return policy instanceof PushedOutTablePolicy + ? (PushedOutTablePolicy) policy : null; } public void openDocument() { @@ -286,18 +294,21 @@ private void openTag( // attrs in place, and their values are wanted for the report. output.expectAttributes(attrs); if (mode == OpenTagMode.REOPENED_TABLE) { - if (!(policy instanceof ReopenedTablePolicy)) { + PushedOutTablePolicy tablePolicy = pushedOutTablePolicy(); + if (tablePolicy == null + || !tablePolicy.supportsPushedOutTableOperations()) { throw new IllegalStateException( "Policy cannot safely reopen a table"); } - ((ReopenedTablePolicy) policy).openReopenedTable(attrs); + tablePolicy.openReopenedTable(attrs); } else if (mode == OpenTagMode.SUPPRESS) { - if (!(policy instanceof OpenTagSuppressionPolicy)) { + PushedOutTablePolicy tablePolicy = pushedOutTablePolicy(); + if (tablePolicy == null + || !tablePolicy.supportsPushedOutTableOperations()) { throw new IllegalStateException( "Policy cannot suppress a table-structure tag"); } - ((OpenTagSuppressionPolicy) policy) - .openTagWithoutOutput(elementName, attrs); + tablePolicy.openTagWithoutOutput(elementName, attrs); } else { policy.openTag(elementName, attrs); } diff --git a/owasp-java-html-sanitizer/src/main/java/org/owasp/html/TagBalancingHtmlStreamEventReceiver.java b/owasp-java-html-sanitizer/src/main/java/org/owasp/html/TagBalancingHtmlStreamEventReceiver.java index efca5e44..a0b15177 100644 --- a/owasp-java-html-sanitizer/src/main/java/org/owasp/html/TagBalancingHtmlStreamEventReceiver.java +++ b/owasp-java-html-sanitizer/src/main/java/org/owasp/html/TagBalancingHtmlStreamEventReceiver.java @@ -187,23 +187,20 @@ interface OpenTagOutputPolicy { } /** - * Implemented by a policy that can apply its element and text decisions to - * a start tag while deliberately emitting no tag for it. + * Optional policy operations used to keep output around a pushed-out table + * in a browser context that matches the logical input context. */ - interface OpenTagSuppressionPolicy { + interface PushedOutTablePolicy { + /** Whether the operations below are available for the current policy. */ + boolean supportsPushedOutTableOperations(); + + /** Applies element and text policy while deliberately emitting no tag. */ void openTagWithoutOutput(String elementName, List attrs); - } - /** - * Implemented by a policy that emits a synthetic table only when its element - * policy keeps it as a table, and otherwise retains it as a virtual element. - */ - interface ReopenedTablePolicy { + /** Emits a synthetic table only if policy keeps it as a table. */ void openReopenedTable(List attrs); - } - /** Reports the browser context of the elements the policy actually emits. */ - interface OutputContextPolicy { + /** Reports the browser context of the elements actually emitted. */ boolean isOutputInForeignContent(); @Nullable String outputForeignContentRootName(); @@ -391,24 +388,31 @@ private int outputElementIndexForLastOpenTag(int inputElementIndex) { * behavior, including its nesting-limit accounting. */ private boolean isOutputInForeignContent() { - return underlying instanceof OutputContextPolicy - && ((OutputContextPolicy) underlying).isOutputInForeignContent(); + PushedOutTablePolicy policy = pushedOutTablePolicy(); + return policy != null && policy.isOutputInForeignContent(); } /** The outermost foreign root the policy has actually emitted, if known. */ private @Nullable String outputForeignContentRootName() { - return underlying instanceof OutputContextPolicy - ? ((OutputContextPolicy) underlying).outputForeignContentRootName() - : null; + PushedOutTablePolicy policy = pushedOutTablePolicy(); + return policy != null ? policy.outputForeignContentRootName() : null; } /** Whether the current output context applies foreign rules to this tag. */ private boolean outputStartTagUsesForeignContentRules( String elementName, List attrs) { - return underlying instanceof OutputContextPolicy - ? ((OutputContextPolicy) underlying) - .outputStartTagUsesForeignContentRules(elementName, attrs) - : false; + PushedOutTablePolicy policy = pushedOutTablePolicy(); + return policy != null + && policy.outputStartTagUsesForeignContentRules(elementName, attrs); + } + + /** The pushed-out-table operations supported by the current policy. */ + private @Nullable PushedOutTablePolicy pushedOutTablePolicy() { + if (underlying instanceof PushedOutTablePolicy) { + PushedOutTablePolicy policy = (PushedOutTablePolicy) underlying; + if (policy.supportsPushedOutTableOperations()) { return policy; } + } + return null; } /** Opens one logical element, suppressing unsafe table structure if needed. */ @@ -423,10 +427,10 @@ private int openElement(int inputElementIndex, List attrs) { private int openElement( int inputElementIndex, List attrs, boolean implied) { String inputElementName = METADATA.canonNameForIndex(inputElementIndex); - if (shouldSuppressTablePart(inputElementIndex, implied) - && underlying instanceof OpenTagSuppressionPolicy) { - ((OpenTagSuppressionPolicy) underlying) - .openTagWithoutOutput(inputElementName, attrs); + PushedOutTablePolicy policy = pushedOutTablePolicy(); + if (policy != null + && shouldSuppressTablePart(inputElementIndex, implied)) { + policy.openTagWithoutOutput(inputElementName, attrs); return NO_OUTPUT_ELEMENT; } underlying.openTag(inputElementName, attrs); @@ -701,13 +705,13 @@ private void returnToPushedOutTable(int elIndex) { underlying.closeTag(foreignRootPendingTableReturn); foreignRootPendingTableReturn = null; } + PushedOutTablePolicy policy = pushedOutTablePolicy(); for (int i = 0; i < n; ++i) { int outputElementIndex = NO_OUTPUT_ELEMENT; if (openElements.size() < nestingLimit) { List attrs = new ArrayList<>(); - if (run[i] == TABLE_TAG - && underlying instanceof ReopenedTablePolicy) { - ((ReopenedTablePolicy) underlying).openReopenedTable(attrs); + if (run[i] == TABLE_TAG && policy != null) { + policy.openReopenedTable(attrs); outputElementIndex = outputElementIndexForLastOpenTag(run[i]); } else { outputElementIndex = openElement(run[i], attrs); @@ -715,7 +719,8 @@ private void returnToPushedOutTable(int elIndex) { } openElements.add(run[i]); outputElements.add(outputElementIndex); - if (run[i] == TABLE_TAG && outputElementIndex != TABLE_TAG) { + if (policy != null + && run[i] == TABLE_TAG && outputElementIndex != TABLE_TAG) { reopenedWithoutTable.set(openElements.size() - 1); } } diff --git a/owasp-java-html-sanitizer/src/test/java/org/owasp/html/HtmlChangeReporterTest.java b/owasp-java-html-sanitizer/src/test/java/org/owasp/html/HtmlChangeReporterTest.java index c8063d80..deccda3c 100644 --- a/owasp-java-html-sanitizer/src/test/java/org/owasp/html/HtmlChangeReporterTest.java +++ b/owasp-java-html-sanitizer/src/test/java/org/owasp/html/HtmlChangeReporterTest.java @@ -149,6 +149,47 @@ public void openTag(String elementName, List attrs) { assertEquals("", log.toString()); } + /** A public policy decorator cannot implement the package-private feedback. */ + @Test + void testPolicyDecoratorDoesNotClaimPushedOutTableOperations() { + final Context testContext = new Context(); + StringBuilder out = new StringBuilder(); + final StringBuilder log = new StringBuilder(); + HtmlStreamRenderer renderer = HtmlStreamRenderer.create( + out, Handler.DO_NOTHING); + HtmlChangeReporter hcr = new HtmlChangeReporter<>( + renderer, loggingListener(testContext, log), testContext); + final HtmlSanitizer.Policy delegate = new HtmlPolicyBuilder() + .allowElements("table", "tbody", "tr", "td", "div") + .toFactory() + .apply(hcr.getWrappedRenderer()); + hcr.setPolicy(new HtmlSanitizer.Policy() { + public void openDocument() { delegate.openDocument(); } + + public void closeDocument() { delegate.closeDocument(); } + + public void openTag(String elementName, List attrs) { + delegate.openTag(elementName, attrs); + } + + public void closeTag(String elementName) { + delegate.closeTag(elementName); + } + + public void text(String textChunk) { delegate.text(textChunk); } + }); + + HtmlSanitizer.sanitize( + "
x
y
tail", + hcr.getWrappedPolicy()); + + assertEquals( + "
x
" + + "
y
tail", + out.toString()); + assertEquals("", log.toString()); + } + /** * {@link ElementPolicy#apply} may return another element name, and the * reporter used to decide whether a tag survived by comparing names, so a