Skip to content

crypto: fix inclusion of OPENSSL_IS_BORINGSSL define - #58845

Merged
nodejs-github-bot merged 2 commits into
nodejs:mainfrom
codebytere:fix-have-crypto
Jul 1, 2025
Merged

crypto: fix inclusion of OPENSSL_IS_BORINGSSL define#58845
nodejs-github-bot merged 2 commits into
nodejs:mainfrom
codebytere:fix-have-crypto

Conversation

@codebytere

Copy link
Copy Markdown
Member

Refs #58387.

Fixes OPENSSL_IS_BORINGSSL not being defined in cases where it should be due to missing include. See relevant BoringSSL file

@codebytere
codebytere requested review from jasnell and tniessen June 26, 2025 12:59
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/config

@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 Jun 26, 2025
@codebytere

codebytere commented Jun 26, 2025

Copy link
Copy Markdown
Member Author

@jasnell looks like openssl itself doesn't have base.h - looks like to ensure it works I just need to include a file that normal OpenSSL does export, bc in BoringSSL they all include base.h. Do you have a preference?

See ex:

Screenshot 2025-06-26 at 3 17 36 PM

@codecov

codecov Bot commented Jun 26, 2025

Copy link
Copy Markdown

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.10%. Comparing base (b4c5fb4) to head (a0c73cd).
Report is 33 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #58845      +/-   ##
==========================================
+ Coverage   90.08%   90.10%   +0.02%     
==========================================
  Files         640      640              
  Lines      188446   188426      -20     
  Branches    36960    36953       -7     
==========================================
+ Hits       169757   169786      +29     
+ Misses      11412    11350      -62     
- Partials     7277     7290      +13     
Files with missing lines Coverage Δ
src/node_config.cc 96.55% <ø> (ø)

... and 46 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@jasnell

jasnell commented Jun 27, 2025

Copy link
Copy Markdown
Member

No particular preference @codebytere ... I'd trust whatever you think needs to be done and whatever makes it work for you

@codebytere codebytere added request-ci Add this label to start a Jenkins CI on a PR. and removed needs-ci PRs that need a full CI run. labels Jun 27, 2025
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jun 27, 2025
@nodejs-github-bot

This comment was marked as outdated.

Comment thread src/node_config.cc Outdated
Co-authored-by: Anna Henningsen <github@addaleax.net>
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@codebytere codebytere added the commit-queue PRs queued for automated landing through the Commit Queue. label Jul 1, 2025
@nodejs-github-bot nodejs-github-bot added commit-queue-failed PRs whose Commit Queue landing failed and need manual intervention before retrying. and removed commit-queue PRs queued for automated landing through the Commit Queue. labels Jul 1, 2025
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator
Commit Queue failed
- Loading data for nodejs/node/pull/58845
✔  Done loading data for nodejs/node/pull/58845
----------------------------------- PR info ------------------------------------
Title      crypto: fix inclusion of `OPENSSL_IS_BORINGSSL` define (#58845)
Author     Shelley Vohr <shelley.vohr@gmail.com> (@codebytere)
Branch     codebytere:fix-have-crypto -> nodejs:main
Labels     c++
Commits    2
 - crypto: fix inclusion of OPENSSL_IS_BORINGSSL define
 - Update src/node_config.cc
Committers 1
 - Shelley Vohr <shelley.vohr@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/58845
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ethan Arrowood <ethan@arrowood.dev>
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/58845
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ethan Arrowood <ethan@arrowood.dev>
--------------------------------------------------------------------------------
   ℹ  This PR was created on Thu, 26 Jun 2025 12:59:57 GMT
   ✔  Approvals: 5
   ✔  - James M Snell (@jasnell) (TSC): https://github.com/nodejs/node/pull/58845#pullrequestreview-2970781596
   ✔  - Juan José Arboleda (@juanarbol): https://github.com/nodejs/node/pull/58845#pullrequestreview-2962886452
   ✔  - Anna Henningsen (@addaleax): https://github.com/nodejs/node/pull/58845#pullrequestreview-2967069855
   ✔  - Luigi Pinca (@lpinca): https://github.com/nodejs/node/pull/58845#pullrequestreview-2967665439
   ✔  - Ethan Arrowood (@Ethan-Arrowood): https://github.com/nodejs/node/pull/58845#pullrequestreview-2972085344
   ✔  Last GitHub CI successful
   ℹ  Last Full PR CI on 2025-06-30T18:46:52Z: https://ci.nodejs.org/job/node-test-pull-request/67753/
- Querying data for job/node-test-pull-request/67753/
   ✔  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 58845
From https://github.com/nodejs/node
 * branch                  refs/pull/58845/merge -> FETCH_HEAD
✔  Fetched commits as 2022f832a98b..a0c73cda8176
--------------------------------------------------------------------------------
[main 77a12159c6] crypto: fix inclusion of OPENSSL_IS_BORINGSSL define
 Author: Shelley Vohr <shelley.vohr@gmail.com>
 Date: Thu Jun 26 14:57:05 2025 +0200
 1 file changed, 4 insertions(+)
[main baccb14ceb] Update src/node_config.cc
 Author: Shelley Vohr <shelley.vohr@gmail.com>
 Date: Mon Jun 30 10:16:10 2025 +0200
 1 file changed, 1 insertion(+), 1 deletion(-)
   ✔  Patches applied
There are 2 commits in the PR. Attempting autorebase.
Rebasing (2/4)
Executing: git node land --amend --yes
--------------------------------- New Message ----------------------------------
crypto: fix inclusion of OPENSSL_IS_BORINGSSL define

PR-URL: #58845
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ethan Arrowood <ethan@arrowood.dev>

[detached HEAD 9574c03bfe] crypto: fix inclusion of OPENSSL_IS_BORINGSSL define
Author: Shelley Vohr <shelley.vohr@gmail.com>
Date: Thu Jun 26 14:57:05 2025 +0200
1 file changed, 4 insertions(+)
Rebasing (3/4)
Rebasing (4/4)
Executing: git node land --amend --yes
--------------------------------- New Message ----------------------------------
Update src/node_config.cc

Co-authored-by: Anna Henningsen <github@addaleax.net>
PR-URL: #58845
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ethan Arrowood <ethan@arrowood.dev>

[detached HEAD 243b9df2f2] Update src/node_config.cc
Author: Shelley Vohr <shelley.vohr@gmail.com>
Date: Mon Jun 30 10:16:10 2025 +0200
1 file changed, 1 insertion(+), 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/15993773799

@richardlau richardlau removed the commit-queue-failed PRs whose Commit Queue landing failed and need manual intervention before retrying. label Jul 1, 2025
@richardlau richardlau 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 Jul 1, 2025
@nodejs-github-bot nodejs-github-bot removed the commit-queue PRs queued for automated landing through the Commit Queue. label Jul 1, 2025
@nodejs-github-bot
nodejs-github-bot merged commit a7a37c3 into nodejs:main Jul 1, 2025
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Landed in a7a37c3

targos pushed a commit that referenced this pull request Jul 3, 2025
PR-URL: #58845
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ethan Arrowood <ethan@arrowood.dev>
Reviewed-By: Richard Lau <rlau@redhat.com>
aduh95 pushed a commit that referenced this pull request Jul 21, 2025
PR-URL: #58845
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ethan Arrowood <ethan@arrowood.dev>
Reviewed-By: Richard Lau <rlau@redhat.com>
aduh95 pushed a commit that referenced this pull request Jul 24, 2025
PR-URL: #58845
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ethan Arrowood <ethan@arrowood.dev>
Reviewed-By: Richard Lau <rlau@redhat.com>
codebytere added a commit to electron/electron that referenced this pull request Aug 2, 2025
jkleinsc pushed a commit to electron/electron that referenced this pull request Aug 4, 2025
* chore: bump node in DEPS to v22.18.0

* crypto: fix inclusion of OPENSSL_IS_BORINGSSL define

nodejs/node#58845

* crypto: fix SHAKE128/256 breaking change introduced with OpenSSL 3.4

nodejs/node#58960

* permission: propagate permission model flags on spawn

nodejs/node#58853

* esm: syncify default path of ModuleLoader\.load

nodejs/node#57419

* src: remove fast API for InternalModuleStat

nodejs/node#58489

* src: simplify adding fast APIs to ExternalReferenceRegistry

nodejs/node#58896

* chore: fixup patch indices

* src: fix internalModuleStat v8 fast path

nodejs/node#58054

* test: add tests to ensure that node.1 is kept in sync with cli.md

nodejs/node#58878

* crypto: fix SHAKE128/256 breaking change introduced with OpenSSL 3.4

nodejs/node#58942

---------

Co-authored-by: electron-roller[bot] <84116207+electron-roller[bot]@users.noreply.github.com>
Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com>
codebytere added a commit to electron/electron that referenced this pull request Aug 5, 2025
* chore: bump node in DEPS to v22.18.0

* crypto: fix inclusion of OPENSSL_IS_BORINGSSL define

nodejs/node#58845

* crypto: fix SHAKE128/256 breaking change introduced with OpenSSL 3.4

nodejs/node#58960

* permission: propagate permission model flags on spawn

nodejs/node#58853

* esm: syncify default path of ModuleLoader\.load

nodejs/node#57419

* src: remove fast API for InternalModuleStat

nodejs/node#58489

* src: simplify adding fast APIs to ExternalReferenceRegistry

nodejs/node#58896

* chore: fixup patch indices

* src: fix internalModuleStat v8 fast path

nodejs/node#58054

* test: add tests to ensure that node.1 is kept in sync with cli.md

nodejs/node#58878

* crypto: fix SHAKE128/256 breaking change introduced with OpenSSL 3.4

nodejs/node#58942

---------

Co-authored-by: electron-roller[bot] <84116207+electron-roller[bot]@users.noreply.github.com>
Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com>
codebytere added a commit to electron/electron that referenced this pull request Aug 5, 2025
chore: bump node to v22.18.0 (main) (#47937)

* chore: bump node in DEPS to v22.18.0

* crypto: fix inclusion of OPENSSL_IS_BORINGSSL define

nodejs/node#58845

* crypto: fix SHAKE128/256 breaking change introduced with OpenSSL 3.4

nodejs/node#58960

* permission: propagate permission model flags on spawn

nodejs/node#58853

* esm: syncify default path of ModuleLoader\.load

nodejs/node#57419

* src: remove fast API for InternalModuleStat

nodejs/node#58489

* src: simplify adding fast APIs to ExternalReferenceRegistry

nodejs/node#58896

* chore: fixup patch indices

* src: fix internalModuleStat v8 fast path

nodejs/node#58054

* test: add tests to ensure that node.1 is kept in sync with cli.md

nodejs/node#58878

* crypto: fix SHAKE128/256 breaking change introduced with OpenSSL 3.4

nodejs/node#58942

---------

Co-authored-by: electron-roller[bot] <84116207+electron-roller[bot]@users.noreply.github.com>
Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com>
kigh-ota pushed a commit to kigh-ota/electron that referenced this pull request Sep 30, 2025
* chore: bump node in DEPS to v22.18.0

* crypto: fix inclusion of OPENSSL_IS_BORINGSSL define

nodejs/node#58845

* crypto: fix SHAKE128/256 breaking change introduced with OpenSSL 3.4

nodejs/node#58960

* permission: propagate permission model flags on spawn

nodejs/node#58853

* esm: syncify default path of ModuleLoader\.load

nodejs/node#57419

* src: remove fast API for InternalModuleStat

nodejs/node#58489

* src: simplify adding fast APIs to ExternalReferenceRegistry

nodejs/node#58896

* chore: fixup patch indices

* src: fix internalModuleStat v8 fast path

nodejs/node#58054

* test: add tests to ensure that node.1 is kept in sync with cli.md

nodejs/node#58878

* crypto: fix SHAKE128/256 breaking change introduced with OpenSSL 3.4

nodejs/node#58942

---------

Co-authored-by: electron-roller[bot] <84116207+electron-roller[bot]@users.noreply.github.com>
Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c++ Issues and PRs that require attention from people who are familiar with C++. commit-queue-squash PRs the Commit Queue should land as one squashed commit.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants