Skip to content

src: allow embedder control of code generation policy #46368

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.

Sign up for GitHub

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

Jump to bottom
Merged
nodejs-github-bot merged 1 commit into from
Feb 3, 2023
Merged

src: allow embedder control of code generation policy #46368

nodejs-github-bot merged 1 commit into from
Feb 3, 2023

Conversation

@codebytere
Copy link
Member

@codebytere codebytere commented Jan 26, 2023

Refs electron/electron#36567.

This PR allows for embedder control over whether or not code generation from strings is allowed. This is particularly important for embedders that combine Node and Blink, as Electron does, because Blink must be able to make Content Security Policy-based decisions. Without the ability to control this, we experience crashes.

This allows Electron to remove a patch we currently float to enable this.

@codebytere codebytere added the embedding Issues and PRs related to embedding Node.js in another project. label Jan 26, 2023
@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run. labels Jan 26, 2023
@targos
Copy link
Member

targos commented Jan 26, 2023

@nodejs/cpp-reviewers

@addaleax addaleax added the semver-major PRs that contain breaking changes and should be released in the next major version. label Jan 26, 2023
addaleax
addaleax approved these changes Jan 26, 2023
View reviewed changes
mhdawson
mhdawson approved these changes Jan 27, 2023
View reviewed changes
Copy link
Member

@mhdawson mhdawson left a comment

Choose a reason for hiding this comment

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

LGTM

@codebytere codebytere force-pushed the allow-embedder-codegen-control branch from 8c13d11 to fb174f5 Compare January 30, 2023 19:30
@addaleax addaleax 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 30, 2023
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jan 30, 2023
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Jan 30, 2023

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

@codebytere codebytere force-pushed the allow-embedder-codegen-control branch from fb174f5 to 78e7268 Compare January 30, 2023 19:52
@addaleax addaleax added the request-ci Add this label to start a Jenkins CI on a PR. label Jan 30, 2023
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jan 30, 2023
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Jan 30, 2023

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

@codebytere codebytere added semver-minor PRs that contain new features and should be released in the next minor version. and removed semver-major PRs that contain breaking changes and should be released in the next major version. labels Jan 31, 2023
@codebytere codebytere force-pushed the allow-embedder-codegen-control branch from 78e7268 to 885fce8 Compare January 31, 2023 10:53
@codebytere codebytere added the request-ci Add this label to start a Jenkins CI on a PR. label Jan 31, 2023
addaleax
addaleax approved these changes Jan 31, 2023
View reviewed changes
Copy link
Member

@addaleax addaleax left a comment

Choose a reason for hiding this comment

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

(just re-approving so that CQ doesn’t complain when merging this)

@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jan 31, 2023
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Jan 31, 2023

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

@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Feb 2, 2023

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

@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Feb 2, 2023

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

@codebytere codebytere added the commit-queue Add this label to land a pull request using GitHub Actions. label Feb 3, 2023
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Feb 3, 2023
@nodejs-github-bot nodejs-github-bot merged commit 3aaeb30 into nodejs:main Feb 3, 2023
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Feb 3, 2023

Landed in 3aaeb30

@github-actions github-actions bot mentioned this pull request Feb 5, 2023
Open
22 tasks
MylesBorins pushed a commit that referenced this pull request Feb 18, 2023
@codebytere @MylesBorins
src: allow embedder control of code generation policy
e77d538 
PR-URL: #46368
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Rafael Gonzaga <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
MylesBorins added a commit that referenced this pull request Feb 19, 2023
@MylesBorins
2023-02-21, Version 19.7.0 (Current)
eb53c0f 
Notable changes:

deps:
  * upgrade npm to 9.5.0 (npm team) #46673
  * add ada as a dependency (Yagiz Nizipli) #46410
doc:
  * add debadree25 to collaborators (Debadree Chatterjee) #46716
  * add deokjinkim to collaborators (Deokjin Kim) #46444
doc,lib,src,test:
  * rename --test-coverage (Colin Ihrig) #46017
lib:
  * (SEMVER-MINOR) add aborted() utility function (Debadree Chatterjee) #46494
src:
  * (SEMVER-MINOR) add initial support for single executable applications (Darshan Sen) #45038
  * (SEMVER-MINOR) allow optional Isolate termination in node::Stop() (Shelley Vohr) #46583
  * (SEMVER-MINOR) allow blobs in addition to `FILE*`s in embedder snapshot API (Anna Henningsen) #46491
  * (SEMVER-MINOR) allow snapshotting from the embedder API (Anna Henningsen) #45888
  * (SEMVER-MINOR) make build_snapshot a per-Isolate option, rather than a global one (Anna Henningsen) #45888
  * (SEMVER-MINOR) add snapshot support for embedder API (Anna Henningsen) #45888
  * (SEMVER-MINOR) allow embedder control of code generation policy (Shelley Vohr) #46368
stream:
  * (SEMVER-MINOR) add abort signal for ReadableStream and WritableStream (Debadree Chatterjee) #46273
test_runner:
  * add initial code coverage support (Colin Ihrig) #46017
url:
  * replace url-parser with ada (Yagiz Nizipli) #46410

PR-URL: TODO
@MylesBorins MylesBorins mentioned this pull request Feb 19, 2023
Merged
MylesBorins added a commit that referenced this pull request Feb 20, 2023
@MylesBorins
2023-02-21, Version 19.7.0 (Current)
051c9c3 
Notable changes:

deps:
  * upgrade npm to 9.5.0 (npm team) #46673
  * add ada as a dependency (Yagiz Nizipli) #46410
doc:
  * add debadree25 to collaborators (Debadree Chatterjee) #46716
  * add deokjinkim to collaborators (Deokjin Kim) #46444
doc,lib,src,test:
  * rename --test-coverage (Colin Ihrig) #46017
lib:
  * (SEMVER-MINOR) add aborted() utility function (Debadree Chatterjee) #46494
src:
  * (SEMVER-MINOR) add initial support for single executable applications (Darshan Sen) #45038
  * (SEMVER-MINOR) allow optional Isolate termination in node::Stop() (Shelley Vohr) #46583
  * (SEMVER-MINOR) allow blobs in addition to `FILE*`s in embedder snapshot API (Anna Henningsen) #46491
  * (SEMVER-MINOR) allow snapshotting from the embedder API (Anna Henningsen) #45888
  * (SEMVER-MINOR) make build_snapshot a per-Isolate option, rather than a global one (Anna Henningsen) #45888
  * (SEMVER-MINOR) add snapshot support for embedder API (Anna Henningsen) #45888
  * (SEMVER-MINOR) allow embedder control of code generation policy (Shelley Vohr) #46368
stream:
  * (SEMVER-MINOR) add abort signal for ReadableStream and WritableStream (Debadree Chatterjee) #46273
test_runner:
  * add initial code coverage support (Colin Ihrig) #46017
url:
  * replace url-parser with ada (Yagiz Nizipli) #46410

PR-URL: #46725
MylesBorins added a commit to MylesBorins/node that referenced this pull request Feb 20, 2023
@MylesBorins
2023-02-21, Version 19.7.0 (Current)
397dc01 
Notable changes:

deps:
  * upgrade npm to 9.5.0 (npm team) nodejs#46673
  * add ada as a dependency (Yagiz Nizipli) nodejs#46410
doc:
  * add debadree25 to collaborators (Debadree Chatterjee) nodejs#46716
  * add deokjinkim to collaborators (Deokjin Kim) nodejs#46444
doc,lib,src,test:
  * rename --test-coverage (Colin Ihrig) nodejs#46017
lib:
  * (SEMVER-MINOR) add aborted() utility function (Debadree Chatterjee) nodejs#46494
src:
  * (SEMVER-MINOR) add initial support for single executable applications (Darshan Sen) nodejs#45038
  * (SEMVER-MINOR) allow optional Isolate termination in node::Stop() (Shelley Vohr) nodejs#46583
  * (SEMVER-MINOR) allow blobs in addition to `FILE*`s in embedder snapshot API (Anna Henningsen) nodejs#46491
  * (SEMVER-MINOR) allow snapshotting from the embedder API (Anna Henningsen) nodejs#45888
  * (SEMVER-MINOR) make build_snapshot a per-Isolate option, rather than a global one (Anna Henningsen) nodejs#45888
  * (SEMVER-MINOR) add snapshot support for embedder API (Anna Henningsen) nodejs#45888
  * (SEMVER-MINOR) allow embedder control of code generation policy (Shelley Vohr) nodejs#46368
stream:
  * (SEMVER-MINOR) add abort signal for ReadableStream and WritableStream (Debadree Chatterjee) nodejs#46273
test_runner:
  * add initial code coverage support (Colin Ihrig) nodejs#46017
url:
  * replace url-parser with ada (Yagiz Nizipli) nodejs#46410

PR-URL: nodejs#46725
MylesBorins added a commit that referenced this pull request Feb 20, 2023
@MylesBorins
2023-02-21, Version 19.7.0 (Current)
3ee97ee 
Notable changes:

deps:
  * upgrade npm to 9.5.0 (npm team) #46673
  * add ada as a dependency (Yagiz Nizipli) #46410
doc:
  * add debadree25 to collaborators (Debadree Chatterjee) #46716
  * add deokjinkim to collaborators (Deokjin Kim) #46444
doc,lib,src,test:
  * rename --test-coverage (Colin Ihrig) #46017
lib:
  * (SEMVER-MINOR) add aborted() utility function (Debadree Chatterjee) #46494
src:
  * (SEMVER-MINOR) add initial support for single executable applications (Darshan Sen) #45038
  * (SEMVER-MINOR) allow optional Isolate termination in node::Stop() (Shelley Vohr) #46583
  * (SEMVER-MINOR) allow blobs in addition to `FILE*`s in embedder snapshot API (Anna Henningsen) #46491
  * (SEMVER-MINOR) allow snapshotting from the embedder API (Anna Henningsen) #45888
  * (SEMVER-MINOR) make build_snapshot a per-Isolate option, rather than a global one (Anna Henningsen) #45888
  * (SEMVER-MINOR) add snapshot support for embedder API (Anna Henningsen) #45888
  * (SEMVER-MINOR) allow embedder control of code generation policy (Shelley Vohr) #46368
stream:
  * (SEMVER-MINOR) add abort signal for ReadableStream and WritableStream (Debadree Chatterjee) #46273
test_runner:
  * add initial code coverage support (Colin Ihrig) #46017
url:
  * replace url-parser with ada (Yagiz Nizipli) #46410

PR-URL: #46725
MylesBorins added a commit that referenced this pull request Feb 20, 2023
@MylesBorins
2023-02-21, Version 19.7.0 (Current)
a7dbed7 
Notable changes:

deps:
  * upgrade npm to 9.5.0 (npm team) #46673
  * add ada as a dependency (Yagiz Nizipli) #46410
doc:
  * add debadree25 to collaborators (Debadree Chatterjee) #46716
  * add deokjinkim to collaborators (Deokjin Kim) #46444
doc,lib,src,test:
  * rename --test-coverage (Colin Ihrig) #46017
lib:
  * (SEMVER-MINOR) add aborted() utility function (Debadree Chatterjee) #46494
src:
  * (SEMVER-MINOR) add initial support for single executable applications (Darshan Sen) #45038
  * (SEMVER-MINOR) allow optional Isolate termination in node::Stop() (Shelley Vohr) #46583
  * (SEMVER-MINOR) allow blobs in addition to `FILE*`s in embedder snapshot API (Anna Henningsen) #46491
  * (SEMVER-MINOR) allow snapshotting from the embedder API (Anna Henningsen) #45888
  * (SEMVER-MINOR) make build_snapshot a per-Isolate option, rather than a global one (Anna Henningsen) #45888
  * (SEMVER-MINOR) add snapshot support for embedder API (Anna Henningsen) #45888
  * (SEMVER-MINOR) allow embedder control of code generation policy (Shelley Vohr) #46368
stream:
  * (SEMVER-MINOR) add abort signal for ReadableStream and WritableStream (Debadree Chatterjee) #46273
test_runner:
  * add initial code coverage support (Colin Ihrig) #46017
url:
  * replace url-parser with ada (Yagiz Nizipli) #46410

PR-URL: #46725
@codebytere codebytere deleted the allow-embedder-codegen-control branch April 2, 2023 14:33
@danielleadams danielleadams mentioned this pull request Apr 11, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@addaleax addaleax addaleax approved these changes

@mhdawson mhdawson mhdawson approved these changes

@RafaelGSS RafaelGSS RafaelGSS approved these changes

Assignees

No one assigned

Labels

author ready PRs that have at least one approval, no pending requests for changes, and a CI started. c++ Issues and PRs that require attention from people who are familiar with C++. embedding Issues and PRs related to embedding Node.js in another project. needs-ci PRs that need a full CI run. semver-minor PRs that contain new features and should be released in the next minor version.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@codebytere @targos @nodejs-github-bot @addaleax @mhdawson @RafaelGSS