Bug 184267 - Array.prototype.concat with Proxy
Summary: Array.prototype.concat with Proxy
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: Safari 11
Hardware: Mac macOS 10.13
: P2 Normal
Assignee: Caitlin Potter (:caitp)
URL:
Keywords: InRadar
: 186991 186992 186993 (view as bug list)
Depends on:
Blocks:
 
Reported: 2018-04-03 10:00 PDT by Pierre-Marie Dartus
Modified: 2020-05-08 17:02 PDT (History)
13 users (show)

See Also:


Attachments
Patch (4.85 KB, patch)
2018-05-28 15:34 PDT, Caitlin Potter (:caitp)
no flags Details | Formatted Diff | Diff
Patch for landing (4.45 KB, patch)
2018-05-29 09:17 PDT, Caitlin Potter (:caitp)
no flags Details | Formatted Diff | Diff
Patch (1.90 KB, patch)
2018-05-29 14:49 PDT, Caitlin Potter (:caitp)
no flags Details | Formatted Diff | Diff
Patch for landing (1.90 KB, patch)
2018-05-29 17:02 PDT, Caitlin Potter (:caitp)
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Pierre-Marie Dartus 2018-04-03 10:00:04 PDT
**Step to reproduce:**
const proxy = new Proxy([3, 4], {});
const res = [1, 2].concat(proxy);

console.log(res[0]);
console.log(res[1]);
console.log(res[2]);
console.log(res[3]);

**Expected output:**
1
2
3
4

**Actual output:**
1
2
Proxy {0: 3, 1: 4}
undefined

**Observation:**
Works in Chrome 65 and Firefox 58
Comment 1 Radar WebKit Bug Importer 2018-04-06 20:58:13 PDT
<rdar://problem/39256680>
Comment 2 Caitlin Potter (:caitp) 2018-05-28 15:34:53 PDT
Created attachment 341460 [details]
Patch
Comment 3 Saam Barati 2018-05-29 08:59:57 PDT
Comment on attachment 341460 [details]
Patch

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

> Source/JavaScriptCore/ChangeLog:11
> +        This incorrectly treated prevented Proxy objects from being spread when

“treated prevented” => prevented

> Source/JavaScriptCore/ChangeLog:12
> +        they were the only argument passed to A.p.concat(), violating ECMA-262.

Nit: I’d just spell out Array.prototype.concat

> JSTests/ChangeLog:8
> +        Before this patch, the fast case for Array.prototype.concat was taken if

Don’t think you need to repeat the same text in this changelog here
Comment 4 Caitlin Potter (:caitp) 2018-05-29 09:17:43 PDT
Created attachment 341489 [details]
Patch for landing
Comment 5 Caitlin Potter (:caitp) 2018-05-29 09:24:04 PDT
Comment on attachment 341489 [details]
Patch for landing

I think this patch is still slightly wrong, going to verify a bit..
Comment 6 Caitlin Potter (:caitp) 2018-05-29 09:26:47 PDT
Comment on attachment 341489 [details]
Patch for landing

nevermind, I guess it was ok after all..
Comment 7 WebKit Commit Bot 2018-05-29 09:56:35 PDT
Comment on attachment 341489 [details]
Patch for landing

Clearing flags on attachment: 341489

Committed r232261: <https://trac.webkit.org/changeset/232261>
Comment 8 WebKit Commit Bot 2018-05-29 09:56:36 PDT
All reviewed patches have been landed.  Closing bug.
Comment 9 Caitlin Potter (:caitp) 2018-05-29 14:49:16 PDT
Reopening to attach new patch.
Comment 10 Caitlin Potter (:caitp) 2018-05-29 14:49:17 PDT
Created attachment 341520 [details]
Patch
Comment 11 Caitlin Potter (:caitp) 2018-05-29 14:50:05 PDT
I didn't know we had a new test262 runner in the tree. This patch fixed some failures, so here's another patch to fix the test262 expectations.
Comment 12 Saam Barati 2018-05-29 15:13:02 PDT
Comment on attachment 341520 [details]
Patch

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

> JSTests/ChangeLog:3
> +
> +

Only need one new line here
Comment 13 Caitlin Potter (:caitp) 2018-05-29 17:02:08 PDT
Created attachment 341538 [details]
Patch for landing
Comment 14 WebKit Commit Bot 2018-05-29 17:40:33 PDT
Comment on attachment 341538 [details]
Patch for landing

Clearing flags on attachment: 341538

Committed r232282: <https://trac.webkit.org/changeset/232282>
Comment 15 WebKit Commit Bot 2018-05-29 17:40:35 PDT
All reviewed patches have been landed.  Closing bug.
Comment 16 Alexey Shvayka 2020-05-07 15:11:25 PDT
*** Bug 186991 has been marked as a duplicate of this bug. ***
Comment 17 Alexey Shvayka 2020-05-07 15:20:06 PDT
*** Bug 186993 has been marked as a duplicate of this bug. ***
Comment 18 Alexey Shvayka 2020-05-07 15:40:13 PDT
*** Bug 186992 has been marked as a duplicate of this bug. ***