Document.currentScript does not work for SVGScriptElements
authorweinig@apple.com <weinig@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 12 Jun 2020 14:28:43 +0000 (14:28 +0000)
committerweinig@apple.com <weinig@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 12 Jun 2020 14:28:43 +0000 (14:28 +0000)
https://bugs.webkit.org/show_bug.cgi?id=213104

Reviewed by Yusuke Suzuki.

LayoutTests/imported/w3c:

* web-platform-tests/html/dom/documents/dom-tree-accessors/Document.currentScript-expected.txt:
Update test results after making currentScript work with SVGScriptElements.

* web-platform-tests/content-security-policy/nonce-hiding/svgscript-nonces-hidden-meta.tentative.sub-expected.txt:
* web-platform-tests/content-security-policy/nonce-hiding/svgscript-nonces-hidden.tentative-expected.txt:
Update results. Not passing, but getting further.

Source/WebCore:

Updates results for existing tests.

* WebCore.xcodeproj/project.pbxproj:
Add CurrentScriptIncrementer.h to the Xcode project as it was missing.

* dom/CurrentScriptIncrementer.h:
(WebCore::CurrentScriptIncrementer::CurrentScriptIncrementer):
(WebCore::CurrentScriptIncrementer::~CurrentScriptIncrementer):
Re-work using ScriptElement, removing the HTMLScriptElement checks. Also changes
scriptType check to explicitly check that against classic scripts, as they are
the only supported type, and if any types other than modules are added in the
future, we would not want this to change behavior.

* dom/Document.cpp:
(WebCore::Document::pushCurrentScript):
* dom/Document.h:
(WebCore::Document::currentScript const):
Use an Element, rather than an HTMLScriptElement for currentScript/currentScriptStack
so that either an HTMLScriptElement or an SVGScriptElement can be stored. Using a
ScriptElement would be possible, but would complicate the implementation unnecessarily
by requiring currentScript to have an additional checks before extracting the Element.
Variant<RefPtr<HTMLScriptElement>, RefPtr<SVGScriptElement>> could also have been used
but also would unnecessarily complicated the interface and caused more memory to be used.

* dom/Document.idl:
Update interface to use Element rather HTMLScriptElement with a comment explaining why we
are not using HTMLOrSVGScriptElement but retaining the same observable behavior.

* dom/ScriptElement.cpp:
(WebCore::ScriptElement::executeClassicScript):
(WebCore::ScriptElement::executeModuleScript):
Pass *this directly to CurrentScriptIncrementer to simplify implementation.

Source/WebKit:

* WebProcess/InjectedBundle/API/gtk/DOM/WebKitDOMDocumentGtk.cpp:
(webkit_dom_document_get_current_script):
Update to account for change in Document::currentScript() now returning an
Element* that can be either an HTMLScriptElement or an SVGScriptElement. To
keep API compatibility, only return non-null for HTMLScriptElements.

Source/WebKitLegacy/mac:

* DOM/DOMDocument.mm:
(-[DOMDocument currentScript]):
Update to account for change in Document::currentScript() now returning an
Element* that can be either an HTMLScriptElement or an SVGScriptElement. To
keep API compatibility, only return non-null for HTMLScriptElements.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@262945 268f45cc-cd09-0410-ab3c-d52691b4dbfc

15 files changed:
LayoutTests/imported/w3c/ChangeLog
LayoutTests/imported/w3c/web-platform-tests/content-security-policy/nonce-hiding/svgscript-nonces-hidden-meta.tentative.sub-expected.txt
LayoutTests/imported/w3c/web-platform-tests/content-security-policy/nonce-hiding/svgscript-nonces-hidden.tentative-expected.txt
LayoutTests/imported/w3c/web-platform-tests/html/dom/documents/dom-tree-accessors/Document.currentScript-expected.txt
Source/WebCore/ChangeLog
Source/WebCore/WebCore.xcodeproj/project.pbxproj
Source/WebCore/dom/CurrentScriptIncrementer.h
Source/WebCore/dom/Document.cpp
Source/WebCore/dom/Document.h
Source/WebCore/dom/Document.idl
Source/WebCore/dom/ScriptElement.cpp
Source/WebKit/ChangeLog
Source/WebKit/WebProcess/InjectedBundle/API/gtk/DOM/WebKitDOMDocumentGtk.cpp
Source/WebKitLegacy/mac/ChangeLog
Source/WebKitLegacy/mac/DOM/DOMDocument.mm

index bf37c72..7b013ce 100644 (file)
@@ -1,3 +1,17 @@
+2020-06-11  Sam Weinig  <weinig@apple.com>
+
+        Document.currentScript does not work for SVGScriptElements
+        https://bugs.webkit.org/show_bug.cgi?id=213104
+
+        Reviewed by Yusuke Suzuki.
+
+        * web-platform-tests/html/dom/documents/dom-tree-accessors/Document.currentScript-expected.txt:
+        Update test results after making currentScript work with SVGScriptElements.
+
+        * web-platform-tests/content-security-policy/nonce-hiding/svgscript-nonces-hidden-meta.tentative.sub-expected.txt:
+        * web-platform-tests/content-security-policy/nonce-hiding/svgscript-nonces-hidden.tentative-expected.txt:
+        Update results. Not passing, but getting further. 
+
 2020-06-11  Oriol Brufau  <obrufau@igalia.com>
 
         [css-grid] Reimport WPT tests removed in r262809
index a4e9d0a..8ccb7ca 100644 (file)
@@ -1,4 +1,13 @@
+ undefined
 
-Harness Error (FAIL), message = TypeError: null is not an object (evaluating 'document.currentScript.setAttribute')
+FAIL Reading 'nonce' content attribute and IDL attribute. assert_equals: expected (string) "abc" but got (undefined) undefined
+FAIL Cloned node retains nonce. assert_equals: IDL attribute expected (string) "abc" but got (undefined) undefined
+FAIL Cloned node retains nonce when inserted. assert_equals: expected (string) "abc" but got (undefined) undefined
+FAIL Writing 'nonce' content attribute. assert_equals: expected (string) "abc" but got (undefined) undefined
+PASS Writing 'nonce' IDL attribute. 
+PASS Document-written script executes. 
+FAIL Document-written script's nonce value. assert_equals: expected (string) "abc" but got (undefined) undefined
+PASS createElement.nonce. 
+PASS createElement.setAttribute. 
+PASS Nonces don't leak via CSS side-channels. 
 
index a4e9d0a..871bf35 100644 (file)
@@ -1,4 +1,14 @@
 
-Harness Error (FAIL), message = TypeError: null is not an object (evaluating 'document.currentScript.setAttribute')
+FAIL Reading 'nonce' content attribute and IDL attribute. assert_equals: expected Element node <script nonce="abc" id="testScript" executed="yay">
+    d... but got null
+FAIL Cloned node retains nonce. assert_equals: IDL attribute expected (string) "abc" but got (undefined) undefined
+FAIL Cloned node retains nonce when inserted. assert_equals: expected (string) "abc" but got (undefined) undefined
+FAIL Writing 'nonce' content attribute. assert_equals: expected (string) "abc" but got (undefined) undefined
+PASS Writing 'nonce' IDL attribute. 
+PASS Document-written script executes. 
+FAIL Document-written script's nonce value. assert_equals: expected "" but got "abc"
+FAIL createElement.nonce. assert_equals: expected (object) null but got (string) "abc"
+FAIL createElement.setAttribute. assert_equals: Post-insertion content expected "" but got "abc"
+FAIL Nonces don't leak via CSS side-channels. assert_equals: expected "none" but got "url(\"http://localhost:8800/security/resources/abe.png\")"
 
index a3a661c..9bdbe92 100644 (file)
@@ -13,11 +13,7 @@ PASS Script script-window-error
 PASS Script timeout 
 PASS Script eval 
 PASS Script xhr-test 
-FAIL Script script-svg assert_array_equals: expected property 0 to be Element node <script id="script-svg">
-    verify('script-svg');
-    fi... but got null (expected array [Element node <script id="script-svg">
-    verify('script-svg');
-    fi...] got [null])
+PASS Script script-svg 
 PASS Script script-async 
 PASS Script script-defer 
 PASS Script script-async-false 
index 6f14d0c..cbde62a 100644 (file)
@@ -1,3 +1,43 @@
+2020-06-11  Sam Weinig  <weinig@apple.com>
+
+        Document.currentScript does not work for SVGScriptElements
+        https://bugs.webkit.org/show_bug.cgi?id=213104
+
+        Reviewed by Yusuke Suzuki.
+
+        Updates results for existing tests.
+
+        * WebCore.xcodeproj/project.pbxproj:
+        Add CurrentScriptIncrementer.h to the Xcode project as it was missing.
+
+        * dom/CurrentScriptIncrementer.h:
+        (WebCore::CurrentScriptIncrementer::CurrentScriptIncrementer):
+        (WebCore::CurrentScriptIncrementer::~CurrentScriptIncrementer):
+        Re-work using ScriptElement, removing the HTMLScriptElement checks. Also changes
+        scriptType check to explicitly check that against classic scripts, as they are
+        the only supported type, and if any types other than modules are added in the 
+        future, we would not want this to change behavior.
+
+        * dom/Document.cpp:
+        (WebCore::Document::pushCurrentScript):
+        * dom/Document.h:
+        (WebCore::Document::currentScript const):
+        Use an Element, rather than an HTMLScriptElement for currentScript/currentScriptStack
+        so that either an HTMLScriptElement or an SVGScriptElement can be stored. Using a 
+        ScriptElement would be possible, but would complicate the implementation unnecessarily
+        by requiring currentScript to have an additional checks before extracting the Element.
+        Variant<RefPtr<HTMLScriptElement>, RefPtr<SVGScriptElement>> could also have been used
+        but also would unnecessarily complicated the interface and caused more memory to be used. 
+
+        * dom/Document.idl:
+        Update interface to use Element rather HTMLScriptElement with a comment explaining why we
+        are not using HTMLOrSVGScriptElement but retaining the same observable behavior.
+
+        * dom/ScriptElement.cpp:
+        (WebCore::ScriptElement::executeClassicScript):
+        (WebCore::ScriptElement::executeModuleScript):
+        Pass *this directly to CurrentScriptIncrementer to simplify implementation.
+
 2020-06-12  Carlos Garcia Campos  <cgarcia@igalia.com>
 
         [GTK4] Make WebDriver work
index 05b90f5..deb4716 100644 (file)
                7CEB57EA1F95651500097AEC /* Settings.cpp */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.cpp; name = Settings.cpp; path = DerivedSources/WebCore/Settings.cpp; sourceTree = BUILT_PRODUCTS_DIR; };
                7CF158991EBBCDC700D4BFB7 /* SubresourceIntegrity.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = SubresourceIntegrity.cpp; sourceTree = "<group>"; };
                7CF1589A1EBBCDC700D4BFB7 /* SubresourceIntegrity.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = SubresourceIntegrity.h; sourceTree = "<group>"; };
+               7CF570C62492BD49008EB33C /* CurrentScriptIncrementer.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = CurrentScriptIncrementer.h; sourceTree = "<group>"; };
                7CF930E61E01F9AD00BAFFBE /* PaymentHeaders.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = PaymentHeaders.h; sourceTree = "<group>"; };
                7CFDC57A1AC1D80500E24A57 /* ContentExtensionError.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = ContentExtensionError.cpp; sourceTree = "<group>"; };
                7CFDC57B1AC1D80500E24A57 /* ContentExtensionError.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = ContentExtensionError.h; sourceTree = "<group>"; };
                                E1A1470711102B1500EEC0F3 /* ContainerNodeAlgorithms.h */,
                                97627B8B14FB3CEE002CDCA1 /* ContextDestructionObserver.cpp */,
                                97627B8C14FB3CEE002CDCA1 /* ContextDestructionObserver.h */,
+                               7CF570C62492BD49008EB33C /* CurrentScriptIncrementer.h */,
                                9B56C9A91C89329A00C456DF /* CustomElementReactionQueue.cpp */,
                                9B56C9A81C89312800C456DF /* CustomElementReactionQueue.h */,
                                9BD4E9181C462CFC005065BC /* CustomElementRegistry.cpp */,
index d089023..14d0500 100644 (file)
 #pragma once
 
 #include "Document.h"
-#include "HTMLScriptElement.h"
+#include "ScriptElement.h"
 
 namespace WebCore {
 
 class CurrentScriptIncrementer {
     WTF_MAKE_NONCOPYABLE(CurrentScriptIncrementer);
 public:
-    CurrentScriptIncrementer(Document& document, Element& element)
+    CurrentScriptIncrementer(Document& document, ScriptElement& scriptElement)
         : m_document(document)
-        , m_isHTMLScriptElement(is<HTMLScriptElement>(element))
     {
-        if (!m_isHTMLScriptElement)
-            return;
-        auto& scriptElement = downcast<HTMLScriptElement>(element);
-        bool shouldPushNullForCurrentScript = scriptElement.isInShadowTree() || scriptElement.scriptType() == ScriptElement::ScriptType::Module;
-        m_document.pushCurrentScript(shouldPushNullForCurrentScript ? nullptr : &scriptElement);
+        bool shouldPushNullForCurrentScript = scriptElement.element().isInShadowTree() || scriptElement.scriptType() != ScriptElement::ScriptType::Classic;
+        m_document.pushCurrentScript(shouldPushNullForCurrentScript ? nullptr : &scriptElement.element());
     }
 
     ~CurrentScriptIncrementer()
     {
-        if (!m_isHTMLScriptElement)
-            return;
         m_document.popCurrentScript();
     }
 
 private:
     Document& m_document;
-    bool m_isHTMLScriptElement;
 };
 
 } // namespace WebCore
index a18f1a6..ac92e68 100644 (file)
@@ -5590,7 +5590,7 @@ String Document::queryCommandValue(const String& commandName)
     return command(this, commandName).value();
 }
 
-void Document::pushCurrentScript(HTMLScriptElement* newCurrentScript)
+void Document::pushCurrentScript(Element* newCurrentScript)
 {
     m_currentScriptStack.append(newCurrentScript);
 }
index 7f20094..7eabf65 100644 (file)
@@ -148,7 +148,6 @@ class HTMLImageElement;
 class HTMLMapElement;
 class HTMLMediaElement;
 class HTMLVideoElement;
-class HTMLScriptElement;
 class HighlightMap;
 class HitTestLocation;
 class HitTestRequest;
@@ -187,7 +186,6 @@ class SVGDocumentExtensions;
 class SVGSVGElement;
 class SVGUseElement;
 class SWClientConnection;
-class ScriptElementData;
 class ScriptModuleLoader;
 class ScriptRunner;
 class ScriptableDocumentParser;
@@ -1038,8 +1036,8 @@ public:
     ScriptRunner& scriptRunner() { return *m_scriptRunner; }
     ScriptModuleLoader& moduleLoader() { return *m_moduleLoader; }
 
-    HTMLScriptElement* currentScript() const { return !m_currentScriptStack.isEmpty() ? m_currentScriptStack.last().get() : nullptr; }
-    void pushCurrentScript(HTMLScriptElement*);
+    Element* currentScript() const { return !m_currentScriptStack.isEmpty() ? m_currentScriptStack.last().get() : nullptr; }
+    void pushCurrentScript(Element*);
     void popCurrentScript();
 
     bool shouldDeferAsynchronousScriptsUntilParsingFinishes() const;
@@ -1791,7 +1789,7 @@ private:
     std::unique_ptr<ScriptRunner> m_scriptRunner;
     std::unique_ptr<ScriptModuleLoader> m_moduleLoader;
 
-    Vector<RefPtr<HTMLScriptElement>> m_currentScriptStack;
+    Vector<RefPtr<Element>> m_currentScriptStack;
 
 #if ENABLE(XSLT)
     void applyPendingXSLTransformsTimerFired();
index 7bd055c..81442ee 100644 (file)
@@ -31,6 +31,8 @@ typedef (
     ImageBitmapRenderingContext or 
     CanvasRenderingContext2D) RenderingContext;
 
+typedef (HTMLScriptElement or SVGScriptElement) HTMLOrSVGScriptElement;
+
 [
     Constructor,
     ConstructorCallWith=Document,
@@ -100,7 +102,10 @@ typedef (
     readonly attribute HTMLCollection forms; // Should be [SameObject].
     readonly attribute HTMLCollection scripts; // Should be [SameObject].
     NodeList getElementsByName([AtomString] DOMString elementName);
-    readonly attribute HTMLScriptElement? currentScript; // FIXME: Should return a HTMLOrSVGScriptElement.
+    
+    // currentScript is specified to use type HTMLOrSVGScriptElement?, but implemented using
+    // shared base type Element? to optimize implementation without an observable difference.
+    readonly attribute Element? currentScript;
 
     // dynamic markup insertion
     // FIXME: The HTML spec says this should consult the "responsible document". We should ensure
index fded040..05758b6 100644 (file)
@@ -36,6 +36,7 @@
 #include "FrameLoader.h"
 #include "HTMLNames.h"
 #include "HTMLParserIdioms.h"
+#include "HTMLScriptElement.h"
 #include "IgnoreDestructiveWriteCountIncrementer.h"
 #include "InlineClassicScript.h"
 #include "LoadableClassicScript.h"
@@ -393,7 +394,7 @@ void ScriptElement::executeClassicScript(const ScriptSourceCode& sourceCode)
         return;
 
     IgnoreDestructiveWriteCountIncrementer ignoreDesctructiveWriteCountIncrementer(m_isExternalScript ? &document : nullptr);
-    CurrentScriptIncrementer currentScriptIncrementer(document, m_element);
+    CurrentScriptIncrementer currentScriptIncrementer(document, *this);
 
     WTFBeginSignpost(this, "Execute Script Element", "executing classic script from URL: %{public}s async: %d defer: %d", m_isExternalScript ? sourceCode.url().string().utf8().data() : "inline", hasAsyncAttribute(), hasDeferAttribute());
     frame->script().evaluateIgnoringException(sourceCode);
@@ -412,7 +413,7 @@ void ScriptElement::executeModuleScript(LoadableModuleScript& loadableModuleScri
         return;
 
     IgnoreDestructiveWriteCountIncrementer ignoreDesctructiveWriteCountIncrementer(&document);
-    CurrentScriptIncrementer currentScriptIncrementer(document, m_element);
+    CurrentScriptIncrementer currentScriptIncrementer(document, *this);
 
     WTFBeginSignpost(this, "Execute Script Element", "executing module script");
     frame->script().linkAndEvaluateModuleScript(loadableModuleScript);
index 8aa4fe7..240c002 100644 (file)
@@ -1,3 +1,16 @@
+2020-06-11  Sam Weinig  <weinig@apple.com>
+
+        Document.currentScript does not work for SVGScriptElements
+        https://bugs.webkit.org/show_bug.cgi?id=213104
+
+        Reviewed by Yusuke Suzuki.
+
+        * WebProcess/InjectedBundle/API/gtk/DOM/WebKitDOMDocumentGtk.cpp:
+        (webkit_dom_document_get_current_script):
+        Update to account for change in Document::currentScript() now returning an
+        Element* that can be either an HTMLScriptElement or an SVGScriptElement. To
+        keep API compatibility, only return non-null for HTMLScriptElements.
+
 2020-06-12  Charlie Turner  <cturner@igalia.com>
 
         [GTK] Add an internal API to run javascript without forced user gestures
index ef6890c..05881a4 100644 (file)
@@ -1908,8 +1908,10 @@ WebKitDOMHTMLScriptElement* webkit_dom_document_get_current_script(WebKitDOMDocu
     WebCore::JSMainThreadNullState state;
     g_return_val_if_fail(WEBKIT_DOM_IS_DOCUMENT(self), 0);
     WebCore::Document* item = WebKit::core(self);
-    RefPtr<WebCore::HTMLScriptElement> gobjectResult = WTF::getPtr(item->currentScript());
-    return WebKit::kit(gobjectResult.get());
+    WebCore::Element* element = item->currentScript();
+    if (!is<WebCore::HTMLScriptElement>(element))
+        return nullptr;
+    return WebKit::kit(downcast<WebCore::HTMLScriptElement>(element));
 }
 
 gchar* webkit_dom_document_get_origin(WebKitDOMDocument* self)
index c77601e..7b66b3a 100644 (file)
@@ -1,3 +1,16 @@
+2020-06-11  Sam Weinig  <weinig@apple.com>
+
+        Document.currentScript does not work for SVGScriptElements
+        https://bugs.webkit.org/show_bug.cgi?id=213104
+
+        Reviewed by Yusuke Suzuki.
+
+        * DOM/DOMDocument.mm:
+        (-[DOMDocument currentScript]):
+        Update to account for change in Document::currentScript() now returning an
+        Element* that can be either an HTMLScriptElement or an SVGScriptElement. To
+        keep API compatibility, only return non-null for HTMLScriptElements.
+
 2020-06-12  Keith Rollin  <krollin@apple.com>
 
         Add dependencies for Migrate Headers and Generate Export Files build phases
index 0119d3b..dd85f97 100644 (file)
 - (DOMHTMLScriptElement *)currentScript
 {
     WebCore::JSMainThreadNullState state;
-    return kit(WTF::getPtr(IMPL->currentScript()));
+    WebCore::Element* element = IMPL->currentScript();
+    if (!is<WebCore::HTMLScriptElement>(element))
+        return nil;
+    return kit(WTF::getPtr(downcast<WebCore::HTMLScriptElement>(element)));
 }
 
 - (NSString *)origin