[SCM] WebKit Debian packaging branch, webkit-1.1, updated. upstream/1.1.15.1-1414-gc69ee75
darin at apple.com
darin at apple.com
Thu Oct 29 20:34:27 UTC 2009
The following commit has been merged in the webkit-1.1 branch:
commit b3314e50f7d8d6cbff41c5dd2b7d4736394d4db7
Author: darin at apple.com <darin at apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Date: Fri Sep 25 21:08:17 2009 +0000
Null-deref when first access to an Attr node is after its Element is destroyed
https://bugs.webkit.org/show_bug.cgi?id=29748
Patch by Darin Adler <darin at apple.com> on 2009-09-25
Reviewed by Geoffrey Garen.
WebCore:
Test: fast/dom/Attr/access-after-element-destruction.html
* bindings/js/JSAttrCustom.cpp:
(WebCore::JSAttr::markChildren): Added. Keeps the ownerElement alive as
long as the Attr is alive.
* bindings/js/JSNamedNodeMapCustom.cpp:
(WebCore::JSNamedNodeMap::markChildren): Added. Keeps the Element alive as
long as the NamedNodeMap is alive.
* dom/Attr.idl: Added CustomMarkFunction attribute.
* dom/NamedAttrMap.cpp:
(WebCore::NamedNodeMap::getAttributeItem): Tweaked formatting.
(WebCore::NamedNodeMap::detachFromElement): Call clearAttributes so we don't
have attributes hanging around that might need an Attr node created; that way
we won't crash with a null-dereference trying to deal with one of them. This
can't happen when working with JavaScript since the Element will be kept
alive due to the change above.
(WebCore::NamedNodeMap::addAttribute): Fix function name in comment.
(WebCore::NamedNodeMap::removeAttribute): Removed unneeded "+ 1" and added
missing braces.
* dom/NamedAttrMap.h: Made the element function public so it can be used by
the JavaScript binding to keep the Element alive.
* dom/NamedNodeMap.idl: Added CustomMarkFunction attribute.
LayoutTests:
* fast/dom/Attr/access-after-element-destruction-expected.txt: Added.
* fast/dom/Attr/access-after-element-destruction.html: Added.
* fast/dom/Attr/script-tests/TEMPLATE.html: Copied from LayoutTests/fast/dom/Node/script-tests/TEMPLATE.html.
* fast/dom/Attr/script-tests/access-after-element-destruction.js: Added.
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@48769 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index c7b6448..d259e9a 100644
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,3 +1,15 @@
+2009-09-25 Darin Adler <darin at apple.com>
+
+ Reviewed by Geoffrey Garen.
+
+ Null-deref when first access to an Attr node is after its Element is destroyed
+ https://bugs.webkit.org/show_bug.cgi?id=29748
+
+ * fast/dom/Attr/access-after-element-destruction-expected.txt: Added.
+ * fast/dom/Attr/access-after-element-destruction.html: Added.
+ * fast/dom/Attr/script-tests/TEMPLATE.html: Copied from LayoutTests/fast/dom/Node/script-tests/TEMPLATE.html.
+ * fast/dom/Attr/script-tests/access-after-element-destruction.js: Added.
+
2009-09-24 Alexey Proskuryakov <ap at apple.com>
Reviewed by Darin Adler and Sam Weinig.
diff --git a/LayoutTests/fast/dom/Attr/access-after-element-destruction-expected.txt b/LayoutTests/fast/dom/Attr/access-after-element-destruction-expected.txt
new file mode 100644
index 0000000..8c09132
--- /dev/null
+++ b/LayoutTests/fast/dom/Attr/access-after-element-destruction-expected.txt
@@ -0,0 +1,25 @@
+Tests that accessing Attr after its Element has been destroyed works without crashing.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS attributes.length is 1
+PASS attributes[0] is attributes.item(0)
+PASS attributes.getNamedItem('a') is attributes.item(0)
+PASS attributes.item(0).name is 'a'
+PASS attributes.item(0).specified is true
+PASS attributes.item(0).value is 'b'
+PASS attributes.item(0).ownerElement.tagName is 'P'
+PASS attributes.item(0).style is null
+PASS attributes.item(0).value is 'c'
+PASS attributes.length is 0
+PASS attr.name is 'a'
+PASS attr.specified is true
+PASS attr.value is 'b'
+PASS attr.ownerElement.tagName is 'P'
+PASS attr.style is null
+PASS attr.value is 'c'
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/fast/dom/Attr/access-after-element-destruction.html b/LayoutTests/fast/dom/Attr/access-after-element-destruction.html
new file mode 100644
index 0000000..39e014d
--- /dev/null
+++ b/LayoutTests/fast/dom/Attr/access-after-element-destruction.html
@@ -0,0 +1,13 @@
+<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
+<html>
+<head>
+<link rel="stylesheet" href="../../js/resources/js-test-style.css">
+<script src="../../js/resources/js-test-pre.js"></script>
+</head>
+<body>
+<p id="description"></p>
+<div id="console"></div>
+<script src="script-tests/access-after-element-destruction.js"></script>
+<script src="../../js/resources/js-test-post.js"></script>
+</body>
+</html>
diff --git a/LayoutTests/fast/backgrounds/repeat/script-tests/TEMPLATE.html b/LayoutTests/fast/dom/Attr/script-tests/TEMPLATE.html
similarity index 100%
copy from LayoutTests/fast/backgrounds/repeat/script-tests/TEMPLATE.html
copy to LayoutTests/fast/dom/Attr/script-tests/TEMPLATE.html
diff --git a/LayoutTests/fast/dom/Attr/script-tests/access-after-element-destruction.js b/LayoutTests/fast/dom/Attr/script-tests/access-after-element-destruction.js
new file mode 100644
index 0000000..91588a3
--- /dev/null
+++ b/LayoutTests/fast/dom/Attr/script-tests/access-after-element-destruction.js
@@ -0,0 +1,55 @@
+description("Tests that accessing Attr after its Element has been destroyed works without crashing.");
+
+function gc()
+{
+ if (window.GCController)
+ return GCController.collect();
+
+ // Trigger garbage collection indirectly.
+ for (var i = 0; i < 100000; i++)
+ new String(i);
+}
+
+var element = document.createElement("p");
+element.setAttribute("a", "b");
+var attributes = element.attributes;
+element = null;
+
+gc();
+
+shouldBe("attributes.length", "1");
+shouldBe("attributes[0]", "attributes.item(0)");
+shouldBe("attributes.getNamedItem('a')", "attributes.item(0)");
+
+shouldBe("attributes.item(0).name", "'a'");
+shouldBe("attributes.item(0).specified", "true");
+shouldBe("attributes.item(0).value", "'b'");
+shouldBe("attributes.item(0).ownerElement.tagName", "'P'");
+shouldBe("attributes.item(0).style", "null");
+
+attributes.item(0).value = 'c';
+
+shouldBe("attributes.item(0).value", "'c'");
+
+attributes.removeNamedItem('a');
+
+shouldBe("attributes.length", "0");
+
+element = document.createElement("p");
+element.setAttribute("a", "b");
+var attr = element.attributes.item(0);
+element = null;
+
+gc();
+
+shouldBe("attr.name", "'a'");
+shouldBe("attr.specified", "true");
+shouldBe("attr.value", "'b'");
+shouldBe("attr.ownerElement.tagName", "'P'");
+shouldBe("attr.style", "null");
+
+attr.value = 'c';
+
+shouldBe("attr.value", "'c'");
+
+var successfullyParsed = true;
diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog
index e8b7432..7aff711 100644
--- a/WebCore/ChangeLog
+++ b/WebCore/ChangeLog
@@ -1,3 +1,38 @@
+2009-09-25 Darin Adler <darin at apple.com>
+
+ Reviewed by Geoffrey Garen.
+
+ Null-deref when first access to an Attr node is after its Element is destroyed
+ https://bugs.webkit.org/show_bug.cgi?id=29748
+
+ Test: fast/dom/Attr/access-after-element-destruction.html
+
+ * bindings/js/JSAttrCustom.cpp:
+ (WebCore::JSAttr::markChildren): Added. Keeps the ownerElement alive as
+ long as the Attr is alive.
+
+ * bindings/js/JSNamedNodeMapCustom.cpp:
+ (WebCore::JSNamedNodeMap::markChildren): Added. Keeps the Element alive as
+ long as the NamedNodeMap is alive.
+
+ * dom/Attr.idl: Added CustomMarkFunction attribute.
+
+ * dom/NamedAttrMap.cpp:
+ (WebCore::NamedNodeMap::getAttributeItem): Tweaked formatting.
+ (WebCore::NamedNodeMap::detachFromElement): Call clearAttributes so we don't
+ have attributes hanging around that might need an Attr node created; that way
+ we won't crash with a null-dereference trying to deal with one of them. This
+ can't happen when working with JavaScript since the Element will be kept
+ alive due to the change above.
+ (WebCore::NamedNodeMap::addAttribute): Fix function name in comment.
+ (WebCore::NamedNodeMap::removeAttribute): Removed unneeded "+ 1" and added
+ missing braces.
+
+ * dom/NamedAttrMap.h: Made the element function public so it can be used by
+ the JavaScript binding to keep the Element alive.
+
+ * dom/NamedNodeMap.idl: Added CustomMarkFunction attribute.
+
2009-09-24 Alexey Proskuryakov <ap at apple.com>
Reviewed by Darin Adler and Sam Weinig.
diff --git a/WebCore/bindings/js/JSAttrCustom.cpp b/WebCore/bindings/js/JSAttrCustom.cpp
index e217023..14457c4 100644
--- a/WebCore/bindings/js/JSAttrCustom.cpp
+++ b/WebCore/bindings/js/JSAttrCustom.cpp
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2007, 2008 Apple Inc. All rights reserved.
+ * Copyright (C) 2007, 2008, 2009 Apple Inc. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
@@ -59,4 +59,16 @@ void JSAttr::setValue(ExecState* exec, JSValue value)
setDOMException(exec, ec);
}
+void JSAttr::markChildren(MarkStack& markStack)
+{
+ Base::markChildren(markStack);
+
+ // Mark the element so that this will work to access the attribute even if the last
+ // other reference goes away.
+ if (Element* element = impl()->ownerElement()) {
+ if (JSNode* wrapper = getCachedDOMNodeWrapper(element->document(), element))
+ markStack.append(wrapper);
+ }
+}
+
} // namespace WebCore
diff --git a/WebCore/bindings/js/JSNamedNodeMapCustom.cpp b/WebCore/bindings/js/JSNamedNodeMapCustom.cpp
index 7bd95b4..93aedca 100644
--- a/WebCore/bindings/js/JSNamedNodeMapCustom.cpp
+++ b/WebCore/bindings/js/JSNamedNodeMapCustom.cpp
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2007, 2008 Apple Inc. All rights reserved.
+ * Copyright (C) 2007, 2008, 2009 Apple Inc. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
@@ -27,10 +27,6 @@
#include "JSNamedNodeMap.h"
#include "JSNode.h"
-#include "NamedNodeMap.h"
-#include "Node.h"
-#include "PlatformString.h"
-#include "JSDOMBinding.h"
using namespace JSC;
@@ -47,4 +43,16 @@ JSValue JSNamedNodeMap::nameGetter(ExecState* exec, const Identifier& propertyNa
return toJS(exec, thisObj->impl()->getNamedItem(propertyName));
}
+void JSNamedNodeMap::markChildren(MarkStack& markStack)
+{
+ Base::markChildren(markStack);
+
+ // Mark the element so that this will work to access the attribute even if the last
+ // other reference goes away.
+ if (Element* element = impl()->element()) {
+ if (JSNode* wrapper = getCachedDOMNodeWrapper(element->document(), element))
+ markStack.append(wrapper);
+ }
+}
+
} // namespace WebCore
diff --git a/WebCore/dom/Attr.idl b/WebCore/dom/Attr.idl
index 29f4be1..c01f34a 100644
--- a/WebCore/dom/Attr.idl
+++ b/WebCore/dom/Attr.idl
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2006, 2007, 2008 Apple Inc. All rights reserved.
+ * Copyright (C) 2006, 2007, 2008, 2009 Apple Inc. All rights reserved.
* Copyright (C) 2006 Samuel Weinig <sam.weinig at gmail.com>
*
* This library is free software; you can redistribute it and/or
@@ -21,6 +21,7 @@
module core {
interface [
+ CustomMarkFunction,
GenerateConstructor,
GenerateNativeConverter,
InterfaceUUID=EEE8E22B-22C3-4e50-95F4-5E0B8AAD8231,
diff --git a/WebCore/dom/NamedAttrMap.cpp b/WebCore/dom/NamedAttrMap.cpp
index fe631c8..d4ec598 100644
--- a/WebCore/dom/NamedAttrMap.cpp
+++ b/WebCore/dom/NamedAttrMap.cpp
@@ -178,10 +178,8 @@ Attribute* NamedNodeMap::getAttributeItem(const String& name, bool shouldIgnoreA
{
unsigned len = length();
for (unsigned i = 0; i < len; ++i) {
- if (!m_attributes[i]->name().hasPrefix() &&
- m_attributes[i]->name().localName() == name)
- return m_attributes[i].get();
-
+ if (!m_attributes[i]->name().hasPrefix() && m_attributes[i]->name().localName() == name)
+ return m_attributes[i].get();
if (shouldIgnoreAttributeCase ? equalIgnoringCase(m_attributes[i]->name().toString(), name) : name == m_attributes[i]->name().toString())
return m_attributes[i].get();
}
@@ -206,10 +204,12 @@ void NamedNodeMap::clearAttributes()
void NamedNodeMap::detachFromElement()
{
- // we allow a NamedNodeMap w/o an element in case someone still has a reference
- // to if after the element gets deleted - but the map is now invalid
+ // This can't happen if the holder of the map is JavaScript, because we mark the
+ // element if the map is alive. So it has no impact on web page behavior. Because
+ // of that, we can simply clear all the attributes to avoid accessing stale
+ // pointers to do things like create Attr objects.
m_element = 0;
- detachAttributesFromElement();
+ clearAttributes();
}
void NamedNodeMap::setAttributes(const NamedNodeMap& other)
@@ -251,7 +251,7 @@ void NamedNodeMap::addAttribute(PassRefPtr<Attribute> prpAttribute)
attr->m_element = m_element;
// Notify the element that the attribute has been added, and dispatch appropriate mutation events
- // Note that element may be null here if we are called from insertAttr() during parsing
+ // Note that element may be null here if we are called from insertAttribute() during parsing
if (m_element) {
m_element->attributeChanged(attribute.get());
// Because of our updateStyleAttribute() style modification events are never sent at the right time, so don't bother sending them.
@@ -265,12 +265,13 @@ void NamedNodeMap::addAttribute(PassRefPtr<Attribute> prpAttribute)
void NamedNodeMap::removeAttribute(const QualifiedName& name)
{
unsigned len = length();
- unsigned index = len + 1;
- for (unsigned i = 0; i < len; ++i)
+ unsigned index = len;
+ for (unsigned i = 0; i < len; ++i) {
if (m_attributes[i]->name().matches(name)) {
index = i;
break;
}
+ }
if (index >= len)
return;
diff --git a/WebCore/dom/NamedAttrMap.h b/WebCore/dom/NamedAttrMap.h
index 4fb96de..759900b 100644
--- a/WebCore/dom/NamedAttrMap.h
+++ b/WebCore/dom/NamedAttrMap.h
@@ -94,11 +94,11 @@ public:
void addAttribute(PassRefPtr<Attribute>);
void removeAttribute(const QualifiedName&);
+ Element* element() const { return m_element; }
+
protected:
virtual void clearAttributes();
- Element* element() const { return m_element; }
-
private:
void detachAttributesFromElement();
void detachFromElement();
diff --git a/WebCore/dom/NamedNodeMap.idl b/WebCore/dom/NamedNodeMap.idl
index 3310ded..8166853 100644
--- a/WebCore/dom/NamedNodeMap.idl
+++ b/WebCore/dom/NamedNodeMap.idl
@@ -1,6 +1,6 @@
/*
* Copyright (C) 2006 Samuel Weinig <sam.weinig at gmail.com>
- * Copyright (C) 2007 Apple Inc. All rights reserved.
+ * Copyright (C) 2007, 2009 Apple Inc. All rights reserved.
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Library General Public
@@ -21,6 +21,7 @@
module core {
interface [
+ CustomMarkFunction,
GenerateConstructor,
HasIndexGetter,
HasNameGetter,
--
WebKit Debian packaging
More information about the Pkg-webkit-commits
mailing list