Skip to content

Keep a table open across content a browser would put in front of it (#342) #481

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
jmanico merged 5 commits into from
Sep 11, 2026
Merged

Keep a table open across content a browser would put in front of it (#342) #481

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
b93f7b6
Keep a table open across content a browser would put in front of it
jmanico Sep 11, 2026
6025077
Stop the return to a pushed-out table at a table-scope boundary
jmanico Sep 11, 2026
77fc910
Judge pushed-out content by the element that holds the table
jmanico Sep 11, 2026
3240a05
Fix table foster-parenting edge cases
jmanico Sep 11, 2026
705b331
Preserve custom policy reporter compatibility
jmanico Sep 11, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions change_log.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,30 @@

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. 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. 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,
Expand Down
67 changes: 65 additions & 2 deletions ...sanitizer/src/main/java/org/owasp/html/ElementAndAttributePolicyBasedSanitizerPolicy.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ class ElementAndAttributePolicyBasedSanitizerPolicy
implements HtmlSanitizer.Policy,
TagBalancingHtmlStreamEventReceiver.TextSuppressionPolicy,
TagBalancingHtmlStreamEventReceiver.OpenTagOutputPolicy,
TagBalancingHtmlStreamEventReceiver.PushedOutTablePolicy,
HtmlChangeReporter.AttributelessSkipPolicy,
HtmlChangeReporter.DroppedTextSource,
HtmlChangeReporter.DiscardedAttributeSource {
Expand Down Expand Up @@ -105,6 +106,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
Expand Down Expand Up @@ -187,6 +191,7 @@ public void openDocument() {
inKeptCdataElement = false;
keptCdataElementName = null;
inForeignContent = false;
outputForeignContent = new HtmlSanitizer.ForeignContentContext();
literalTextTail = "";
droppedTextListener = null;
discardedAttributeListener = null;
Expand All @@ -204,6 +209,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);
}
}
Expand Down Expand Up @@ -234,6 +240,22 @@ public void reportDiscardedAttributesTo(
return outputElementNameForLastOpenTag;
}

public boolean supportsPushedOutTableOperations() { return true; }

public boolean isOutputInForeignContent() {
return outputForeignContent.isInForeignContent();
}

public @Nullable String outputForeignContentRootName() {
return outputForeignContent.outermostForeignElementName();
}

public boolean outputStartTagUsesForeignContentRules(
String elementName, List<String> 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
Expand Down Expand Up @@ -762,13 +784,40 @@ private boolean isLiteralContentElement(String adjustedElementName) {
}

public void openTag(String elementName, List<String> attrs) {
openTag(elementName, attrs, OpenTagMode.NORMAL);
}

public void openTagWithoutOutput(String elementName, List<String> attrs) {
openTag(elementName, attrs, OpenTagMode.SUPPRESS);
}

public void openReopenedTable(List<String> attrs) {
openTag("table", attrs, OpenTagMode.REOPENED_TABLE);
}

private void openTag(
String elementName, List<String> 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.
Expand All @@ -777,6 +826,12 @@ public void openTag(String elementName, List<String> attrs) {
deferOpenTag(elementName);
}

private enum OpenTagMode {
NORMAL,
REOPENED_TABLE,
SUPPRESS,
}

public boolean skippedLastTagAsAttributeless() {
return skippedLastTagAsAttributeless;
}
Expand Down Expand Up @@ -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);
}
}
Expand All @@ -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;
Expand All @@ -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;
}
Expand All @@ -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);
}

Expand Down
82 changes: 77 additions & 5 deletions owasp-java-html-sanitizer/src/main/java/org/owasp/html/HtmlChangeReporter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,9 @@

import javax.annotation.Nullable;

import org.owasp.html.TagBalancingHtmlStreamEventReceiver.TextSuppressionPolicy;
import org.owasp.html.TagBalancingHtmlStreamEventReceiver.OpenTagOutputPolicy;
import org.owasp.html.TagBalancingHtmlStreamEventReceiver.PushedOutTablePolicy;
import org.owasp.html.TagBalancingHtmlStreamEventReceiver.TextSuppressionPolicy;

/**
* Sits between the HTML parser, the policy, and the renderer so that it
Expand Down Expand Up @@ -127,8 +128,9 @@ void reportDiscardedAttributesTo(
private static final class InputChannel<T>
implements HtmlSanitizer.Policy,
TagBalancingHtmlStreamEventReceiver.NestingLimitListener,
TextSuppressionPolicy,
OpenTagOutputPolicy,
PushedOutTablePolicy,
TextSuppressionPolicy,
HtmlStreamRenderer.DropListener {
HtmlStreamEventReceiver policy;
final OutputChannel output;
Expand All @@ -139,6 +141,12 @@ private static final class InputChannel<T>
/** 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<? super T> listener,
@Nullable T context) {
Expand Down Expand Up @@ -201,6 +209,36 @@ public void droppedAttribute(
return outputElementNameForLastOpenTag;
}

public boolean supportsPushedOutTableOperations() {
PushedOutTablePolicy tablePolicy = pushedOutTablePolicy();
return tablePolicy != null
&& tablePolicy.supportsPushedOutTableOperations();
}

public boolean isOutputInForeignContent() {
PushedOutTablePolicy tablePolicy = pushedOutTablePolicy();
return tablePolicy != null && tablePolicy.isOutputInForeignContent();
}

public @Nullable String outputForeignContentRootName() {
PushedOutTablePolicy tablePolicy = pushedOutTablePolicy();
return tablePolicy != null
? tablePolicy.outputForeignContentRootName() : null;
}

public boolean outputStartTagUsesForeignContentRules(
String elementName, List<String> 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() {
pendingDroppedText.clear();
outputElementNameForLastOpenTag = null;
Expand Down Expand Up @@ -237,20 +275,54 @@ public void closeDocument() {
}

public void openTag(String elementName, List<String> attrs) {
openTag(elementName, attrs, OpenTagMode.NORMAL);
}

public void openTagWithoutOutput(
String elementName, List<String> attrs) {
openTag(elementName, attrs, OpenTagMode.SUPPRESS);
}

public void openReopenedTable(List<String> attrs) {
openTag("table", attrs, OpenTagMode.REOPENED_TABLE);
}

private void openTag(
String elementName, List<String> 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) {
PushedOutTablePolicy tablePolicy = pushedOutTablePolicy();
if (tablePolicy == null
|| !tablePolicy.supportsPushedOutTableOperations()) {
throw new IllegalStateException(
"Policy cannot safely reopen a table");
}
tablePolicy.openReopenedTable(attrs);
} else if (mode == OpenTagMode.SUPPRESS) {
PushedOutTablePolicy tablePolicy = pushedOutTablePolicy();
if (tablePolicy == null
|| !tablePolicy.supportsPushedOutTableOperations()) {
throw new IllegalStateException(
"Policy cannot suppress a table-structure tag");
}
tablePolicy.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
// occur, but if it does it will be a source of subtle confusing bugs.
//
// 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;
Expand Down
Loading
Loading