Skip to content

Commit

Permalink
fix: stop ignoring NODE_TLS_REJECT_UNAUTHORIZED when strictSSL is not…
Browse files Browse the repository at this point in the history
… defined (#316)

Currently NODE_TLS_REJECT_UNAUTHORIZED is simply ignored as
options.rejectUnauthorized is always set to false when strictSSL is not
defined.

Most notably this causes issues for users behind corporate proxies using
npm and pnpm when installing a package that uses node-gyp. Example:
nodejs/node-gyp#2663

This change only takes into account NODE_TLS_REJECT_UNAUTHORIZED when
strictSSL is not passed to fetch.

unit tests were added to ensure strictSSL is still the primary driver.

Co-authored-by: Bruno Oliveira <[email protected]>
  • Loading branch information
brunoargolo and Bruno Oliveira authored Oct 21, 2024
1 parent 9cacb5b commit 3e0fe87
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 1 deletion.
7 changes: 6 additions & 1 deletion lib/options.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,12 @@ const conditionalHeaders = [
const configureOptions = (opts) => {
const { strictSSL, ...options } = { ...opts }
options.method = options.method ? options.method.toUpperCase() : 'GET'
options.rejectUnauthorized = strictSSL !== false

if (strictSSL === undefined || strictSSL === null) {
options.rejectUnauthorized = process.env.NODE_TLS_REJECT_UNAUTHORIZED !== '0'
} else {
options.rejectUnauthorized = strictSSL !== false
}

if (!options.retry) {
options.retry = { retries: 0 }
Expand Down
24 changes: 24 additions & 0 deletions test/options.js
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,30 @@ t.test('configure options', async (t) => {
'should treat strictSSL: null as true just like tls.connect')
})

t.test('when NODE_TLS_REJECT_UNAUTHORIZED is defined', async (t) => {
process.env.NODE_TLS_REJECT_UNAUTHORIZED = '0'

t.same(configureOptions({ }).rejectUnauthorized, false,
'should return rejectUnauthorized false when NODE_TLS_REJECT_UNAUTHORIZED="0" ' +
'and strictSSL is undefined')

t.same(configureOptions({ strictSSL: null }).rejectUnauthorized, false,
'should return rejectUnauthorized false when NODE_TLS_REJECT_UNAUTHORIZED="0" ' +
'and strictSSL is null')

t.same(configureOptions({ strictSSL: true }).rejectUnauthorized, true,
'should return rejectUnauthorized true when NODE_TLS_REJECT_UNAUTHORIZED="0" ' +
'and strictSSL is true')

process.env.NODE_TLS_REJECT_UNAUTHORIZED = '1'

t.same(configureOptions({ strictSSL: false }).rejectUnauthorized, false,
'should return rejectUnauthorized false when NODE_TLS_REJECT_UNAUTHORIZED="1" ' +
'and strictSSL is false')

process.env.NODE_TLS_REJECT_UNAUTHORIZED = undefined
})

t.test('should set dns property correctly', async (t) => {
t.test('no property given', async (t) => {
const actualOpts = { method: 'GET' }
Expand Down

0 comments on commit 3e0fe87

Please sign in to comment.