Skip to content

Close elements emitted at the nesting limit #486

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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
jmanico merged 1 commit into from
Sep 12, 2026
Merged

Close elements emitted at the nesting limit #486

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
6 changes: 6 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,12 @@

Most recent at top.
* Next release
* Elements written at the configured nesting limit now receive their end
tags when they close explicitly, implicitly, or with an ancestor. The
previous close checks were one level too strict and could leave the
sanitized output unbalanced. Close decisions for stacked elements also
continue to match what was written if the limit changes mid-document
(#485).
* 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
Expand Down
38 changes: 29 additions & 9 deletions ...java-html-sanitizer/src/main/java/org/owasp/html/TagBalancingHtmlStreamEventReceiver.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,13 @@ public class TagBalancingHtmlStreamEventReceiver
* When the receiver below cannot report that, the input name is used.
*/
private final IntVector outputElements = new IntVector();
/**
* Bit {@code i} is set when the open event for entry {@code i} of
* {@link #openElements} was sent to {@link #underlying}. Resumed formatting
* and returned table entries can remain on the logical stack at or beyond
* the nesting limit without an open event, and the limit may later change.
*/
private final BitSet sentToUnderlying = new BitSet();
private final IntVector toResumeInReverse = new IntVector();
/**
* Bit {@code i} is set while the element at {@code i} of
Expand Down Expand Up @@ -266,14 +273,15 @@ 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.
for (int i = openElements.size(); --i >= 0;) {
if (!sentToUnderlying.get(i) || pushedOut.get(i)) { continue; }
int elIndex = openElements.get(i);
String elname = METADATA.canonNameForIndex(elIndex);
underlying.closeTag(elname);
}
openElements.clear();
outputElements.clear();
sentToUnderlying.clear();
pushedOut.clear();
reopenedWithoutTable.clear();
foreignRootPendingTableReturn = null;
Expand Down Expand Up @@ -339,6 +347,7 @@ && isOutputInForeignContent()) {
if (!HtmlTextEscapingMode.isVoidElement(canonElementName)) {
openElements.add(elIndex);
outputElements.add(outputElementIndex);
sentToUnderlying.set(openElements.size() - 1);
}
} else {
if (contentIsSkippable(canonElementName)) { ++droppedSkippableDepth; }
Expand Down Expand Up @@ -498,6 +507,7 @@ private void prepareForContent(int elIndex) {
int outputElementIndex = openElement(impliedElIndex, attrs, true);
openElements.add(impliedElIndex);
outputElements.add(outputElementIndex);
sentToUnderlying.set(openElements.size() - 1);
if (suppressedImpliedTable) {
reopenedWithoutTable.set(openElements.size() - 1);
}
Expand Down Expand Up @@ -527,11 +537,12 @@ private void prepareForContent(int elIndex) {
// 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)) {
if (sentToUnderlying.get(i) && !pushedOut.get(i)) {
underlying.closeTag(METADATA.canonNameForIndex(unclosed));
}
openElements.remove(i);
outputElements.remove(i);
sentToUnderlying.clear(i);
pushedOut.clear(i);
reopenedWithoutTable.clear(i);
if (METADATA.resumable(unclosed) && unclosed != elIndex) {
Expand All @@ -556,11 +567,13 @@ && canContain(elIndex, toResume, nOpen)
&& (elIndex == A_TAG || hasOpenLinkInFormattingScope()))) {
toResumeInReverse.removeLast();
int outputElementIndex = NO_OUTPUT_ELEMENT;
if (openElements.size() < nestingLimit) {
boolean sent = openElements.size() < nestingLimit;
if (sent) {
outputElementIndex = openElement(toResume, new ArrayList<>());
}
openElements.add(toResume);
outputElements.add(outputElementIndex);
sentToUnderlying.set(openElements.size() - 1, sent);
} else {
break;
}
Expand Down Expand Up @@ -641,7 +654,7 @@ private void pushOutTable(int topIndex) {
int elIndex = openElements.get(i);
if (!TABLE_CONTEXT.get(elIndex)) { break; }
if (!pushedOut.get(i)) {
if (i < nestingLimit) {
if (sentToUnderlying.get(i)) {
underlying.closeTag(METADATA.canonNameForIndex(elIndex));
}
pushedOut.set(i);
Expand Down Expand Up @@ -673,9 +686,10 @@ private void returnToPushedOutTable(int elIndex) {
for (int i = openElements.size(); --i > top;) {
int unclosed = openElements.remove(i);
outputElements.remove(i);
if (i < nestingLimit) {
if (sentToUnderlying.get(i)) {
underlying.closeTag(METADATA.canonNameForIndex(unclosed));
}
sentToUnderlying.clear(i);
if (METADATA.resumable(unclosed)) {
toResumeInReverse.add(unclosed);
}
Expand All @@ -685,6 +699,7 @@ private void returnToPushedOutTable(int elIndex) {
if (canHold(elIndex, openElements.get(top), top)) { break; }
openElements.remove(top);
outputElements.remove(top);
sentToUnderlying.clear(top);
pushedOut.clear(top);
reopenedWithoutTable.clear(top);
--top;
Expand All @@ -698,6 +713,7 @@ private void returnToPushedOutTable(int elIndex) {
for (int i = n; --i >= 0;) {
run[i] = openElements.remove(start + i);
outputElements.remove(start + i);
sentToUnderlying.clear(start + i);
pushedOut.clear(start + i);
reopenedWithoutTable.clear(start + i);
}
Expand All @@ -708,7 +724,8 @@ private void returnToPushedOutTable(int elIndex) {
PushedOutTablePolicy policy = pushedOutTablePolicy();
for (int i = 0; i < n; ++i) {
int outputElementIndex = NO_OUTPUT_ELEMENT;
if (openElements.size() < nestingLimit) {
boolean sent = openElements.size() < nestingLimit;
if (sent) {
List<String> attrs = new ArrayList<>();
if (run[i] == TABLE_TAG && policy != null) {
policy.openReopenedTable(attrs);
Expand All @@ -719,6 +736,7 @@ private void returnToPushedOutTable(int elIndex) {
}
openElements.add(run[i]);
outputElements.add(outputElementIndex);
sentToUnderlying.set(openElements.size() - 1, sent);
if (policy != null
&& run[i] == TABLE_TAG && outputElementIndex != TABLE_TAG) {
reopenedWithoutTable.set(openElements.size() - 1);
Expand Down Expand Up @@ -877,9 +895,10 @@ && isOutputInForeignContent()) {
while (--last > index) {
int unclosed = openElements.remove(last);
outputElements.remove(last);
if (last + 1 < nestingLimit && !pushedOut.get(last)) {
if (sentToUnderlying.get(last) && !pushedOut.get(last)) {
underlying.closeTag(METADATA.canonNameForIndex(unclosed));
}
sentToUnderlying.clear(last);
pushedOut.clear(last);
reopenedWithoutTable.clear(last);
if (METADATA.resumable(unclosed)) {
Expand All @@ -890,9 +909,10 @@ && isOutputInForeignContent()) {
underlying.closeTag(foreignRootPendingTableReturn);
foreignRootPendingTableReturn = null;
}
if (openElements.size() < nestingLimit && !pushedOut.get(index)) {
if (sentToUnderlying.get(index) && !pushedOut.get(index)) {
underlying.closeTag(METADATA.canonNameForIndex(elIndex));
}
sentToUnderlying.clear(index);
pushedOut.clear(index);
reopenedWithoutTable.clear(index);
openElements.remove(index);
Expand Down
15 changes: 15 additions & 0 deletions owasp-java-html-sanitizer/src/test/java/org/owasp/html/HtmlSanitizerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,21 @@ void testTextSurvivesPastTheNestingLimit() {
assertEquals(256, deep.split("</div>", -1).length - 1);
}

/** The element at the 256th level is emitted and must also be closed. */
@Test
void testElementAtDefaultNestingLimitRoundTrips() throws Exception {
PolicyFactory p = new HtmlPolicyBuilder()
.allowElements("div", "span", "p")
.allowWithoutAttributes("span")
.toFactory();
String input = nest("<span>x</span><p>y</p>", 255);
String out = p.sanitize(input);

assertEquals(input, out);
assertEquals(out, p.sanitize(out));
assertEquals(parseAsBrowser(input), parseAsBrowser(out));
}

/**
* Issue #205. Text kept past the limit is still text: it is escaped on the
* way out and cannot reintroduce markup.
Expand Down
109 changes: 106 additions & 3 deletions ...-java-html-sanitizer/src/test/java/org/owasp/html/TagBalancingHtmlStreamRendererTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,14 +44,32 @@ class TagBalancingHtmlStreamRendererTest {

StringBuilder htmlOutputBuffer;
TagBalancingHtmlStreamEventReceiver balancer;
int emittedOpenElements;
int emittedCloseElements;

@BeforeEach
void createBalancer() {
htmlOutputBuffer = new StringBuilder();
HtmlStreamEventReceiver renderer = HtmlStreamRenderer.create(
htmlOutputBuffer,
x -> fail("Unexpected renderer error: " + x));
balancer = new TagBalancingHtmlStreamEventReceiver(
HtmlStreamRenderer.create(
htmlOutputBuffer,
x -> fail("Unexpected renderer error: " + x)));
new HtmlStreamEventReceiverWrapper(renderer) {
@Override
public void openTag(String elementName, List<String> attrs) {
super.openTag(elementName, attrs);
if (!HtmlTextEscapingMode.isVoidElement(
HtmlLexer.canonicalElementName(elementName))) {
++emittedOpenElements;
}
}

@Override
public void closeTag(String elementName) {
super.closeTag(elementName);
++emittedCloseElements;
}
});
}

@Test
Expand Down Expand Up @@ -388,6 +406,91 @@ void testNestingLimits() {
"<div><div><div><div><div><div><div><div><div><div>"
+ "</div></div></div></div></div></div></div></div></div></div>",
htmlOutputBuffer.toString());
assertEquals(emittedOpenElements, emittedCloseElements);
}

/** An explicit end tag closes an element written at the nesting limit. */
@Test
void testExplicitCloseAtNestingLimit() {
balancer.setNestingLimit(3);
balancer.openDocument();
balancer.openTag("div", j8().listOf());
balancer.openTag("div", j8().listOf());
balancer.openTag("span", j8().listOf());
balancer.text("x");
balancer.closeTag("span");
balancer.openTag("p", j8().listOf());
balancer.text("y");
balancer.closeDocument();

assertEquals(
"<div><div><span>x</span><p>y</p></div></div>",
htmlOutputBuffer.toString());
assertEquals(emittedOpenElements, emittedCloseElements);
}

/** A sibling implicitly closes an incompatible element at the limit. */
@Test
void testImplicitCloseAtNestingLimit() {
balancer.setNestingLimit(3);
balancer.openDocument();
balancer.openTag("div", j8().listOf());
balancer.openTag("div", j8().listOf());
balancer.openTag("textarea", j8().listOf());
balancer.text("x");
balancer.openTag("p", j8().listOf());
balancer.text("y");
balancer.closeDocument();

assertEquals(
"<div><div><textarea>x</textarea><p>y</p></div></div>",
htmlOutputBuffer.toString());
assertEquals(emittedOpenElements, emittedCloseElements);
}

/** Closing an ancestor also closes descendants written at the limit. */
@Test
void testAncestorCloseAtNestingLimit() {
balancer.setNestingLimit(3);
balancer.openDocument();
balancer.openTag("div", j8().listOf());
balancer.openTag("div", j8().listOf());
balancer.openTag("span", j8().listOf());
balancer.text("x");
balancer.closeTag("div");
balancer.closeDocument();

assertEquals(
"<div><div><span>x</span></div></div>",
htmlOutputBuffer.toString());
assertEquals(emittedOpenElements, emittedCloseElements);
}

/** Raising the limit must not close a resumed element that was not written. */
@Test
void testRaisedLimitDoesNotCloseUnemittedResumedElement() {
balancer.setNestingLimit(3);
balancer.openDocument();
balancer.openTag("table", j8().listOf());
balancer.openTag("caption", j8().listOf());
balancer.openTag("strong", j8().listOf());
balancer.closeTag("caption");
balancer.text("x");
balancer.openTag("strong", j8().listOf());
balancer.openTag("caption", j8().listOf());
balancer.closeTag("caption");
balancer.openTag("strong", j8().listOf());
balancer.openTag("span", j8().listOf());
balancer.setNestingLimit(4);
balancer.closeTag("table");
balancer.closeDocument();

assertEquals(
"<table><caption><strong></strong></caption></table>"
+ "x<strong></strong><table><caption></caption></table>"
+ "<strong><strong></strong></strong>",
htmlOutputBuffer.toString());
assertEquals(emittedOpenElements, emittedCloseElements);
}

@Test
Expand Down
Loading