Skip to content

Get tests passing and recognize foreign content nodes better #318

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.

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
merged 1 commit into from
Feb 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions src/main/java/org/owasp/html/Encoding.java
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ public final class Encoding {
* @return text/plain
* @deprecated specify whether s is in an attribute value
*/
@Deprecated
public static String decodeHtml(String s) {
return decodeHtml(s, false);
}
Expand Down
1 change: 1 addition & 0 deletions src/main/java/org/owasp/html/HtmlEntities.java
Original file line number Diff line number Diff line change
Expand Up @@ -2308,6 +2308,7 @@ final class HtmlEntities {
* @return The offset after the end of the decoded sequence in {@code html}.
* @deprecated specify whether html is in an attribute value.
*/
@Deprecated
public static int appendDecodedEntity(
String html, int offset, int limit, StringBuilder sb) {
return appendDecodedEntity(html, offset, limit, false, sb);
Expand Down
16 changes: 13 additions & 3 deletions src/main/java/org/owasp/html/HtmlPolicyBuilder.java
Original file line number Diff line number Diff line change
Expand Up @@ -1039,6 +1039,7 @@ public String apply(String elementName, List<String> attrs) {
relValue = DEFAULT_RELS_ON_TARGETTED_LINKS_STR;
} else {
StringBuilder sb = new StringBuilder();
Set<String> present = new HashSet<String>();
if (relIndex >= 0) {
// Preserve values that are not explicitly skipped.
String rels = attrs.get(relIndex);
Expand All @@ -1049,25 +1050,34 @@ public String apply(String elementName, List<String> attrs) {
if (skip.isEmpty()
|| !skip.contains(
Strings.toLowerCase(rels.substring(left, i)))) {
sb.append(rels, left, i).append(' ');
String rel = rels.substring(left, i);
present.add(rel);
sb.append(rel).append(' ');
}
}
left = i + 1;
}
}
}
for (String s : extra) {
sb.append(s).append(' ');
if (!present.contains(s)) {
sb.append(s).append(' ');
present.add(s);
}
}
if (hasTarget) {
for (String s : whenTargetPresent) {
sb.append(s).append(' ');
if (!present.contains(s)) {
sb.append(s).append(' ');
present.add(s);
}
}
}
int sblen = sb.length();
if (sblen == 0) {
relValue = "";
} else {
// Trim last space.
relValue = sb.substring(0, sb.length() - 1);
}
}
Expand Down
29 changes: 28 additions & 1 deletion src/main/java/org/owasp/html/HtmlStreamRenderer.java
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
import java.io.IOException;
import java.util.Iterator;
import java.util.List;
import java.util.Set;

import javax.annotation.WillCloseWhenClosed;
import javax.annotation.concurrent.NotThreadSafe;
Expand All @@ -57,6 +58,8 @@ public class HtmlStreamRenderer implements HtmlStreamEventReceiver {
private StringBuilder pendingUnescaped;
private HtmlTextEscapingMode escapingMode = HtmlTextEscapingMode.PCDATA;
private boolean open;
/** The count of {@link #foreignContentRootElementNames} opened and not subsequently closed. */
private int foreignContentDepth = 0;

/**
* Factory.
Expand Down Expand Up @@ -168,7 +171,25 @@ private void writeOpenTag(
return;
}

escapingMode = HtmlTextEscapingMode.getModeForTag(elementName);
if (foreignContentRootElementNames.contains(elementName)) {
foreignContentDepth += 1;
}

HtmlTextEscapingMode tentativeEscapingMode = HtmlTextEscapingMode.getModeForTag(elementName);
if (foreignContentDepth == 0) {
escapingMode = tentativeEscapingMode;
} else {
switch (tentativeEscapingMode) {
case PCDATA:
case VOID:
escapingMode = tentativeEscapingMode;
break;
default: // escape special characters but do not allow tags
escapingMode = HtmlTextEscapingMode.RCDATA;
break;
}
}


switch (escapingMode) {
case CDATA_SOMETIMES:
Expand Down Expand Up @@ -240,6 +261,10 @@ private final void writeCloseTag(String uncanonElementName)
return;
}

if (foreignContentDepth != 0 && foreignContentRootElementNames.contains(elementName)) {
foreignContentDepth -= 1;
}

if (pendingUnescaped != null) {
if (!lastTagOpened.equals(elementName)) {
error("Tag content cannot appear inside CDATA element", elementName);
Expand Down Expand Up @@ -436,4 +461,6 @@ public void close() throws IOException {
private static boolean isTagEnd(char ch) {
return ch < 63 && 0 != (TAG_ENDS & (1L << ch));
}

private static final Set<String> foreignContentRootElementNames = Set.of("svg", "math");
}
2 changes: 1 addition & 1 deletion src/test/java/org/owasp/html/Benchmark.java
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ public class Benchmark {
* specifies a benchmark to run and unspecified ones are not run.
*/
public static void main(String[] args) throws Exception {
String html = Files.readString(new File(args[0]).toPath(), StandardCharsets.UTF_8);
String html = new String(Files.readAllBytes(new File(args[0]).toPath()), StandardCharsets.UTF_8);

boolean timeLibhtmlparser = true;
boolean timeSanitize = true;
Expand Down
4 changes: 2 additions & 2 deletions src/test/java/org/owasp/html/HtmlLexerTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -45,12 +45,12 @@ public class HtmlLexerTest extends TestCase {
@Test
public final void testHtmlLexer() throws Exception {
// Do the lexing.
String input = new String(Files.readString(Paths.get(getClass().getResource("htmllexerinput1.html").toURI()), StandardCharsets.UTF_8));
String input = new String(Files.readAllBytes(Paths.get(getClass().getResource("htmllexerinput1.html").toURI())), StandardCharsets.UTF_8);
StringBuilder actual = new StringBuilder();
lex(input, actual);

// Get the golden.
String golden = new String(Files.readString(Paths.get(getClass().getResource("htmllexergolden1.txt").toURI()), StandardCharsets.UTF_8));
String golden = new String(Files.readAllBytes(Paths.get(getClass().getResource("htmllexergolden1.txt").toURI())), StandardCharsets.UTF_8);

// Compare.
assertEquals(golden, actual.toString());
Expand Down
4 changes: 2 additions & 2 deletions src/test/java/org/owasp/html/HtmlPolicyBuilderTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -801,7 +801,7 @@ public static final void testLinkRelsWhenRelPresent() {
}

@Test
public static final void testRelLinksWhenRelisPartOfData() {
public final void testRelLinksWhenRelIsPartOfData() {
PolicyFactory pf = new HtmlPolicyBuilder()
.allowElements("a")
.allowAttributes("href").onElements("a")
Expand All @@ -810,7 +810,7 @@ public static final void testRelLinksWhenRelisPartOfData() {
.allowStandardUrlProtocols()
.toFactory();
String toSanitize = "<a target=\"_blank\" rel=\"noopener noreferrer\" href=\"https://google.com\">test</a>";
assertTrue("Failure in testRelLinksWhenRelisPartOfData", pf.sanitize(toSanitize).equals(toSanitize));
assertEquals(toSanitize, pf.sanitize(toSanitize));
}

@Test
Expand Down