Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[v20.x] vm: harden module type checks #53109

Open
wants to merge 1 commit into
base: v20.x-staging
Choose a base branch
from

Conversation

legendecas
Copy link
Member

Check if the value returned from user linker function is a null-ish value.

validateInternalField should be preferred when checking this argument to guard against null-ish this.

Co-authored-by: Mike Ralphson mike.ralphson@gmail.com
PR-URL: #52162
Reviewed-By: Vinícius Lourenço Claro Cardoso contact@viniciusl.com.br
Reviewed-By: Yagiz Nizipli yagiz.nizipli@sentry.io

@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. v20.x v20.x Issues that can be reproduced on v20.x or PRs targeting the v20.x-staging branch. vm Issues and PRs related to the vm subsystem. labels May 22, 2024
@marco-ippolito marco-ippolito added the request-ci Add this label to start a Jenkins CI on a PR. label May 23, 2024
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label May 23, 2024
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@marco-ippolito marco-ippolito added the request-ci Add this label to start a Jenkins CI on a PR. label Jun 17, 2024
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jun 17, 2024
@nodejs-github-bot
Copy link
Collaborator

@marco-ippolito marco-ippolito force-pushed the v20.x-staging branch 2 times, most recently from d702971 to 044bcce Compare June 17, 2024 14:26
this[kDependencySpecifiers] = this[kWrap].getStaticDependencySpecifiers();
}
validateInternalField(this, kDependencySpecifiers, 'SourceTextModule');
this[kDependencySpecifiers] ??= ObjectFreeze(this[kWrap].getStaticDependencySpecifiers());
Copy link
Member

Choose a reason for hiding this comment

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

Isn't the freezing (from #49720) semver-major?

Copy link
Member

@marco-ippolito marco-ippolito left a comment

Choose a reason for hiding this comment

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

Blocking on #53109 (comment)

Check if the value returned from user linker function is a null-ish
value.

`validateInternalField` should be preferred when checking `this`
argument to guard against null-ish `this`.

Co-authored-by: Mike Ralphson <mike.ralphson@gmail.com>
PR-URL: nodejs#52162
Reviewed-By: Vinícius Lourenço Claro Cardoso <contact@viniciusl.com.br>
Reviewed-By: Yagiz Nizipli <yagiz.nizipli@sentry.io>
@legendecas legendecas added the request-ci Add this label to start a Jenkins CI on a PR. label Jun 18, 2024
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jun 18, 2024
@nodejs-github-bot
Copy link
Collaborator

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-ci PRs that need a full CI run. v20.x v20.x Issues that can be reproduced on v20.x or PRs targeting the v20.x-staging branch. vm Issues and PRs related to the vm subsystem.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants