Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(273)

Issue 2255413004: IndexedDB: Avoid side effects by evaluating key paths w/ HasOwnProperty (Closed)

Created:
4 years, 4 months ago by jsbell
Modified:
4 years, 4 months ago
Reviewers:
haraken
CC:
jsbell+idb_chromium.org, cmumford
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

IndexedDB: Avoid side effects by evaluating key paths w/ HasOwnProperty Although key path evaluation is always done against clones (so getters are flattened into simple data properties), property lookups were not restricted to inherited properties, allowing getters on prototypes to be executed. This allows evaluation to be observable, and cause side effects. Restrict the lookup to own properties, and introduce special-case code for those non-own values identified in the spec[1] as special cases. [1] https://w3c.github.io/IndexedDB/#key-path-construct BUG=637963 R=haraken@chromium.org Committed: https://crrev.com/fb18204c77e3f6e43ce05dd3ce24f00e0201bac1 Cr-Commit-Position: refs/heads/master@{#414170}

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+105 lines, -59 lines) Patch
M third_party/WebKit/LayoutTests/storage/indexeddb/bindings-edges.html View 2 chunks +35 lines, -35 lines 0 comments Download
M third_party/WebKit/LayoutTests/storage/indexeddb/key_conversion_exceptions.html View 3 chunks +18 lines, -14 lines 0 comments Download
M third_party/WebKit/Source/bindings/modules/v8/V8BindingForModules.cpp View 1 chunk +52 lines, -10 lines 0 comments Download

Messages

Total messages: 13 (6 generated)
jsbell
haraken@ - can you please take a look? See the bug for more context about ...
4 years, 4 months ago (2016-08-19 21:39:26 UTC) #1
haraken
LGTM
4 years, 4 months ago (2016-08-22 05:01:24 UTC) #2
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2255413004/1
4 years, 4 months ago (2016-08-24 16:20:27 UTC) #5
commit-bot: I haz the power
Try jobs failed on following builders: win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_ng/builds/280095)
4 years, 4 months ago (2016-08-24 18:18:06 UTC) #7
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2255413004/1
4 years, 4 months ago (2016-08-24 18:42:01 UTC) #9
commit-bot: I haz the power
Committed patchset #1 (id:1)
4 years, 4 months ago (2016-08-24 22:18:33 UTC) #11
commit-bot: I haz the power
4 years, 4 months ago (2016-08-24 22:20:28 UTC) #13
Message was sent while issue was closed.
Patchset 1 (id:??) landed as
https://crrev.com/fb18204c77e3f6e43ce05dd3ce24f00e0201bac1
Cr-Commit-Position: refs/heads/master@{#414170}

Powered by Google App Engine
This is Rietveld 408576698