Skip to content

Commit e8aa0f1

Browse files
authored
Get tests passing and recognize foreign content nodes better (#318)
Prepping for a release. One test was not passing: HtmlBuilderTest.testRelLinksWhenRelisPartOfData I was also getting IDE warnings that there were some uses of `@since 10` APIs which made it hard to run tests outside mvn. This commit cleans up those issues and a few warnings about `@deprecated` in javadoc without a corresponding annotation. Signed-off-by: Mike Samuel <mikesamuel@gmail.com>
1 parent 73b86b5 commit e8aa0f1

File tree

7 files changed

+48
-9
lines changed

7 files changed

+48
-9
lines changed

src/main/java/org/owasp/html/Encoding.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ public final class Encoding {
4343
* @return text/plain
4444
* @deprecated specify whether s is in an attribute value
4545
*/
46+
@Deprecated
4647
public static String decodeHtml(String s) {
4748
return decodeHtml(s, false);
4849
}

src/main/java/org/owasp/html/HtmlEntities.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2308,6 +2308,7 @@ final class HtmlEntities {
23082308
* @return The offset after the end of the decoded sequence in {@code html}.
23092309
* @deprecated specify whether html is in an attribute value.
23102310
*/
2311+
@Deprecated
23112312
public static int appendDecodedEntity(
23122313
String html, int offset, int limit, StringBuilder sb) {
23132314
return appendDecodedEntity(html, offset, limit, false, sb);

src/main/java/org/owasp/html/HtmlPolicyBuilder.java

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1037,6 +1037,7 @@ public String apply(String elementName, List<String> attrs) {
10371037
relValue = DEFAULT_RELS_ON_TARGETTED_LINKS_STR;
10381038
} else {
10391039
StringBuilder sb = new StringBuilder();
1040+
Set<String> present = new HashSet<String>();
10401041
if (relIndex >= 0) {
10411042
// Preserve values that are not explicitly skipped.
10421043
String rels = attrs.get(relIndex);
@@ -1047,25 +1048,34 @@ public String apply(String elementName, List<String> attrs) {
10471048
if (skip.isEmpty()
10481049
|| !skip.contains(
10491050
Strings.toLowerCase(rels.substring(left, i)))) {
1050-
sb.append(rels, left, i).append(' ');
1051+
String rel = rels.substring(left, i);
1052+
present.add(rel);
1053+
sb.append(rel).append(' ');
10511054
}
10521055
}
10531056
left = i + 1;
10541057
}
10551058
}
10561059
}
10571060
for (String s : extra) {
1058-
sb.append(s).append(' ');
1061+
if (!present.contains(s)) {
1062+
sb.append(s).append(' ');
1063+
present.add(s);
1064+
}
10591065
}
10601066
if (hasTarget) {
10611067
for (String s : whenTargetPresent) {
1062-
sb.append(s).append(' ');
1068+
if (!present.contains(s)) {
1069+
sb.append(s).append(' ');
1070+
present.add(s);
1071+
}
10631072
}
10641073
}
10651074
int sblen = sb.length();
10661075
if (sblen == 0) {
10671076
relValue = "";
10681077
} else {
1078+
// Trim last space.
10691079
relValue = sb.substring(0, sb.length() - 1);
10701080
}
10711081
}

src/main/java/org/owasp/html/HtmlStreamRenderer.java

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
import java.io.IOException;
3434
import java.util.Iterator;
3535
import java.util.List;
36+
import java.util.Set;
3637

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

6164
/**
6265
* Factory.
@@ -168,7 +171,25 @@ private void writeOpenTag(
168171
return;
169172
}
170173

171-
escapingMode = HtmlTextEscapingMode.getModeForTag(elementName);
174+
if (foreignContentRootElementNames.contains(elementName)) {
175+
foreignContentDepth += 1;
176+
}
177+
178+
HtmlTextEscapingMode tentativeEscapingMode = HtmlTextEscapingMode.getModeForTag(elementName);
179+
if (foreignContentDepth == 0) {
180+
escapingMode = tentativeEscapingMode;
181+
} else {
182+
switch (tentativeEscapingMode) {
183+
case PCDATA:
184+
case VOID:
185+
escapingMode = tentativeEscapingMode;
186+
break;
187+
default: // escape special characters but do not allow tags
188+
escapingMode = HtmlTextEscapingMode.RCDATA;
189+
break;
190+
}
191+
}
192+
172193

173194
switch (escapingMode) {
174195
case CDATA_SOMETIMES:
@@ -240,6 +261,10 @@ private final void writeCloseTag(String uncanonElementName)
240261
return;
241262
}
242263

264+
if (foreignContentDepth != 0 && foreignContentRootElementNames.contains(elementName)) {
265+
foreignContentDepth -= 1;
266+
}
267+
243268
if (pendingUnescaped != null) {
244269
if (!lastTagOpened.equals(elementName)) {
245270
error("Tag content cannot appear inside CDATA element", elementName);
@@ -436,4 +461,6 @@ public void close() throws IOException {
436461
private static boolean isTagEnd(char ch) {
437462
return ch < 63 && 0 != (TAG_ENDS & (1L << ch));
438463
}
464+
465+
private static final Set<String> foreignContentRootElementNames = Set.of("svg", "math");
439466
}

src/test/java/org/owasp/html/Benchmark.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ public class Benchmark {
5858
* specifies a benchmark to run and unspecified ones are not run.
5959
*/
6060
public static void main(String[] args) throws Exception {
61-
String html = Files.readString(new File(args[0]).toPath(), StandardCharsets.UTF_8);
61+
String html = new String(Files.readAllBytes(new File(args[0]).toPath()), StandardCharsets.UTF_8);
6262

6363
boolean timeLibhtmlparser = true;
6464
boolean timeSanitize = true;

src/test/java/org/owasp/html/HtmlLexerTest.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,12 +45,12 @@ public class HtmlLexerTest extends TestCase {
4545
@Test
4646
public final void testHtmlLexer() throws Exception {
4747
// Do the lexing.
48-
String input = new String(Files.readString(Paths.get(getClass().getResource("htmllexerinput1.html").toURI()), StandardCharsets.UTF_8));
48+
String input = new String(Files.readAllBytes(Paths.get(getClass().getResource("htmllexerinput1.html").toURI())), StandardCharsets.UTF_8);
4949
StringBuilder actual = new StringBuilder();
5050
lex(input, actual);
5151

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

5555
// Compare.
5656
assertEquals(golden, actual.toString());

src/test/java/org/owasp/html/HtmlPolicyBuilderTest.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -862,7 +862,7 @@ public static final void testLinkRelsWhenRelPresent() {
862862
}
863863

864864
@Test
865-
public static final void testRelLinksWhenRelisPartOfData() {
865+
public final void testRelLinksWhenRelIsPartOfData() {
866866
PolicyFactory pf = new HtmlPolicyBuilder()
867867
.allowElements("a")
868868
.allowAttributes("href").onElements("a")
@@ -871,7 +871,7 @@ public static final void testRelLinksWhenRelisPartOfData() {
871871
.allowStandardUrlProtocols()
872872
.toFactory();
873873
String toSanitize = "<a target=\"_blank\" rel=\"noopener noreferrer\" href=\"https://google.com\">test</a>";
874-
assertTrue("Failure in testRelLinksWhenRelisPartOfData", pf.sanitize(toSanitize).equals(toSanitize));
874+
assertEquals(toSanitize, pf.sanitize(toSanitize));
875875
}
876876

877877
@Test

0 commit comments

Comments
 (0)