Bug 210267 - ProxyObject::defineOwnProperty() should conditionally throw on falsy trap result
Summary: ProxyObject::defineOwnProperty() should conditionally throw on falsy trap result
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Trivial
Assignee: Alexey Shvayka
URL:
Keywords: InRadar
: 217051 (view as bug list)
Depends on:
Blocks:
 
Reported: 2020-04-09 05:35 PDT by Alexey Shvayka
Modified: 2020-09-29 12:44 PDT (History)
9 users (show)

See Also:

Attachments
Patch (4.97 KB, patch)
2020-04-09 06:50 PDT, Alexey Shvayka 		no flags Details | Formatted Diff | Diff
Patch (5.04 KB, patch)
2020-04-09 08:22 PDT, Alexey Shvayka 		no flags Details | Formatted Diff | Diff
Patch (5.13 KB, patch)
2020-04-09 08:30 PDT, Alexey Shvayka 		no flags Details | Formatted Diff | Diff
Patch (22.65 KB, patch)
2020-04-09 12:37 PDT, Alexey Shvayka 		no flags Details | Formatted Diff | Diff
Show Obsolete (3) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Alexey Shvayka 2020-04-09 05:35:46 PDT 
Test case:
  Object.defineProperty(new Proxy({}, {defineProperty: () => false}), "foo", {})

Expected:
  TypeError thrown

Actual:
  Proxy object returned

ECMA262: https://tc39.es/ecma262/#sec-object.defineproperty (step 4)
Test262: https://test262.report/browse/built-ins/Proxy/defineProperty/trap-is-undefined-target-is-proxy.js
Comment 1 Alexey Shvayka 2020-04-09 06:50:50 PDT 
Created attachment 395945 [details]
Patch
Comment 2 Alexey Shvayka 2020-04-09 08:22:51 PDT 
Created attachment 395952 [details]
Patch

Add spec link to ChangeLog.
Comment 3 Alexey Shvayka 2020-04-09 08:30:41 PDT 
Created attachment 395956 [details]
Patch

Rebase patch.
Comment 4 Ross Kirsling 2020-04-09 11:20:32 PDT 
Comment on attachment 395956 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=395956&action=review

r=me with comment

> Source/JavaScriptCore/ChangeLog:12
> +        Also replaces 2 recently added throwTypeError() calls with throwVMTypeError(),
> +        as the latter seems to be preferred in ProxyObject.

Seems like we should go the other way if we're not using the result, since throwVMTypeError just encodes the JSValue.
Comment 5 Alexey Shvayka 2020-04-09 12:37:15 PDT 
Created attachment 395990 [details]
Patch

Set reviewer and replace throwVMTypeError() with unused results.
Comment 6 EWS 2020-04-09 13:40:43 PDT 
Committed r259822: <https://trac.webkit.org/changeset/259822>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 395990 [details].
Comment 7 Radar WebKit Bug Importer 2020-04-09 13:41:19 PDT 
<rdar://problem/61537090>
Comment 8 Alexey Shvayka 2020-09-29 12:44:16 PDT 
*** Bug 217051 has been marked as a duplicate of this bug. ***