Skip to content
Permalink
Browse files
Reproducible crash removing name attribute from <img> node
<https://webkit.org/b/144371>
<rdar://problem/17198583>

Reviewed by Darin Adler.

Source/WebCore:

The problem here was with HTMLImageElement::getNameAttribute(), which relies
on Element::hasName() to avoid slow attribute lookups when the attribute
is already known not to be present. Unfortunately hasName() uses an ElementData
flag that wasn't getting updated until after the call to parseAttribute().

This patch fixes the issue by moving the code that updates the hasName() flag
before the parseAttribute() virtual dispatch.

Test: fast/dom/HTMLImageElement/remove-name-id-attribute-from-image.html

* dom/Element.cpp:
(WebCore::Element::attributeChanged):

LayoutTests:

* fast/dom/HTMLImageElement/remove-name-id-attribute-from-image-expected.txt: Added.
* fast/dom/HTMLImageElement/remove-name-id-attribute-from-image.html: Added.


Canonical link: https://commits.webkit.org/162543@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@183706 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information
Andreas Kling committed May 2, 2015
1 parent be107b0 commit 62c5bda092a1c25ed9cf5a0d0719d788f9454bdd
@@ -1,3 +1,14 @@
2015-05-01 Andreas Kling <akling@apple.com>

Reproducible crash removing name attribute from <img> node
<https://webkit.org/b/144371>
<rdar://problem/17198583>

Reviewed by Darin Adler.

* fast/dom/HTMLImageElement/remove-name-id-attribute-from-image-expected.txt: Added.
* fast/dom/HTMLImageElement/remove-name-id-attribute-from-image.html: Added.

2015-05-01 Eric Carlson <eric.carlson@apple.com>

Postpone caption style sheet creation
@@ -0,0 +1,14 @@
This test checks that there's no crash when removing the name or id attribute from a HTMLImageElement.

On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".


PASS doc.body.firstChild.getAttribute('name') is 'bar'
PASS doc.body.firstChild.getAttribute('id') is 'foo'
Removing name and id attributes...
PASS doc.body.firstChild.getAttribute('name') is null
PASS doc.body.firstChild.getAttribute('id') is null
PASS successfullyParsed is true

TEST COMPLETE

@@ -0,0 +1,25 @@
<!DOCTYPE html>
<html>
<head>
<meta charset="utf-8">
<script src="../../../resources/js-test-pre.js"></script>
</head>
<body>
<script>

description("This test checks that there's no crash when removing the name or id attribute from a HTMLImageElement.");

var doc = document.implementation.createHTMLDocument( '' );
doc.body.innerHTML = '<img name="bar" id="foo">';
shouldBe("doc.body.firstChild.getAttribute('name')", "'bar'");
shouldBe("doc.body.firstChild.getAttribute('id')", "'foo'");
debug("Removing name and id attributes...");
doc.body.firstChild.removeAttribute("name");
doc.body.firstChild.removeAttribute("id");
shouldBe("doc.body.firstChild.getAttribute('name')", "null");
shouldBe("doc.body.firstChild.getAttribute('id')", "null");

</script>
<script src="../../../resources/js-test-post.js"></script>
</body>
</html>
@@ -1,3 +1,24 @@
2015-05-01 Andreas Kling <akling@apple.com>

Reproducible crash removing name attribute from <img> node
<https://webkit.org/b/144371>
<rdar://problem/17198583>

Reviewed by Darin Adler.

The problem here was with HTMLImageElement::getNameAttribute(), which relies
on Element::hasName() to avoid slow attribute lookups when the attribute
is already known not to be present. Unfortunately hasName() uses an ElementData
flag that wasn't getting updated until after the call to parseAttribute().

This patch fixes the issue by moving the code that updates the hasName() flag
before the parseAttribute() virtual dispatch.

Test: fast/dom/HTMLImageElement/remove-name-id-attribute-from-image.html

* dom/Element.cpp:
(WebCore::Element::attributeChanged):

2015-05-01 Eric Carlson <eric.carlson@apple.com>

Postpone caption style sheet creation
@@ -1216,36 +1216,40 @@ static bool checkNeedsStyleInvalidationForIdChange(const AtomicString& oldId, co

void Element::attributeChanged(const QualifiedName& name, const AtomicString& oldValue, const AtomicString& newValue, AttributeModificationReason)
{
parseAttribute(name, newValue);

document().incDOMTreeVersion();

if (oldValue == newValue)
return;
bool valueIsSameAsBefore = oldValue == newValue;

StyleResolver* styleResolver = document().styleResolverIfExists();
bool testShouldInvalidateStyle = inRenderedDocument() && styleResolver && styleChangeType() < FullStyleChange;

bool shouldInvalidateStyle = false;

if (name == HTMLNames::idAttr) {
if (!oldValue.isEmpty())
treeScope().idTargetObserverRegistry().notifyObservers(*oldValue.impl());
if (!newValue.isEmpty())
treeScope().idTargetObserverRegistry().notifyObservers(*newValue.impl());

AtomicString oldId = elementData()->idForStyleResolution();
AtomicString newId = makeIdForStyleResolution(newValue, document().inQuirksMode());
if (newId != oldId) {
elementData()->setIdForStyleResolution(newId);
shouldInvalidateStyle = testShouldInvalidateStyle && checkNeedsStyleInvalidationForIdChange(oldId, newId, styleResolver);
}
} else if (name == classAttr)
classAttributeChanged(newValue);
else if (name == HTMLNames::nameAttr)
elementData()->setHasNameAttribute(!newValue.isNull());
else if (name == HTMLNames::pseudoAttr)
shouldInvalidateStyle |= testShouldInvalidateStyle && isInShadowTree();
if (!valueIsSameAsBefore) {
if (name == HTMLNames::idAttr) {
if (!oldValue.isEmpty())
treeScope().idTargetObserverRegistry().notifyObservers(*oldValue.impl());
if (!newValue.isEmpty())
treeScope().idTargetObserverRegistry().notifyObservers(*newValue.impl());

AtomicString oldId = elementData()->idForStyleResolution();
AtomicString newId = makeIdForStyleResolution(newValue, document().inQuirksMode());
if (newId != oldId) {
elementData()->setIdForStyleResolution(newId);
shouldInvalidateStyle = testShouldInvalidateStyle && checkNeedsStyleInvalidationForIdChange(oldId, newId, styleResolver);
}
} else if (name == classAttr)
classAttributeChanged(newValue);
else if (name == HTMLNames::nameAttr)
elementData()->setHasNameAttribute(!newValue.isNull());
else if (name == HTMLNames::pseudoAttr)
shouldInvalidateStyle |= testShouldInvalidateStyle && isInShadowTree();
}

parseAttribute(name, newValue);

document().incDOMTreeVersion();

if (valueIsSameAsBefore)
return;

invalidateNodeListAndCollectionCachesInAncestors(&name, this);

0 comments on commit 62c5bda

Please sign in to comment.