GeezerVison

View reviewed changes

GeoffreyBooth approved these changes

View reviewed changes

GeoffreyBooth left a comment

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

Out of scope for this PR, but should we have tests that things like our primordial PromisePrototypeAll behaves equivalently to Promise.all? Maybe an identical expected result from each in a test, where we wouldn’t have to worry about prototype pollution?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

ljharb and aduh95 reacted with thumbs up emoji

All reactions

👍 2 reactions

aduh95 added author ready

PRs with CI started, the required approvals, and no outstanding review comments.

request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. labels

github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label

This comment was marked as outdated.

mcollina approved these changes

View reviewed changes

mcollina left a comment

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

aduh95 commented

CI: https://ci.nodejs.org/job/node-test-pull-request/47496/

View reviewed changes

Comment thread

lib/internal/per_context/primordials.js Resolved

Uh oh!

There was an error while loading. Please reload this page.


          Update lib/internal/per_context/primordials.js

ae20149

nodejs-github-bot commented Oct 26, 2022

Copy link

Copy Markdown

Collaborator

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

nodejs-github-bot commented Oct 26, 2022

Copy link

Copy Markdown

Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/47498/

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

github-actions Bot mentioned this pull request

CI Reliability 2022-10-27 nodejs/reliability#412

Open

13 tasks

anonrig approved these changes

View reviewed changes

aduh95 added commit-queue

PRs queued for automated landing through the Commit Queue.

commit-queue-squash PRs the Commit Queue should land as one squashed commit. labels

nodejs-github-bot removed the commit-queue PRs queued for automated landing through the Commit Queue. label

nodejs-github-bot merged commit 2e2dc99 into nodejs:main

nodejs-github-bot commented Oct 27, 2022

Copy link

Copy Markdown

Collaborator

Landed in 2e2dc99

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

aduh95 deleted the esm-no-prototype-pollution branch

October 27, 2022 20:11

aduh95 mentioned this pull request

fs: add recursive watch for linux #45098

Merged

5 tasks done

github-actions Bot mentioned this pull request

Oct 28, 2022

CI Reliability 2022-10-28 nodejs/reliability#413

Open

10 tasks

This was referenced Oct 29, 2022

CI Reliability 2022-10-29 nodejs/reliability#414

Open

CI Reliability 2022-10-30 nodejs/reliability#415

Open

CI Reliability 2022-10-31 nodejs/reliability#416

Open

RafaelGSS pushed a commit that referenced this pull request

Nov 1, 2022


          esm: protect ESM loader from prototype pollution

f48b51a

In a previous commit, the loader implementation was modified to be
protected against most prototype pollution, but was kept vulnerable to
`Array.prototype` pollution. This commit fixes that, the tradeoff is
that it modifies the `ESMLoader.prototype.import` return type from an
`Array` to an array-like object.

Refs: #45044
PR-URL: #45175
Reviewed-By: Geoffrey Booth <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>

RafaelGSS mentioned this pull request

Nov 1, 2022

v19.1.0 proposal #45269

Merged

RafaelGSS pushed a commit that referenced this pull request

Nov 10, 2022


          esm: protect ESM loader from prototype pollution

40a5e22

In a previous commit, the loader implementation was modified to be
protected against most prototype pollution, but was kept vulnerable to
`Array.prototype` pollution. This commit fixes that, the tradeoff is
that it modifies the `ESMLoader.prototype.import` return type from an
`Array` to an array-like object.

Refs: #45044
PR-URL: #45175
Reviewed-By: Geoffrey Booth <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>

danielleadams mentioned this pull request

Dec 30, 2022

v18.13.0 release proposal #46025

Merged

danielleadams pushed a commit that referenced this pull request

Dec 30, 2022


          esm: protect ESM loader from prototype pollution

61d05e4

In a previous commit, the loader implementation was modified to be
protected against most prototype pollution, but was kept vulnerable to
`Array.prototype` pollution. This commit fixes that, the tradeoff is
that it modifies the `ESMLoader.prototype.import` return type from an
`Array` to an array-like object.

Refs: #45044
PR-URL: #45175
Reviewed-By: Geoffrey Booth <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>

danielleadams pushed a commit that referenced this pull request

Jan 3, 2023


          esm: protect ESM loader from prototype pollution

2f27d05

In a previous commit, the loader implementation was modified to be
protected against most prototype pollution, but was kept vulnerable to
`Array.prototype` pollution. This commit fixes that, the tradeoff is
that it modifies the `ESMLoader.prototype.import` return type from an
`Array` to an array-like object.

Refs: #45044
PR-URL: #45175
Reviewed-By: Geoffrey Booth <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>

tniessen mentioned this pull request

Apr 22, 2023

test_runner: fix test runner concurrency #47675

Closed

alan-agius4 mentioned this pull request

fix(zone.js): patch entire promise in node angular/angular#50552

Closed

alan-agius4 added a commit to alan-agius4/angular that referenced this pull request


          fix(zone.js): patch entire promise in node

88bb6e8

In angular#49144 we introduced a change to only path `Promise.prototype.then` due to Node.js `SafePromise` complaining about `Promise.prototype.then` called on incompatible receiver. This however introduced a number of regressions. This commit reverts this change and re-introduces the changes to patch the entire promise on Node.

The original `SafePromise` problem is no longer reproducible as of Node.js version 18.13+ as it was addressed as part of nodejs/node#45175.

While the Angular CLI does not yet generate ESM server bundles, users using ESM with dynamic imports will require using Node.js 18.13 or later.

Closes angular#50513, closes angular#50457, closes angular#50414 and closes angular#49930

alxhub pushed a commit to angular/angular that referenced this pull request


          fix(zone.js): patch entire promise in node (#50552)

cb31dbc

In #49144 we introduced a change to only path `Promise.prototype.then` due to Node.js `SafePromise` complaining about `Promise.prototype.then` called on incompatible receiver. This however introduced a number of regressions. This commit reverts this change and re-introduces the changes to patch the entire promise on Node.

The original `SafePromise` problem is no longer reproducible as of Node.js version 18.13+ as it was addressed as part of nodejs/node#45175.

While the Angular CLI does not yet generate ESM server bundles, users using ESM with dynamic imports will require using Node.js 18.13 or later.

Closes #50513, closes #50457, closes #50414 and closes #49930

PR Close #50552

alxhub pushed a commit to angular/angular that referenced this pull request