lib,esm: handle bypass network-import via data: by RafaelGSS · Pull Request #53764 · nodejs/node

RafaelGSS · 2024-07-08T18:48:45Z

This commit didn't land cleanly on main from v22.x 110902f. So, I'm opening a manual PR to make sure we won't break anything.

cc: @aduh95 @GeoffreyBooth

nodejs-github-bot · 2024-07-08T18:48:50Z

Review requested:

@nodejs/loaders

RafaelGSS · 2024-07-08T18:51:48Z

The validation needs to accept H1 links as well.

lib/internal/modules/esm/resolve.js

PR-URL: nodejs-private/node-private#522 Refs: https://hackerone.com/bugs?subject=nodejs&report_id=2092749 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com> CVE-ID: CVE-2024-22020

nodejs-github-bot · 2024-07-08T23:19:20Z

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

aduh95 · 2024-07-09T08:04:17Z

lib/internal/modules/esm/resolve.js

      return { __proto__: null, url: parsed.href };
    }
  }
-


nit: unrelated line removal

mcollina

lgtm

nodejs-github-bot · 2024-07-12T12:33:24Z

Commit Queue failed

- Loading data for nodejs/node/pull/53764 ✔ Done loading data for nodejs/node/pull/53764 ----------------------------------- PR info ------------------------------------ Title lib,esm: handle bypass network-import via data: (#53764) Author Rafael Gonzaga <rafael.nunu@hotmail.com> (@RafaelGSS) Branch RafaelGSS:backport-network-import-via-data -> nodejs:main Labels esm, author ready, needs-ci Commits 1 - lib,esm: handle bypass network-import via data: Committers 1 - RafaelGSS <rafael.nunu@hotmail.com> PR-URL: https://github.com/nodejs/node/pull/53764 Fixes: https://hackerone.com/bugs?subject=nodejs&report_id=2092749 Reviewed-By: Yagiz Nizipli <yagiz.nizipli@sentry.io> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> ------------------------------ Generated metadata ------------------------------ PR-URL: https://github.com/nodejs/node/pull/53764 Fixes: https://hackerone.com/bugs?subject=nodejs&report_id=2092749 Reviewed-By: Yagiz Nizipli <yagiz.nizipli@sentry.io> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> -------------------------------------------------------------------------------- ℹ This PR was created on Mon, 08 Jul 2024 18:48:45 GMT ✔ Approvals: 4 ✔ - Yagiz Nizipli (@anonrig): https://github.com/nodejs/node/pull/53764#pullrequestreview-2164507162 ✔ - Antoine du Hamel (@aduh95) (TSC): https://github.com/nodejs/node/pull/53764#pullrequestreview-2165404578 ✔ - Marco Ippolito (@marco-ippolito) (TSC): https://github.com/nodejs/node/pull/53764#pullrequestreview-2165583758 ✔ - Matteo Collina (@mcollina) (TSC): https://github.com/nodejs/node/pull/53764#pullrequestreview-2165697861 ✔ Last GitHub CI successful ℹ Last Full PR CI on 2024-07-08T23:19:20Z: https://ci.nodejs.org/job/node-test-pull-request/60188/ - Querying data for job/node-test-pull-request/60188/ ✔ Last Jenkins CI successful -------------------------------------------------------------------------------- ✔ No git cherry-pick in progress ✔ No git am in progress ✔ No git rebase in progress -------------------------------------------------------------------------------- - Bringing origin/main up to date... From https://github.com/nodejs/node * branch main -> FETCH_HEAD ✔ origin/main is now up-to-date - Downloading patch for 53764 From https://github.com/nodejs/node * branch refs/pull/53764/merge -> FETCH_HEAD ✔ Fetched commits as fc233627ed44..15c2d8d75ed8 -------------------------------------------------------------------------------- [main ad0ac2de27] lib,esm: handle bypass network-import via data: Author: RafaelGSS <rafael.nunu@hotmail.com> Date: Wed Jan 10 14:50:18 2024 -0300 3 files changed, 164 insertions(+), 64 deletions(-) create mode 100644 test/fixtures/es-modules/import-data-url.mjs ✔ Patches applied -------------------------------------------------------------------------------- ⚠ Found Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>, skipping.. ⚠ Found Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>, skipping.. --------------------------------- New Message ---------------------------------- lib,esm: handle bypass network-import via data:

PR-URL: https://github.com/nodejs-private/node-private/pull/522
Refs: https://hackerone.com/bugs?subject=nodejs&report_id=2092749
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
CVE-ID: CVE-2024-22020
PR-URL: #53764
Fixes: https://hackerone.com/bugs?subject=nodejs&report_id=2092749
Reviewed-By: Yagiz Nizipli <yagiz.nizipli@sentry.io>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>

[main 6f2a2306fc] lib,esm: handle bypass network-import via data:
Author: RafaelGSS <rafael.nunu@hotmail.com>
Date: Wed Jan 10 14:50:18 2024 -0300
3 files changed, 164 insertions(+), 64 deletions(-)
create mode 100644 test/fixtures/es-modules/import-data-url.mjs
✖ 6f2a2306fcf219dcd27189810a543b542800cbc4
✔ 0:0 no Co-authored-by metadata co-authored-by-is-trailer
✖ 7:7 Fixes must be a GitHub URL. fixes-url
✔ 0:0 blank line after title line-after-title
✔ 0:0 line-lengths are valid line-length
✔ 0:0 metadata is at end of message metadata-end
✔ 6:8 PR-URL is valid. pr-url
✔ 0:0 reviewers are valid reviewers
✔ 0:0 valid subsystems subsystem
✔ 0:0 Title is formatted correctly. title-format
✔ 0:0 Title is <= 50 columns. title-length

ℹ Please fix the commit message and try again.
Please manually ammend the commit message, by running
git commit --amend
Once commit message is fixed, finish the landing command running
git node land --continue

https://github.com/nodejs/node/actions/runs/9907895685

nodejs-github-bot · 2024-07-12T12:48:55Z

Landed in 24648b5

PR-URL: https://github.com/nodejs-private/node-private/pull/522 Refs: https://hackerone.com/bugs?subject=nodejs&report_id=2092749 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com> CVE-ID: CVE-2024-22020 PR-URL: nodejs#53764 Reviewed-By: Yagiz Nizipli <yagiz.nizipli@sentry.io> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>

nodejs-github-bot added esm Issues and PRs related to the ECMAScript Modules implementation. needs-ci PRs that need a full CI run. labels Jul 8, 2024

RafaelGSS requested review from aduh95 and marco-ippolito July 8, 2024 18:50

anonrig reviewed Jul 8, 2024

View reviewed changes

lib/internal/modules/esm/resolve.js Outdated Show resolved Hide resolved

RafaelGSS force-pushed the backport-network-import-via-data branch from 5c74da6 to 61b1f42 Compare July 8, 2024 19:12

RafaelGSS commented Jul 8, 2024

View reviewed changes

lib/internal/modules/esm/resolve.js Show resolved Hide resolved

RafaelGSS force-pushed the backport-network-import-via-data branch from 61b1f42 to 9dc1933 Compare July 8, 2024 21:45

RafaelGSS force-pushed the backport-network-import-via-data branch from 9dc1933 to 15c2d8d Compare July 8, 2024 21:49

anonrig approved these changes Jul 8, 2024

View reviewed changes

RafaelGSS added the request-ci Add this label to start a Jenkins CI on a PR. label Jul 8, 2024

github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jul 8, 2024

RafaelGSS added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Jul 9, 2024

aduh95 approved these changes Jul 9, 2024

View reviewed changes

lib/internal/modules/esm/resolve.js

return { __proto__: null, url: parsed.href };

}

}

Copy link

Contributor

aduh95 Jul 9, 2024

Choose a reason for hiding this comment

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

nit: unrelated line removal

marco-ippolito approved these changes Jul 9, 2024

View reviewed changes

mcollina approved these changes Jul 9, 2024

View reviewed changes

RafaelGSS added the commit-queue Add this label to land a pull request using GitHub Actions. label Jul 12, 2024

nodejs-github-bot added commit-queue-failed An error occurred while landing this pull request using GitHub Actions. and removed commit-queue Add this label to land a pull request using GitHub Actions. labels Jul 12, 2024

RafaelGSS added commit-queue Add this label to land a pull request using GitHub Actions. and removed commit-queue-failed An error occurred while landing this pull request using GitHub Actions. labels Jul 12, 2024

nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Jul 12, 2024

nodejs-github-bot merged commit 24648b5 into nodejs:main Jul 12, 2024

aduh95 added the backported-to-v18.x label Jul 12, 2024

aduh95 added backported-to-v20.x PRs backported to the v20.x-staging branch. backported-to-v22.x PRs backported to the v22.x-staging branch. labels Jul 12, 2024

aduh95 mentioned this pull request Oct 14, 2024

2024-10-16, Version 23.0.0 (Current) #55338

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

lib,esm: handle bypass network-import via data:#53764

lib,esm: handle bypass network-import via data:#53764
nodejs-github-bot merged 1 commit intonodejs:mainfrom
RafaelGSS:backport-network-import-via-data

RafaelGSS commented Jul 8, 2024 •

edited

Loading

Uh oh!

nodejs-github-bot commented Jul 8, 2024

Uh oh!

RafaelGSS commented Jul 8, 2024

Uh oh!

Uh oh!

Uh oh!

nodejs-github-bot commented Jul 8, 2024

Uh oh!

aduh95 Jul 9, 2024

Uh oh!

mcollina left a comment

Uh oh!

nodejs-github-bot commented Jul 12, 2024

Uh oh!

nodejs-github-bot commented Jul 12, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Uh oh!

Conversation

RafaelGSS commented Jul 8, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nodejs-github-bot commented Jul 8, 2024

Uh oh!

RafaelGSS commented Jul 8, 2024

Uh oh!

Uh oh!

Uh oh!

nodejs-github-bot commented Jul 8, 2024

Uh oh!

aduh95 Jul 9, 2024

Choose a reason for hiding this comment

Uh oh!

mcollina left a comment

Choose a reason for hiding this comment

Uh oh!

nodejs-github-bot commented Jul 12, 2024

Uh oh!

nodejs-github-bot commented Jul 12, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

RafaelGSS commented Jul 8, 2024 •

edited

Loading