module: fix crash when built-in module export a `default` key #51481

aduh95 · 2024-01-16T00:18:21Z

nodejs-github-bot · 2024-01-16T00:18:26Z

Review requested:

@nodejs/loaders
@nodejs/startup

guybedford · 2024-01-16T04:54:28Z

Looking at this bug, it seems that assigning arbitrary keys to process before importing it can affect its shape. For example:

process.asdf = 'blah';
const processMod = await import('node:process');
console.log(processMod.asdf); // blah

I would argue that this is still a bug even with this PR fix for the default crash.

guybedford

Of course I approve the fix, just noting there is some wider inconsistency here.

lib/internal/bootstrap/realm.js

Co-authored-by: Geoffrey Booth <webadmin@geoffreybooth.com>

nodejs-github-bot · 2024-01-16T09:43:53Z

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

nodejs-github-bot · 2024-01-16T17:44:01Z

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

nodejs-github-bot · 2024-01-19T07:06:33Z

Commit Queue failed

- Loading data for nodejs/node/pull/51481
✔  Done loading data for nodejs/node/pull/51481
----------------------------------- PR info ------------------------------------
Title      module: fix crash when built-in module export a `default` key (#51481)
   ⚠  Could not retrieve the email or name of the PR author's from user's GitHub profile!
Branch     aduh95:process-default -> nodejs:main
Labels     lib / src, author ready, needs-ci
Commits    2
 - module: fix crash when built-in module export a `default` key
 - Update lib/internal/bootstrap/realm.js
Committers 2
 - Antoine du Hamel 
 - GitHub 
PR-URL: https://github.com/nodejs/node/pull/51481
Fixes: https://github.com/nodejs/node/issues/51480
Reviewed-By: Chengzhong Wu 
Reviewed-By: Guy Bedford 
Reviewed-By: Geoffrey Booth 
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/51481
Fixes: https://github.com/nodejs/node/issues/51480
Reviewed-By: Chengzhong Wu 
Reviewed-By: Guy Bedford 
Reviewed-By: Geoffrey Booth 
--------------------------------------------------------------------------------
   ⚠  Commits were pushed since the last approving review:
   ⚠  - Update lib/internal/bootstrap/realm.js
   ℹ  This PR was created on Tue, 16 Jan 2024 00:18:21 GMT
   ✔  Approvals: 3
   ✔  - Chengzhong Wu (@legendecas) (TSC): https://github.com/nodejs/node/pull/51481#pullrequestreview-1822478224
   ✔  - Guy Bedford (@guybedford): https://github.com/nodejs/node/pull/51481#pullrequestreview-1822522624
   ✔  - Geoffrey Booth (@GeoffreyBooth) (TSC): https://github.com/nodejs/node/pull/51481#pullrequestreview-1822572122
   ✔  Last GitHub CI successful
   ℹ  Last Full PR CI on 2024-01-16T17:44:01Z: https://ci.nodejs.org/job/node-test-pull-request/56816/
- Querying data for job/node-test-pull-request/56816/
   ✔  Last Jenkins CI successful
--------------------------------------------------------------------------------
   ✔  Aborted `git node land` session in /home/runner/work/node/node/.ncu

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

nodejs-github-bot · 2024-01-19T07:52:03Z

Commit Queue failed

- Loading data for nodejs/node/pull/51481 ✔ Done loading data for nodejs/node/pull/51481 ----------------------------------- PR info ------------------------------------ Title module: fix crash when built-in module export a `default` key (#51481) ⚠ Could not retrieve the email or name of the PR author's from user's GitHub profile! Branch aduh95:process-default -> nodejs:main Labels lib / src, author ready, needs-ci Commits 2 - module: fix crash when built-in module export a `default` key - Update lib/internal/bootstrap/realm.js Committers 2 - Antoine du Hamel - GitHub PR-URL: https://github.com/nodejs/node/pull/51481 Fixes: https://github.com/nodejs/node/issues/51480 Reviewed-By: Chengzhong Wu Reviewed-By: Guy Bedford Reviewed-By: Geoffrey Booth ------------------------------ Generated metadata ------------------------------ PR-URL: https://github.com/nodejs/node/pull/51481 Fixes: https://github.com/nodejs/node/issues/51480 Reviewed-By: Chengzhong Wu Reviewed-By: Guy Bedford Reviewed-By: Geoffrey Booth -------------------------------------------------------------------------------- ℹ This PR was created on Tue, 16 Jan 2024 00:18:21 GMT ✔ Approvals: 3 ✔ - Chengzhong Wu (@legendecas) (TSC): https://github.com/nodejs/node/pull/51481#pullrequestreview-1831785070 ✔ - Guy Bedford (@guybedford): https://github.com/nodejs/node/pull/51481#pullrequestreview-1822522624 ✔ - Geoffrey Booth (@GeoffreyBooth) (TSC): https://github.com/nodejs/node/pull/51481#pullrequestreview-1822572122 ✔ Last GitHub CI successful ℹ Last Full PR CI on 2024-01-19T07:06:33Z: https://ci.nodejs.org/job/node-test-pull-request/56816/ - Querying data for job/node-test-pull-request/56816/ ✔ 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 51481 From https://github.com/nodejs/node * branch refs/pull/51481/merge -> FETCH_HEAD ✔ Fetched commits as eb4432c12c67..da6e2ca80b02 -------------------------------------------------------------------------------- [main d234630551] module: fix crash when built-in module export a `default` key Author: Antoine du Hamel Date: Tue Jan 16 01:17:29 2024 +0100 2 files changed, 10 insertions(+), 1 deletion(-) create mode 100644 test/parallel/test-process-default.js [main 3376c26fbc] Update lib/internal/bootstrap/realm.js Author: Antoine du Hamel Date: Tue Jan 16 10:35:42 2024 +0100 1 file changed, 2 insertions(+), 1 deletion(-) ✔ Patches applied There are 2 commits in the PR. Attempting autorebase. Rebasing (2/4)

Executing: git node land --amend --yes
--------------------------------- New Message ----------------------------------
module: fix crash when built-in module export a default key

PR-URL: #51481
Fixes: #51480
Reviewed-By: Chengzhong Wu legendecas@gmail.com
Reviewed-By: Guy Bedford guybedford@gmail.com
Reviewed-By: Geoffrey Booth webadmin@geoffreybooth.com

[detached HEAD a5e1029a22] module: fix crash when built-in module export a default key
Author: Antoine du Hamel duhamelantoine1995@gmail.com
Date: Tue Jan 16 01:17:29 2024 +0100
2 files changed, 10 insertions(+), 1 deletion(-)
create mode 100644 test/parallel/test-process-default.js
Rebasing (3/4)
Rebasing (4/4)

Executing: git node land --amend --yes
--------------------------------- New Message ----------------------------------
Update lib/internal/bootstrap/realm.js

Co-authored-by: Geoffrey Booth webadmin@geoffreybooth.com
PR-URL: #51481
Fixes: #51480
Reviewed-By: Chengzhong Wu legendecas@gmail.com
Reviewed-By: Guy Bedford guybedford@gmail.com
Reviewed-By: Geoffrey Booth webadmin@geoffreybooth.com

[detached HEAD 7e9f2cd03d] Update lib/internal/bootstrap/realm.js
Author: Antoine du Hamel duhamelantoine1995@gmail.com
Date: Tue Jan 16 10:35:42 2024 +0100
1 file changed, 2 insertions(+), 1 deletion(-)

Successfully rebased and updated refs/heads/main.

ℹ Add commit-queue-squash label to land the PR as one commit, or commit-queue-rebase to land as separate commits.

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

nodejs-github-bot · 2024-01-19T08:42:09Z

Landed in a772973

PR-URL: nodejs#51481 Fixes: nodejs#51480 Reviewed-By: Chengzhong Wu <legendecas@gmail.com> Reviewed-By: Guy Bedford <guybedford@gmail.com> Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>

PR-URL: #51481 Fixes: #51480 Reviewed-By: Chengzhong Wu <legendecas@gmail.com> Reviewed-By: Guy Bedford <guybedford@gmail.com> Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>

PR-URL: nodejs#51481 Fixes: nodejs#51480 Reviewed-By: Chengzhong Wu <legendecas@gmail.com> Reviewed-By: Guy Bedford <guybedford@gmail.com> Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>

PR-URL: #51481 Fixes: #51480 Reviewed-By: Chengzhong Wu <legendecas@gmail.com> Reviewed-By: Guy Bedford <guybedford@gmail.com> Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>

module: fix crash when built-in module export a default key

c03d5ee

nodejs-github-bot added lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. labels Jan 16, 2024

legendecas approved these changes Jan 16, 2024

View reviewed changes

guybedford approved these changes Jan 16, 2024

View reviewed changes

GeoffreyBooth approved these changes Jan 16, 2024

View reviewed changes

lib/internal/bootstrap/realm.js Outdated Show resolved Hide resolved

Update lib/internal/bootstrap/realm.js

da6e2ca

Co-authored-by: Geoffrey Booth <webadmin@geoffreybooth.com>

aduh95 added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. request-ci Add this label to start a Jenkins CI on a PR. labels Jan 16, 2024

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

aymen94 approved these changes Jan 16, 2024

View reviewed changes

debadree25 added the commit-queue Add this label to land a pull request using GitHub Actions. label Jan 19, 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 Jan 19, 2024

legendecas approved these changes Jan 19, 2024

View reviewed changes

legendecas 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 Jan 19, 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 Jan 19, 2024

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

nodejs-github-bot merged commit a772973 into nodejs:main Jan 19, 2024
71 checks passed

aduh95 deleted the process-default branch January 19, 2024 09:04

marco-ippolito mentioned this pull request Mar 1, 2024

v21.7.0 proposal #51932

Merged

richardlau mentioned this pull request Mar 25, 2024

v20.12.0 proposal #52212

Merged

This pull request was closed.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

module: fix crash when built-in module export a `default` key #51481

module: fix crash when built-in module export a `default` key #51481

aduh95 commented Jan 16, 2024

nodejs-github-bot commented Jan 16, 2024

guybedford commented Jan 16, 2024

guybedford left a comment

nodejs-github-bot commented Jan 16, 2024

nodejs-github-bot commented Jan 16, 2024

nodejs-github-bot commented Jan 19, 2024

nodejs-github-bot commented Jan 19, 2024

PR-URL: #51481
Fixes: #51480
Reviewed-By: Chengzhong Wu legendecas@gmail.com
Reviewed-By: Guy Bedford guybedford@gmail.com
Reviewed-By: Geoffrey Booth webadmin@geoffreybooth.com

Co-authored-by: Geoffrey Booth webadmin@geoffreybooth.com
PR-URL: #51481
Fixes: #51480
Reviewed-By: Chengzhong Wu legendecas@gmail.com
Reviewed-By: Guy Bedford guybedford@gmail.com
Reviewed-By: Geoffrey Booth webadmin@geoffreybooth.com

Successfully rebased and updated refs/heads/main.

nodejs-github-bot commented Jan 19, 2024

module: fix crash when built-in module export a default key #51481

module: fix crash when built-in module export a default key #51481

Conversation

aduh95 commented Jan 16, 2024

nodejs-github-bot commented Jan 16, 2024

guybedford commented Jan 16, 2024

guybedford left a comment

Choose a reason for hiding this comment

nodejs-github-bot commented Jan 16, 2024

nodejs-github-bot commented Jan 16, 2024

nodejs-github-bot commented Jan 19, 2024

nodejs-github-bot commented Jan 19, 2024

PR-URL: #51481 Fixes: #51480 Reviewed-By: Chengzhong Wu legendecas@gmail.com Reviewed-By: Guy Bedford guybedford@gmail.com Reviewed-By: Geoffrey Booth webadmin@geoffreybooth.com

Co-authored-by: Geoffrey Booth webadmin@geoffreybooth.com PR-URL: #51481 Fixes: #51480 Reviewed-By: Chengzhong Wu legendecas@gmail.com Reviewed-By: Guy Bedford guybedford@gmail.com Reviewed-By: Geoffrey Booth webadmin@geoffreybooth.com

Successfully rebased and updated refs/heads/main.

nodejs-github-bot commented Jan 19, 2024

module: fix crash when built-in module export a `default` key #51481

module: fix crash when built-in module export a `default` key #51481

PR-URL: #51481
Fixes: #51480
Reviewed-By: Chengzhong Wu legendecas@gmail.com
Reviewed-By: Guy Bedford guybedford@gmail.com
Reviewed-By: Geoffrey Booth webadmin@geoffreybooth.com

Co-authored-by: Geoffrey Booth webadmin@geoffreybooth.com
PR-URL: #51481
Fixes: #51480
Reviewed-By: Chengzhong Wu legendecas@gmail.com
Reviewed-By: Guy Bedford guybedford@gmail.com
Reviewed-By: Geoffrey Booth webadmin@geoffreybooth.com