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

FreeBSD openssl version string doesn't match regex test #43078

Closed
derekschrock opened this issue May 12, 2022 · 4 comments · Fixed by #50505
Closed

FreeBSD openssl version string doesn't match regex test #43078

derekschrock opened this issue May 12, 2022 · 4 comments · Fixed by #50505
Labels
flaky-test Issues and PRs related to the tests with unstable failures on the CI.

Comments

@derekschrock
Copy link

Test

test-process-versions

Platform

FreeBSD

Console output

assert.js:394
    throw err;
    ^

AssertionError [ERR_ASSERTION]: The expression evaluated to a falsy value:

  assert(/^\d+\.\d+\.\d+[a-z]?(-fips)?$/.test(process.versions.openssl))

    at Object.<anonymous> (/usr/home/dereks/repos/node/test/parallel/test-process-versions.js:48:3)
    at Module._compile (internal/modules/cjs/loader.js:1085:14)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:1114:10)
    at Module.load (internal/modules/cjs/loader.js:950:32)
    at Function.Module._load (internal/modules/cjs/loader.js:790:12)
    at Function.executeUserEntryPoint [as runMain] (internal/modules/run_main.js:76:12)
    at internal/main/run_main_module.js:17:47 {
  generatedMessage: true,
  code: 'ERR_ASSERTION',
  actual: false,
  expected: true,
  operator: '=='
}

Build links

No response

Additional information

node output of process.versions.openssl

> process.versions.openssl
'1.1.1k-freebsd'

It seems the regex could change to:

/^\d+\.\d+\.\d+[a-z]?(-freebsd|-fips)?$

Adding the -freebsd

@derekschrock derekschrock added the flaky-test Issues and PRs related to the tests with unstable failures on the CI. label May 12, 2022
@Trott
Copy link
Member

Trott commented May 13, 2022

Did you compile Node.js yourself? If so, what flags did you pass to ./configure?

@derekschrock
Copy link
Author

This would have been from FreeBSD poudriere using the ports tree updated for 14.9.2 WIP. This would have used the base system openssl due to ... --shared-openssl ...

args:

--prefix=/usr/local  --shared-brotli  --shared-cares  --shared-libuv  --shared-nghttp2  --shared-zlib  --without-npm --openssl-use-def-ca-store  --shared-openssl --with-dtrace --with-intl=system-icu              

ENV:

MAKE=gmake PKG_CONFIG=pkgconf PYTHON="/usr/local/bin/python3.8" XDG_DATA_HOME=/wrkdirs/usr/ports/www/node14/work  XDG_CONFIG_HOME=/wrkdirs/usr/ports/www/node14/work  XDG_CACHE_HOME=/wrkdirs/usr/ports/www/node14/work/.cache  HOME=/wrkdirs/usr/ports/www/node14/work
TMPDIR="/tmp" PATH=/wrkdirs/usr/ports/www/node14/work/.bin:/sbin:/bin:/usr/sbin:/usr/bin:/usr/local/sbin:/usr/local/bin:/root/bin SHELL=/bin/sh CONFIG_SHELL=/bin/sh CMAKE_PREFIX_PATH="/usr/local"                                                

@Trott
Copy link
Member

Trott commented May 13, 2022

@nodejs/testing @nodejs/crypto I don't think adding the string "freebsd" is the way to go, but maybe changing the allowance for "fips" to be a general allowance of alpha or alphanumeric strings? We might also add some logic to detect built-in vs. system OpenSSL? Maybe skip the check entirely if OpenSSL is the system one, or at least make it much more lax?

@mhdawson
Copy link
Member

I think when built with a shared openssl allowing pretty much anything for process.versions.openssl is reasonable. Maybe just require that it is defined?

lpinca added a commit to lpinca/node that referenced this issue Nov 1, 2023
Relax the OpenSSL version check when Node.js is built with the
`--shared-openssl` option. Verify only that `process.versions.openssl`
is truthy.

Fixes: nodejs#43078
nodejs-github-bot pushed a commit that referenced this issue Nov 8, 2023
Relax the OpenSSL version check when Node.js is built with the
`--shared-openssl` option. Verify only that `process.versions.openssl`
is truthy.

Fixes: #43078
PR-URL: #50505
Reviewed-By: Richard Lau <rlau@redhat.com>
targos pushed a commit that referenced this issue Nov 11, 2023
Relax the OpenSSL version check when Node.js is built with the
`--shared-openssl` option. Verify only that `process.versions.openssl`
is truthy.

Fixes: #43078
PR-URL: #50505
Reviewed-By: Richard Lau <rlau@redhat.com>
targos pushed a commit that referenced this issue Nov 14, 2023
Relax the OpenSSL version check when Node.js is built with the
`--shared-openssl` option. Verify only that `process.versions.openssl`
is truthy.

Fixes: #43078
PR-URL: #50505
Reviewed-By: Richard Lau <rlau@redhat.com>
targos pushed a commit that referenced this issue Nov 15, 2023
Relax the OpenSSL version check when Node.js is built with the
`--shared-openssl` option. Verify only that `process.versions.openssl`
is truthy.

Fixes: #43078
PR-URL: #50505
Reviewed-By: Richard Lau <rlau@redhat.com>
targos pushed a commit that referenced this issue Nov 15, 2023
Relax the OpenSSL version check when Node.js is built with the
`--shared-openssl` option. Verify only that `process.versions.openssl`
is truthy.

Fixes: #43078
PR-URL: #50505
Reviewed-By: Richard Lau <rlau@redhat.com>
sercher added a commit to sercher/graaljs that referenced this issue Apr 25, 2024
Relax the OpenSSL version check when Node.js is built with the
`--shared-openssl` option. Verify only that `process.versions.openssl`
is truthy.

Fixes: nodejs/node#43078
PR-URL: nodejs/node#50505
Reviewed-By: Richard Lau <rlau@redhat.com>
sercher added a commit to sercher/graaljs that referenced this issue Apr 25, 2024
Relax the OpenSSL version check when Node.js is built with the
`--shared-openssl` option. Verify only that `process.versions.openssl`
is truthy.

Fixes: nodejs/node#43078
PR-URL: nodejs/node#50505
Reviewed-By: Richard Lau <rlau@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
flaky-test Issues and PRs related to the tests with unstable failures on the CI.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants