- Revision
- 114427
- Author
- b...@google.com
- Date
- 2012-04-17 13:32:22 -0700 (Tue, 17 Apr 2012)
Log Message
`localStorage.setItem` can overwrite `localStorage` methods
https://bugs.webkit.org/show_bug.cgi?id=30996
Source/WebCore:
Update the JSC and V8 bindings such that if the name of the DOM
Storage property being retrieved is a match for a property in the
prototype, always return the prototype version. If there is a DOM
Storage key of the same name, it can still be retrieved via the
getItem method. This prevents storage methods from being
accidentally hidden. This brings WebKit behavior in line with the
de facto standard implemented by FireFox and IE.
Reviewed by Kentaro Hara.
Test: storage/domstorage/storage-functions-not-overwritten.html
* bindings/js/JSStorageCustom.cpp:
(WebCore::JSStorage::nameGetter):
* bindings/v8/custom/V8StorageCustom.cpp:
(WebCore::V8Storage::namedPropertyGetter):
LayoutTests:
Add a test to verify that setting a DOM Storage key with the same name
as a function on the Storage object does not prevent that function
being called, but also that it can still be retrieved via the getItem
function.
Reviewed by Kentaro Hara.
* storage/domstorage/script-tests/storage-functions-not-overwritten.js: Added.
(doWedgeThySelf):
(testStorage):
(runTest):
* storage/domstorage/storage-functions-not-overwritten-expected.txt: Added.
* storage/domstorage/storage-functions-not-overwritten.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (114426 => 114427)
--- trunk/LayoutTests/ChangeLog 2012-04-17 20:25:30 UTC (rev 114426)
+++ trunk/LayoutTests/ChangeLog 2012-04-17 20:32:22 UTC (rev 114427)
@@ -1,3 +1,22 @@
+2012-04-17 Ben Murdoch <b...@google.com>
+
+ `localStorage.setItem` can overwrite `localStorage` methods
+ https://bugs.webkit.org/show_bug.cgi?id=30996
+
+ Add a test to verify that setting a DOM Storage key with the same name
+ as a function on the Storage object does not prevent that function
+ being called, but also that it can still be retrieved via the getItem
+ function.
+
+ Reviewed by Kentaro Hara.
+
+ * storage/domstorage/script-tests/storage-functions-not-overwritten.js: Added.
+ (doWedgeThySelf):
+ (testStorage):
+ (runTest):
+ * storage/domstorage/storage-functions-not-overwritten-expected.txt: Added.
+ * storage/domstorage/storage-functions-not-overwritten.html: Added.
+
2012-04-17 Vincent Scheib <sch...@chromium.org>
[Chromium] Rebaseline minor text pixel differences in bidi-menulist-expected.
Added: trunk/LayoutTests/storage/domstorage/script-tests/storage-functions-not-overwritten.js (0 => 114427)
--- trunk/LayoutTests/storage/domstorage/script-tests/storage-functions-not-overwritten.js (rev 0)
+++ trunk/LayoutTests/storage/domstorage/script-tests/storage-functions-not-overwritten.js 2012-04-17 20:32:22 UTC (rev 114427)
@@ -0,0 +1,49 @@
+description("This test checks to ensure that window.localStorage and window.sessionStorage are not rendered unusable by setting a key with the same name as a storage function such that the function is hidden.");
+
+if (window.layoutTestController)
+ layoutTestController.dumpAsText();
+
+function doWedgeThySelf(storage) {
+ storage.setItem("clear", "almost");
+ storage.setItem("key", "too");
+ storage.setItem("getItem", "funny");
+ storage.setItem("removeItem", "to");
+ storage.setItem("length", "be");
+ storage.setItem("setItem", "true");
+}
+
+function testStorage(storageString) {
+ storage = eval(storageString);
+ storage.clear();
+ doWedgeThySelf(storage);
+ shouldBeEqualToString("storage.getItem('clear')", "almost");
+ shouldBeEqualToString("storage.getItem('key')", "too");
+ shouldBeEqualToString("storage.getItem('getItem')", "funny");
+ shouldBeEqualToString("storage.getItem('removeItem')", "to");
+ shouldBeEqualToString("storage.getItem('length')", "be");
+ shouldBeEqualToString("storage.getItem('setItem')", "true");
+
+ // Test to see if an exception is thrown for any of the built in
+ // functions.
+ storage.setItem("test", "123");
+ storage.key(0);
+ storage.getItem("test");
+ storage.removeItem("test");
+ storage.clear();
+ if (storage.length != 0)
+ throw name + ": length wedged";
+}
+
+
+function runTest()
+{
+ testStorage(window.sessionStorage);
+ testStorage(window.localStorage);
+}
+
+try {
+ runTest();
+} catch (e) {
+ testFailed("Caught an exception!");
+}
+
Added: trunk/LayoutTests/storage/domstorage/storage-functions-not-overwritten-expected.txt (0 => 114427)
--- trunk/LayoutTests/storage/domstorage/storage-functions-not-overwritten-expected.txt (rev 0)
+++ trunk/LayoutTests/storage/domstorage/storage-functions-not-overwritten-expected.txt 2012-04-17 20:32:22 UTC (rev 114427)
@@ -0,0 +1,21 @@
+This test checks to ensure that window.localStorage and window.sessionStorage are not rendered unusable by setting a key with the same name as a storage function such that the function is hidden.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS storage.getItem('clear') is "almost"
+PASS storage.getItem('key') is "too"
+PASS storage.getItem('getItem') is "funny"
+PASS storage.getItem('removeItem') is "to"
+PASS storage.getItem('length') is "be"
+PASS storage.getItem('setItem') is "true"
+PASS storage.getItem('clear') is "almost"
+PASS storage.getItem('key') is "too"
+PASS storage.getItem('getItem') is "funny"
+PASS storage.getItem('removeItem') is "to"
+PASS storage.getItem('length') is "be"
+PASS storage.getItem('setItem') is "true"
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: trunk/LayoutTests/storage/domstorage/storage-functions-not-overwritten.html (0 => 114427)
--- trunk/LayoutTests/storage/domstorage/storage-functions-not-overwritten.html (rev 0)
+++ trunk/LayoutTests/storage/domstorage/storage-functions-not-overwritten.html 2012-04-17 20:32:22 UTC (rev 114427)
@@ -0,0 +1,11 @@
+<html>
+<head>
+<script src=""
+</head>
+<body>
+<p id="description"></p>
+<div id="console"></div>
+<script src=""
+<script src=""
+</body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (114426 => 114427)
--- trunk/Source/WebCore/ChangeLog 2012-04-17 20:25:30 UTC (rev 114426)
+++ trunk/Source/WebCore/ChangeLog 2012-04-17 20:32:22 UTC (rev 114427)
@@ -1,3 +1,25 @@
+2012-04-17 Ben Murdoch <b...@google.com>
+
+ `localStorage.setItem` can overwrite `localStorage` methods
+ https://bugs.webkit.org/show_bug.cgi?id=30996
+
+ Update the JSC and V8 bindings such that if the name of the DOM
+ Storage property being retrieved is a match for a property in the
+ prototype, always return the prototype version. If there is a DOM
+ Storage key of the same name, it can still be retrieved via the
+ getItem method. This prevents storage methods from being
+ accidentally hidden. This brings WebKit behavior in line with the
+ de facto standard implemented by FireFox and IE.
+
+ Reviewed by Kentaro Hara.
+
+ Test: storage/domstorage/storage-functions-not-overwritten.html
+
+ * bindings/js/JSStorageCustom.cpp:
+ (WebCore::JSStorage::nameGetter):
+ * bindings/v8/custom/V8StorageCustom.cpp:
+ (WebCore::V8Storage::namedPropertyGetter):
+
2012-04-17 Emil A Eklund <e...@chromium.org>
Fix Chromium/Windows build broken by r114404.
Modified: trunk/Source/WebCore/bindings/js/JSStorageCustom.cpp (114426 => 114427)
--- trunk/Source/WebCore/bindings/js/JSStorageCustom.cpp 2012-04-17 20:25:30 UTC (rev 114426)
+++ trunk/Source/WebCore/bindings/js/JSStorageCustom.cpp 2012-04-17 20:32:22 UTC (rev 114427)
@@ -42,6 +42,11 @@
JSValue JSStorage::nameGetter(ExecState* exec, JSValue slotBase, const Identifier& propertyName)
{
JSStorage* thisObj = jsCast<JSStorage*>(asObject(slotBase));
+
+ JSValue prototype = asObject(slotBase)->prototype();
+ if (prototype.isObject() && asObject(prototype)->hasProperty(exec, propertyName))
+ return asObject(prototype)->get(exec, propertyName);
+
return jsStringOrNull(exec, thisObj->impl()->getItem(identifierToString(propertyName)));
}
Modified: trunk/Source/WebCore/bindings/v8/custom/V8StorageCustom.cpp (114426 => 114427)
--- trunk/Source/WebCore/bindings/v8/custom/V8StorageCustom.cpp 2012-04-17 20:25:30 UTC (rev 114426)
+++ trunk/Source/WebCore/bindings/v8/custom/V8StorageCustom.cpp 2012-04-17 20:32:22 UTC (rev 114427)
@@ -74,6 +74,8 @@
v8::Handle<v8::Value> V8Storage::namedPropertyGetter(v8::Local<v8::String> name, const v8::AccessorInfo& info)
{
INC_STATS("DOM.Storage.NamedPropertyGetter");
+ if (!info.Holder()->GetRealNamedPropertyInPrototypeChain(name).IsEmpty())
+ return notHandledByInterceptor();
return storageGetter(name, info);
}
