Bug 1299373. Scripts with an empty src attribute should not execute. r=bkelly
authorBoris Zbarsky <bzbarsky@mit.edu>
Wed, 07 Sep 2016 14:21:36 -0400
changeset 313082 8edc64f3a6e40c5f8aa67ed2255c49a4a1a2a386
parent 313081 ff96cb069ef2ca380d679b9b9a7dd2f7d4919d2e
child 313083 0e851843cf5a65f8570dbc3f99ab16243b2bb307
push id20479
push userkwierso@gmail.com
push dateThu, 08 Sep 2016 01:08:46 +0000
treeherderfx-team@fb7c6b034329 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbkelly
bugs1299373
milestone51.0a1
Bug 1299373. Scripts with an empty src attribute should not execute. r=bkelly
dom/html/HTMLScriptElement.cpp
dom/svg/SVGScriptElement.cpp
testing/web-platform/meta/html/semantics/scripting-1/the-script-element/fetch-src/empty-with-base.html.ini
testing/web-platform/meta/html/semantics/scripting-1/the-script-element/fetch-src/empty.html.ini
testing/web-platform/meta/old-tests/submission/Opera/script_scheduling/137.html.ini
--- a/dom/html/HTMLScriptElement.cpp
+++ b/dom/html/HTMLScriptElement.cpp
@@ -134,16 +134,20 @@ void
 HTMLScriptElement::SetText(const nsAString& aValue, ErrorResult& rv)
 {
   rv = nsContentUtils::SetNodeTextContent(this, aValue, true);
 }
 
 
 NS_IMPL_STRING_ATTR(HTMLScriptElement, Charset, charset)
 NS_IMPL_BOOL_ATTR(HTMLScriptElement, Defer, defer)
+// If this ever gets changed to return "" if the attr value is "" (see
+// https://github.com/whatwg/html/issues/1739 for why it might not get changed),
+// it may be worth it to use GetSrc instead of GetAttr and manual
+// NewURIWithDocumentCharset in FreezeUriAsyncDefer.
 NS_IMPL_URI_ATTR(HTMLScriptElement, Src, src)
 NS_IMPL_STRING_ATTR(HTMLScriptElement, Type, type)
 NS_IMPL_STRING_ATTR(HTMLScriptElement, HtmlFor, _for)
 NS_IMPL_STRING_ATTR(HTMLScriptElement, Event, event)
 
 void
 HTMLScriptElement::SetCharset(const nsAString& aCharset, ErrorResult& rv)
 {
@@ -265,21 +269,27 @@ HTMLScriptElement::GetScriptCharset(nsAS
 void
 HTMLScriptElement::FreezeUriAsyncDefer()
 {
   if (mFrozen) {
     return;
   }
 
   // variation of this code in nsSVGScriptElement - check if changes
-  // need to be transfered when modifying
-  if (HasAttr(kNameSpaceID_None, nsGkAtoms::src)) {
-    nsAutoString src;
-    GetSrc(src);
-    NS_NewURI(getter_AddRefs(mUri), src);
+  // need to be transfered when modifying.  Note that we don't use GetSrc here
+  // because it will return the base URL when the attr value is "".
+  nsAutoString src;
+  if (GetAttr(kNameSpaceID_None, nsGkAtoms::src, src)) {
+    // Empty src should be treated as invalid URL.
+    if (!src.IsEmpty()) {
+      nsCOMPtr<nsIURI> baseURI = GetBaseURI();
+      nsContentUtils::NewURIWithDocumentCharset(getter_AddRefs(mUri),
+                                                src, OwnerDoc(), baseURI);
+    }
+
     // At this point mUri will be null for invalid URLs.
     mExternal = true;
 
     bool defer, async;
     GetAsync(&async);
     GetDefer(&defer);
 
     mDefer = !async && defer;
--- a/dom/svg/SVGScriptElement.cpp
+++ b/dom/svg/SVGScriptElement.cpp
@@ -149,18 +149,22 @@ SVGScriptElement::FreezeUriAsyncDefer()
     // need to be transfered when modifying
     nsAutoString src;
     if (mStringAttributes[HREF].IsExplicitlySet()) {
       mStringAttributes[HREF].GetAnimValue(src, this);
     } else {
       mStringAttributes[XLINK_HREF].GetAnimValue(src, this);
     }
 
-    nsCOMPtr<nsIURI> baseURI = GetBaseURI();
-    NS_NewURI(getter_AddRefs(mUri), src, nullptr, baseURI);
+    // Empty src should be treated as invalid URL.
+    if (!src.IsEmpty()) {
+      nsCOMPtr<nsIURI> baseURI = GetBaseURI();
+      NS_NewURI(getter_AddRefs(mUri), src, nullptr, baseURI);
+    }
+
     // At this point mUri will be null for invalid URLs.
     mExternal = true;
   }
 
   mFrozen = true;
 }
 
 //----------------------------------------------------------------------
deleted file mode 100644
--- a/testing/web-platform/meta/html/semantics/scripting-1/the-script-element/fetch-src/empty-with-base.html.ini
+++ /dev/null
@@ -1,5 +0,0 @@
-[empty-with-base.html]
-  type: testharness
-  [Script src with an empty URL]
-    expected: FAIL
-
deleted file mode 100644
--- a/testing/web-platform/meta/html/semantics/scripting-1/the-script-element/fetch-src/empty.html.ini
+++ /dev/null
@@ -1,5 +0,0 @@
-[empty.html]
-  type: testharness
-  [Script src with an empty URL]
-    expected: FAIL
-
deleted file mode 100644
--- a/testing/web-platform/meta/old-tests/submission/Opera/script_scheduling/137.html.ini
+++ /dev/null
@@ -1,3 +0,0 @@
-[137.html]
-  type: testharness
-  expected: ERROR