Escape '&' in javascript URLs for innerHTML/outerHTML

Escape '&' in javascript URLs for innerHTML/outerHTML https://bugs.webkit.org/show_bug.cgi?id=249576 Reviewed by Ryosuke Niwa. This patch is to align WebKit with Blink / Chromium and Gecko / Firefox. Merge - https://src.chromium.org/viewvc/blink?revision=192539&view=revision It is to add special handling with HTML escape when serializing URL is already implemented for quote. Added implementation to replace '&' by '&'. * Source/WebCore/editing/MarkupAccumulator.cpp: (MarkupAccumulator::appendQuotedURLAttributeValue): Add condition to escape "&" * LayoutTests/fast/innerHTML/javascript-url.html: Updated * LayoutTests/fast/innerHTML/javascript-url-expected.txt: Updated Expectations Canonical link: https://commits.webkit.org/258112@main
WebKit · Dec 20, 2022 · aff702358ff43fa9220b4e72da9bc3bab8c117b7 · aff7023
1 parent 79b9110
commit aff702358ff43fa9220b4e72da9bc3bab8c117b7
Show file tree

Hide file tree

Showing 3 changed files with 34 additions and 69 deletions.
diff --git a/LayoutTests/fast/innerHTML/javascript-url-expected.txt b/LayoutTests/fast/innerHTML/javascript-url-expected.txt
@@ -1,13 +1,12 @@
-Test that innerHTML does not mangle javascript: urls.
-r.innerHTML = r.innerHTML.replace(/&37;3C!--D--&37;3E/g, 123)
-PASS: r.innerHTML.indexOf('javascript:test(123)') > -1 should be true and is.
-r.firstChild.setAttribute('href', 'javascript:test("text<")')
-PASS: r.innerHTML.indexOf('javascript:test("text<")') > -1 should be true and is.
-r.firstChild.setAttribute("href", "javascript:test('text>')")
-PASS: r.innerHTML.indexOf("javascript:test('text>')") > -1 should be true and is.
-testString = javascript:test('text&',"test2&")
-r.firstChild.setAttribute("href", testString)
-PASS: r.innerHTML.indexOf("javascript:test('text&',&quot;test2&&quot;)") > 1 should be true and is.
-r.firstChild.setAttribute('href', 'http://www.google.fi/search?q=scarlett johansson&meta=&btnG=Google-haku')
-<a href="http://www.google.fi/search?q=scarlett johansson&amp;meta=&amp;btnG=Google-haku">link</a>
+Test that innerHTML/outerHTML does not mangle javascript: urls.
+PASS r.innerHTML is "<a href=\"javascript:test(123)\">link</a>"
+PASS r.innerHTML is "<a href='javascript:test(\"text<\")'>link</a>"
+PASS r.innerHTML is "<a href=\"javascript:test('text>')\">link</a>"
+PASS r.innerHTML is "<a href=\"javascript:test('text&amp;',&quot;test2&amp;&quot;)\">link</a>"
+PASS r.firstChild.outerHTML is "<a href=\"javascript:window.location='?x&amp;y'\">link</a>"
+PASS r.firstChild.outerHTML is "<a href=\"javascript:window.location='?x&amp;amp;y'\">link</a>"
+PASS r.firstChild.outerHTML is "<a href=\"javascript:window.location='?x&amp;y'\">link</a>"
+PASS successfullyParsed is true
+
+TEST COMPLETE
 link
diff --git a/LayoutTests/fast/innerHTML/javascript-url.html b/LayoutTests/fast/innerHTML/javascript-url.html
@@ -1,62 +1,25 @@
-<head>
-<script>
-if (window.testRunner)
-    testRunner.dumpAsText();
-
-function print(message, color) 
-{
-    var paragraph = document.createElement("div");
-    paragraph.appendChild(document.createTextNode(message));
-    paragraph.style.fontFamily = "monospace";
-    if (color)
-        paragraph.style.color = color;
-    document.getElementById("console").appendChild(paragraph);
-}
-
-function run(a)
-{
-    print(a);
-    try {
-        eval(a);
-    } catch(e) {
-        print(e);
-    }
-}
-
-function shouldBe(a, b)
-{
-    var evalA;
-    try {
-        evalA = eval(a);
-    } catch(e) {
-        evalA = e;
-    }
-
-    if (evalA == b)
-        print("PASS: " + a + " should be " + b + " and is.", "green");
-    else
-        print("FAIL: " + a + " should be " + b + " but instead is " + evalA + ".", "red");
-}
-</script>
-</head>
+<!DOCTYPE html>
+<script src="../../resources/js-test.js"></script>
 <body>
-Test that innerHTML does not mangle javascript: urls.
+Test that innerHTML/outerHTML does not mangle javascript: urls.
 <div id=console></div>
-<div id=jsurltest><a href='
- javascript:test(&37;3C!--D--&37;3E)'>link</a></div>
+<div id=jsurltest><a href='javascript:test(&37;3C!--D--&37;3E)'>link</a></div>
 <script>
 var r = document.getElementById('jsurltest');
-run("r.innerHTML = r.innerHTML.replace(/&37;3C!--D--&37;3E/g, 123)");
-shouldBe("r.innerHTML.indexOf('javascript:test(123)') > -1", true);
-run("r.firstChild.setAttribute('href', 'javascript:test(\"text<\")')");
-shouldBe("r.innerHTML.indexOf('javascript:test(\"text<\")') > -1", true);
-run('r.firstChild.setAttribute("href", "javascript:test(\'text>\')")');
-shouldBe('r.innerHTML.indexOf("javascript:test(\'text>\')") > -1', true);
+r.innerHTML = r.innerHTML.replace('&amp;37;3C!--D--&amp;37;3E', 123);
+shouldBeEqualToString('r.innerHTML', '<a href="javascript:test(123)">link</a>');
+r.firstChild.setAttribute('href', 'javascript:test(\"text<\")');
+shouldBeEqualToString('r.innerHTML', '<a href=\'javascript:test("text<")\'>link</a>');
+r.firstChild.setAttribute('href', 'javascript:test(\'text>\')');
+shouldBeEqualToString("r.innerHTML", '<a href="javascript:test(\'text>\')">link</a>');
 testString = 'javascript:test(\'text&\',"test2&")';
-print("testString = " + testString);
-run('r.firstChild.setAttribute("href", testString)');
-shouldBe('r.innerHTML.indexOf("javascript:test(\'text&\',&quot;test2&&quot;)") > 1', true);
-
-run("r.firstChild.setAttribute('href', 'http://www.google.fi/search?q=scarlett johansson&meta=&btnG=Google-haku')");
-print(r.innerHTML);
+r.firstChild.setAttribute('href', testString);
+shouldBeEqualToString('r.innerHTML', '<a href="javascript:test(\'text&amp;\',&quot;test2&amp;&quot;)">link</a>');
+r.firstChild.href = 'javascript:window.location=\'?x&y\'';
+shouldBeEqualToString("r.firstChild.outerHTML", '<a href="javascript:window.location=\'?x&amp;y\'">link</a>');
+// Behavior is same as FF
+r.firstChild.href = 'javascript:window.location=\'?x&amp;y\'';
+shouldBeEqualToString('r.firstChild.outerHTML', '<a href="javascript:window.location=\'?x&amp;amp;y\'">link</a>');
+r.innerHTML = '<a href="javascript:window.location=\'?x&amp;y\'">link</a>';
+shouldBeEqualToString('r.firstChild.outerHTML', '<a href="javascript:window.location=\'?x&amp;y\'">link</a>');
 </script>
diff --git a/Source/WebCore/editing/MarkupAccumulator.cpp b/Source/WebCore/editing/MarkupAccumulator.cpp
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2004-2017 Apple Inc. All rights reserved.
+ * Copyright (C) 2004-2022 Apple Inc. All rights reserved.
  * Copyright (C) 2009-2022 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
@@ -312,6 +312,9 @@ void MarkupAccumulator::appendQuotedURLAttributeValue(StringBuilder& result, con
     char quoteChar = '"';
     if (WTF::protocolIsJavaScript(resolvedURLString)) {
         // minimal escaping for javascript urls
+        if (resolvedURLString.contains('&'))
+            resolvedURLString = makeStringByReplacingAll(resolvedURLString, '&', "&amp;"_s);
+
         if (resolvedURLString.contains('"')) {
             if (resolvedURLString.contains('\''))
                 resolvedURLString = makeStringByReplacingAll(resolvedURLString, '"', "&quot;"_s);