diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog index a6ae6775e89e..a4ec623ae5e1 100644 --- a/LayoutTests/ChangeLog +++ b/LayoutTests/ChangeLog @@ -1,3 +1,15 @@ +2019-06-26 Ryosuke Niwa + + ReplacementFragment should not have script observable side effects + https://bugs.webkit.org/show_bug.cgi?id=199147 + + Reviewed by Wenson Hsieh. + + Added regression tests. + + * editing/pasteboard/paste-contents-with-side-effects-expected.txt: Added. + * editing/pasteboard/paste-contents-with-side-effects.html: Added. + 2019-06-03 Yusuke Suzuki [JSC] JSObject::attemptToInterceptPutByIndexOnHole should use getPrototype instead of getPrototypeDirect diff --git a/LayoutTests/editing/pasteboard/paste-contents-with-side-effects-expected.txt b/LayoutTests/editing/pasteboard/paste-contents-with-side-effects-expected.txt new file mode 100644 index 000000000000..13e14edf7986 --- /dev/null +++ b/LayoutTests/editing/pasteboard/paste-contents-with-side-effects-expected.txt @@ -0,0 +1,22 @@ +This tests inserting content with an event handler. WebKit should never execute event handlers. + +On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE". + + + +Inserting with load event handler +PASS event.dataTransfer.getData('text/html').includes('onload="') is true +PASS event.dataTransfer.getData('text/html').includes('onmouseover="') is true +PASS didExecute is false + +Inserting with script element +PASS event.dataTransfer.getData('text/html').includes('script') is true +PASS didExecute is false + +Inserting iframe with a name into plaintext-only +PASS event.dataTransfer.getData("text/html").includes("iframe name=") is true +PASS didExecute is false +PASS successfullyParsed is true + +TEST COMPLETE + diff --git a/LayoutTests/editing/pasteboard/paste-contents-with-side-effects.html b/LayoutTests/editing/pasteboard/paste-contents-with-side-effects.html new file mode 100644 index 000000000000..9893093f4cae --- /dev/null +++ b/LayoutTests/editing/pasteboard/paste-contents-with-side-effects.html @@ -0,0 +1,67 @@ + + + + +
+ + + diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog index 521b29427307..530d1b6ffca9 100644 --- a/Source/WebCore/ChangeLog +++ b/Source/WebCore/ChangeLog @@ -1,3 +1,31 @@ +2019-06-26 Ryosuke Niwa + + ReplacementFragment should not have script observable side effects + https://bugs.webkit.org/show_bug.cgi?id=199147 + + Reviewed by Wenson Hsieh. + + Fixed the bug that ReplacementFragment has script observable side effects. + + Use a brand new document for sanitization where the script is disabled for test rendering, + and remove style and script elements as well as event handlers before the test rendering + and the actual pasting. + + Test: editing/pasteboard/paste-contents-with-side-effects.html + + * editing/ReplaceSelectionCommand.cpp: + (WebCore::ReplacementFragment::document): Deleted. + (WebCore::ReplacementFragment::ReplacementFragment): Use createPageForSanitizingWebContent + to create our own document for test rendering. We need to copy over the computed style + from the root editable element (editing host) to respect whitespace treatment, etc... + (WebCore::ReplacementFragment::removeContentsWithSideEffects): Moved from removeHeadContents. + Now removes event handlers and JavaScript URLs. + (WebCore::ReplacementFragment::insertFragmentForTestRendering): Renamed variable names. + (WebCore::ReplaceSelectionCommand::willApplyCommand): Create the plain text and HTML markup + for beforeinput and input events before ReplacementFragment removes contents with side effects. + (WebCore::ReplaceSelectionCommand::ensureReplacementFragment): The removal of head elements + is now done in ReplacementFragment's constructor. + 2019-06-25 Keith Miller Add didBecomePrototype() calls to global context prototypes diff --git a/Source/WebCore/editing/ReplaceSelectionCommand.cpp b/Source/WebCore/editing/ReplaceSelectionCommand.cpp index d5eced5825a0..6b8db856f274 100644 --- a/Source/WebCore/editing/ReplaceSelectionCommand.cpp +++ b/Source/WebCore/editing/ReplaceSelectionCommand.cpp @@ -31,6 +31,7 @@ #include "ApplyStyleCommand.h" #include "BeforeTextInsertedEvent.h" #include "BreakBlockquoteCommand.h" +#include "CSSComputedStyleDeclaration.h" #include "CSSStyleDeclaration.h" #include "DOMWrapperWorld.h" #include "DataTransfer.h" @@ -43,6 +44,7 @@ #include "FrameSelection.h" #include "HTMLBRElement.h" #include "HTMLBaseElement.h" +#include "HTMLBodyElement.h" #include "HTMLInputElement.h" #include "HTMLLIElement.h" #include "HTMLLinkElement.h" @@ -54,6 +56,7 @@ #include "NodeRenderStyle.h" #include "RenderInline.h" #include "RenderText.h" +#include "ScriptElement.h" #include "SimplifyMarkupCommand.h" #include "SmartReplace.h" #include "StyleProperties.h" @@ -70,15 +73,13 @@ using namespace HTMLNames; enum EFragmentType { EmptyFragment, SingleTextNodeFragment, TreeFragment }; -static void removeHeadContents(ReplacementFragment&); - // --- ReplacementFragment helper class class ReplacementFragment { WTF_MAKE_FAST_ALLOCATED; WTF_MAKE_NONCOPYABLE(ReplacementFragment); public: - ReplacementFragment(Document&, DocumentFragment*, const VisibleSelection&); + ReplacementFragment(DocumentFragment*, const VisibleSelection&); DocumentFragment* fragment() { return m_fragment.get(); } @@ -94,6 +95,7 @@ class ReplacementFragment { void removeNodePreservingChildren(Node&); private: + void removeContentsWithSideEffects(); Ref insertFragmentForTestRendering(Node* rootEditableNode); void removeUnrenderedNodes(Node*); void restoreAndRemoveTestRenderingNodesToFragment(StyledElement*); @@ -101,9 +103,6 @@ class ReplacementFragment { void insertNodeBefore(Node&, Node& refNode); - Document& document() { return *m_document; } - - RefPtr m_document; RefPtr m_fragment; bool m_hasInterchangeNewlineAtStart; bool m_hasInterchangeNewlineAtEnd; @@ -151,9 +150,8 @@ static Position positionAvoidingPrecedingNodes(Position position) return position; } -ReplacementFragment::ReplacementFragment(Document& document, DocumentFragment* fragment, const VisibleSelection& selection) - : m_document(&document) - , m_fragment(fragment) +ReplacementFragment::ReplacementFragment(DocumentFragment* fragment, const VisibleSelection& selection) + : m_fragment(fragment) , m_hasInterchangeNewlineAtStart(false) , m_hasInterchangeNewlineAtEnd(false) { @@ -161,12 +159,14 @@ ReplacementFragment::ReplacementFragment(Document& document, DocumentFragment* f return; if (!m_fragment->firstChild()) return; - + + removeContentsWithSideEffects(); + RefPtr editableRoot = selection.rootEditableElement(); ASSERT(editableRoot); if (!editableRoot) return; - + auto* shadowHost = editableRoot->shadowHost(); if (!editableRoot->attributeEventListener(eventNames().webkitBeforeTextInsertedEvent, mainThreadNormalWorld()) && !(shadowHost && shadowHost->renderer() && shadowHost->renderer()->isTextControl()) @@ -175,7 +175,14 @@ ReplacementFragment::ReplacementFragment(Document& document, DocumentFragment* f return; } - RefPtr holder = insertFragmentForTestRendering(editableRoot.get()); + auto page = createPageForSanitizingWebContent(); + Document* stagingDocument = page->mainFrame().document(); + ASSERT(stagingDocument->body()); + + ComputedStyleExtractor computedStyleOfEditableRoot(editableRoot.get()); + stagingDocument->body()->setAttributeWithoutSynchronization(styleAttr, computedStyleOfEditableRoot.copyProperties()->asText()); + + RefPtr holder = insertFragmentForTestRendering(stagingDocument->body()); if (!holder) { removeInterchangeNodes(m_fragment.get()); return; @@ -202,13 +209,44 @@ ReplacementFragment::ReplacementFragment(Document& document, DocumentFragment* f if (!m_fragment->firstChild()) return; - holder = insertFragmentForTestRendering(editableRoot.get()); + holder = insertFragmentForTestRendering(stagingDocument->body()); removeInterchangeNodes(holder.get()); removeUnrenderedNodes(holder.get()); restoreAndRemoveTestRenderingNodesToFragment(holder.get()); } } +void ReplacementFragment::removeContentsWithSideEffects() +{ + Vector> elementsToRemove; + Vector, QualifiedName>> attributesToRemove; + + auto it = descendantsOfType(*m_fragment).begin(); + auto end = descendantsOfType(*m_fragment).end(); + while (it != end) { + auto element = makeRef(*it); + if (isScriptElement(element) || (is(element) && element->getAttribute(classAttr) != WebKitMSOListQuirksStyle) + || is(element) || is(element) || is(element) || is(element)) { + elementsToRemove.append(WTFMove(element)); + it.traverseNextSkippingChildren(); + continue; + } + if (element->hasAttributes()) { + for (auto& attribute : element->attributesIterator()) { + if (element->isEventHandlerAttribute(attribute) || element->isJavaScriptURLAttribute(attribute)) + attributesToRemove.append({ element.copyRef(), attribute.name() }); + } + } + ++it; + } + + for (auto& element : elementsToRemove) + removeNode(WTFMove(element)); + + for (auto& item : attributesToRemove) + item.first->removeAttribute(item.second); +} + bool ReplacementFragment::isEmpty() const { return (!m_fragment || !m_fragment->firstChild()) && !m_hasInterchangeNewlineAtStart && !m_hasInterchangeNewlineAtEnd; @@ -252,13 +290,14 @@ void ReplacementFragment::insertNodeBefore(Node& node, Node& refNode) parent->insertBefore(node, &refNode); } -Ref ReplacementFragment::insertFragmentForTestRendering(Node* rootEditableElement) +Ref ReplacementFragment::insertFragmentForTestRendering(Node* rootNode) { - auto holder = createDefaultParagraphElement(document()); + auto document = makeRef(rootNode->document()); + auto holder = createDefaultParagraphElement(document.get()); holder->appendChild(*m_fragment); - rootEditableElement->appendChild(holder); - document().updateLayoutIgnorePendingStylesheets(); + rootNode->appendChild(holder); + document->updateLayoutIgnorePendingStylesheets(); return holder; } @@ -726,29 +765,6 @@ VisiblePosition ReplaceSelectionCommand::positionAtStartOfInsertedContent() cons return m_startOfInsertedContent; } -static void removeHeadContents(ReplacementFragment& fragment) -{ - if (fragment.isEmpty()) - return; - - Vector toRemove; - - auto it = descendantsOfType(*fragment.fragment()).begin(); - auto end = descendantsOfType(*fragment.fragment()).end(); - while (it != end) { - if (is(*it) || is(*it) || is(*it) || is(*it) - || (is(*it) && it->getAttribute(classAttr) != WebKitMSOListQuirksStyle)) { - toRemove.append(&*it); - it.traverseNextSkippingChildren(); - continue; - } - ++it; - } - - for (auto& element : toRemove) - fragment.removeNode(*element); -} - // Remove style spans before insertion if they are unnecessary. It's faster because we'll // avoid doing a layout. static bool handleStyleSpansBeforeInsertion(ReplacementFragment& fragment, const Position& insertionPos) @@ -919,9 +935,9 @@ inline Node* nodeToSplitToAvoidPastingIntoInlineNodesWithStyle(const Position& i bool ReplaceSelectionCommand::willApplyCommand() { - ensureReplacementFragment(); m_documentFragmentPlainText = m_documentFragment->textContent(); m_documentFragmentHTMLMarkup = serializeFragment(*m_documentFragment, SerializedNodes::SubtreeIncludingNode); + ensureReplacementFragment(); return CompositeEditCommand::willApplyCommand(); } @@ -1544,11 +1560,8 @@ void ReplaceSelectionCommand::updateNodesInserted(Node *node) ReplacementFragment* ReplaceSelectionCommand::ensureReplacementFragment() { - if (!m_replacementFragment) { - m_replacementFragment = std::make_unique(document(), m_documentFragment.get(), endingSelection()); - removeHeadContents(*m_replacementFragment); - } - + if (!m_replacementFragment) + m_replacementFragment = std::make_unique(m_documentFragment.get(), endingSelection()); return m_replacementFragment.get(); }