Skip to content

Fix review follow-ups for literal text and link balancing #477

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 11, 2026
Merged

Fix review follow-ups for literal text and link balancing #477

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
19 changes: 19 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,25 @@

Most recent at top.
* Next release
* The filter on text kept inside `style`, `script`, `iframe` and other
literal-content elements now examines a possible tag prefix before
looking for its closing `>`. A long run of `<` characters before one
`>` made it repeatedly scan and copy the same suffix, taking quadratic
time. Removing a tag now also removes every immediately preceding `<`,
rather than letting `<<<b>img` turn into `<img`. Issue #476.
* Nested-link balancing now uses the elements that survived policy when
looking for the nearest formatting marker. If a `td`, `th`, `caption`,
`template`, `applet`, `marquee` or `object` was dropped or renamed to a
non-marker, it no longer protects an outer link from a later link in the
output, so the sanitized HTML parses back to the structure the sanitizer
emitted.
Issue #476.
* `HtmlChangeListener.discardedAttribute` now reports the rejected value
when it precedes a surviving attribute with the same name. The output
comparison used to account for the first input name regardless of which
copy the attribute policy rejected, so the listener could receive the
safe surviving URL instead of the rejected `javascript:` URL. Issue
#476.
* Two more ways past the filter on kept `style`, `script` and `iframe`
text are closed. A `<` that opened no tag carried everything up to
the next `>` through as text, and that `>` could belong to an end tag
Expand Down
104 changes: 80 additions & 24 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 @@ -50,8 +50,10 @@
class ElementAndAttributePolicyBasedSanitizerPolicy
implements HtmlSanitizer.Policy,
TagBalancingHtmlStreamEventReceiver.TextSuppressionPolicy,
TagBalancingHtmlStreamEventReceiver.OpenTagOutputPolicy,
HtmlChangeReporter.AttributelessSkipPolicy,
HtmlChangeReporter.DroppedTextSource {
HtmlChangeReporter.DroppedTextSource,
HtmlChangeReporter.DiscardedAttributeSource {
final Map<String, ElementAndAttributePolicies> elAndAttrPolicies;
final Set<String> allowedTextContainers;
/**
Expand Down Expand Up @@ -114,6 +116,11 @@ class ElementAndAttributePolicyBasedSanitizerPolicy

/** Told about filtered literal content; null while nobody is listening. */
private @Nullable HtmlStreamRenderer.DroppedTextListener droppedTextListener;
/** Told exactly which input attributes an attribute policy rejects. */
private @Nullable HtmlChangeReporter.DiscardedAttributeListener
discardedAttributeListener;
/** The output name, if any, produced by the most recent open-tag call. */
private transient @Nullable String outputElementNameForLastOpenTag;

ElementAndAttributePolicyBasedSanitizerPolicy(
HtmlStreamEventReceiver out,
Expand Down Expand Up @@ -151,6 +158,8 @@ public void openDocument() {
inKeptCdataElement = false;
keptCdataElementName = null;
droppedTextListener = null;
discardedAttributeListener = null;
outputElementNameForLastOpenTag = null;
skippedLastTagAsAttributeless = false;
openElementStack.clear();
skipTextBeforeOpen.clear();
Expand All @@ -173,6 +182,7 @@ public void closeDocument() {
skipText = true;
inKeptCdataElement = false;
keptCdataElementName = null;
outputElementNameForLastOpenTag = null;
out.closeDocument();
}

Expand All @@ -181,6 +191,15 @@ public void reportDroppedTextTo(
this.droppedTextListener = listener;
}

public void reportDiscardedAttributesTo(
@Nullable HtmlChangeReporter.DiscardedAttributeListener listener) {
this.discardedAttributeListener = listener;
}

public @Nullable String outputElementNameForLastOpenTag() {
return outputElementNameForLastOpenTag;
}

public void text(String textChunk) {
if (!skipText) {
// The renderer emits the text of a kept literal-content element as it
Expand Down Expand Up @@ -247,18 +266,34 @@ private String stripTags(String text, String elementName) {
while (i < len) {
int tagStart = text.indexOf('<', i);
if (tagStart < 0) { break; }
int tagEnd = text.indexOf('>', tagStart + 1);
if (tagEnd < 0) { break; } // No '<' from here on starts a tag.
String trimmed = text.substring(tagStart + 1, tagEnd).trim();
boolean isEndTag = trimmed.startsWith("/");
String tagName = tagNameOf(trimmed, isEndTag);
if (tagName == null) {
int nameStart = skipTrimSpace(text, tagStart + 1);
if (nameStart == len) { break; }
boolean isEndTag = text.charAt(nameStart) == '/';
if (isEndTag) {
nameStart = skipTrimSpace(text, nameStart + 1);
if (nameStart == len) { break; }
}
if (!Character.isLetter(text.charAt(nameStart))) {
// Not a tag: "<!-- -->", "</>", "<3" and the like. The '<' is text,
// and the scan resumes right after it: the '>' found above may end
// a tag that starts inside the span, as in "< </noscript>".
// and the scan resumes right after it. In particular, do not search
// for a '>' until the prefix is known to open a tag: repeatedly
// searching the same suffix makes a long run of '<' quadratic.
i = tagStart + 1;
continue;
}
int tagEnd = text.indexOf('>', nameStart + 1);
if (tagEnd < 0) { break; } // No '<' from here on starts a tag.
int bodyEnd = tagEnd;
while (bodyEnd > nameStart && text.charAt(bodyEnd - 1) <= ' ') {
--bodyEnd;
}
int nameEnd = nameStart + 1;
while (nameEnd < bodyEnd
&& !isRegexWhitespace(text.charAt(nameEnd))) {
++nameEnd;
}
String tagName = HtmlLexer.canonicalElementName(
text.substring(nameStart, nameEnd));
int kind = isEndTag ? END_TAG : START_TAG;
int[] tag = { tagStart, tagEnd + 1, -1, kind };
if (kind == START_TAG) {
Expand Down Expand Up @@ -286,11 +321,13 @@ private String stripTags(String text, String elementName) {
int dropStart = tag[TAG_START];
pos = tag[KIND] == END_TAG || tag[MATCH_END] < 0
? tag[TAG_END] : tag[MATCH_END];
// A '<' that the dropped tag followed would meet what follows the tag.
// Any '<'s that the dropped tag followed would meet what follows the
// tag. Take the whole adjacent run, lest "<<<b>img" become "<img".
int last = result.length() - 1;
if (last >= 0 && result.charAt(last) == '<') {
while (last >= 0 && result.charAt(last) == '<') {
result.setLength(last);
dropStart -= 1;
--last;
}
reportDroppedText(elementName, text, dropStart, pos);
}
Expand Down Expand Up @@ -325,18 +362,26 @@ private void reportDroppedText(
/** The kinds of record. */
private static final int START_TAG = 0, END_TAG = 1;

/**
* The canonical name of the tag whose trimmed content between the angle
* brackets is {@code trimmed}, or null if it is not a tag. A tag name
* starts with a letter; whitespace between {@code <} or {@code </} and the
* name is tolerated, which is stricter than a browser.
*/
private static @Nullable String tagNameOf(String trimmed, boolean isEndTag) {
String body = isEndTag ? trimmed.substring(1).trim() : trimmed;
if (body.isEmpty() || !Character.isLetter(body.charAt(0))) {
return null;
/** Skips characters that {@link String#trim} treats as whitespace. */
private static int skipTrimSpace(String s, int start) {
int i = start;
while (i < s.length() && s.charAt(i) <= ' ') { ++i; }
return i;
}

/** The Java 8 regular-expression meaning of {@code \\s}. */
private static boolean isRegexWhitespace(char ch) {
switch (ch) {
case ' ':
case '\t':
case '\n':
case '\u000b':
case '\f':
case '\r':
return true;
default:
return false;
}
return HtmlLexer.canonicalElementName(body.split("\\s")[0]);
}

/**
Expand All @@ -359,6 +404,7 @@ private static boolean isLiteralContentElement(String elementName) {
}

public void openTag(String elementName, List<String> attrs) {
outputElementNameForLastOpenTag = null;
ElementAndAttributePolicies policies = elAndAttrPolicies.get(elementName);
String adjustedElementName = applyPolicies(elementName, attrs, policies);
skippedLastTagAsAttributeless = false;
Expand All @@ -377,7 +423,7 @@ public boolean skippedLastTagAsAttributeless() {
return skippedLastTagAsAttributeless;
}

static final @Nullable String applyPolicies(
private @Nullable String applyPolicies(
String elementName, List<String> attrs,
ElementAndAttributePolicies policies) {
String adjustedElementName;
Expand All @@ -389,12 +435,14 @@ public boolean skippedLastTagAsAttributeless() {
= policies.attrPolicies.get(name);
if (attrPolicy == null) {
attrsIt.remove();
attrsIt.next();
String value = attrsIt.next();
reportDiscardedAttribute(name, value);
attrsIt.remove();
} else {
String value = attrsIt.next();
String adjustedValue = attrPolicy.apply(elementName, name, value);
if (adjustedValue == null) {
reportDiscardedAttribute(name, value);
attrsIt.remove();
attrsIt.previous();
attrsIt.remove();
Expand All @@ -419,6 +467,13 @@ public boolean skippedLastTagAsAttributeless() {
return adjustedElementName;
}

/** Reports an attribute-policy rejection without invoking user code. */
private void reportDiscardedAttribute(String name, String value) {
if (discardedAttributeListener != null) {
discardedAttributeListener.discardedAttribute(name, value);
}
}

public void closeTag(String elementName) {
int n = openElementStack.size();
for (int i = n; i > 0;) {
Expand All @@ -445,6 +500,7 @@ public void closeTag(String elementName) {
void writeOpenTag(
ElementAndAttributePolicies policies, String adjustedElementName,
List<String> attrs) {
outputElementNameForLastOpenTag = adjustedElementName;
if (!HtmlTextEscapingMode.isVoidElement(adjustedElementName)) {
push(policies.elementName, adjustedElementName);
// A kept element is the container for the text inside it. It is judged
Expand Down
Loading
Loading