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.

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

TLSSocket._tlsOptions undefined during TLSWrap.onerror cause an additional TypeError #41501

Closed
sigv opened this issue Jan 13, 2022 · 0 comments · Fixed by #41523
Closed

TLSSocket._tlsOptions undefined during TLSWrap.onerror cause an additional TypeError #41501

sigv opened this issue Jan 13, 2022 · 0 comments · Fixed by #41523
Labels
tls Issues and PRs related to the tls subsystem.

Comments

@sigv
Copy link
Contributor

sigv commented Jan 13, 2022

Version

v16.13.2

Platform

Linux example.com 5.11.0-1025-azure #27~20.04.1-Ubuntu SMP Fri Jan 7 15:02:06 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux

Subsystem

No response

What steps will reproduce the bug?

There is no clean repro case to provide. We are seeing what we believed to be a connection abort/reset issue caused by local router configuration. Something in our setup causes the TLSWrap.onerror to be called where owner has undefined _tlsOptions, potentially that TLS negotiation fails. /lib/_tls_wrap.js L411 is relevant, as visible in log below.

How often does it reproduce? Is there a required condition?

See above; conditions are not fully understood.

What is the expected behavior?

We expect to see a networking error, that informs up what exactly went wrong.
(Instead of it being suppressed by a TypeError.)

What do you see instead?

TypeError: Cannot read properties of undefined (reading 'isServer')
    at TLSWrap.onerror (node:_tls_wrap:411:27)
    at TLSWrap.callbackTrampoline (node:internal/async_hooks:130:17) {
  domainThrown: true
}

Additional information

No response
@Mesteery Mesteery added the tls Issues and PRs related to the tls subsystem. label Jan 13, 2022
sigv added a commit to sigv/node that referenced this issue Jan 14, 2022
@sigv
tls: avoid throw in onerror for bad TLSSocket obj
d16e1dd 
TLSWrap.onerror has a helpful debug() call built in to it. However in
case of a malformed TLSSocket object, where the `_tlsOptions` value is
an unexpected `undefined`, accessing `_tlsOptions.isServer` causes
a TypeError to be thrown.

This commit ensures that the debug() call properly logs the state as
'unknown', instead of the two 'server' and 'client' choices previously
available. Additionally, onerror branching is adjusted to allow such
`undefined` options object, by use of optional chaining.

Other methods are not being adjusted, as such a case of `undefined`
options is not viable during regular processing of the TLSSocket.

Fixes: nodejs#41501
sigv added a commit to sigv/node that referenced this issue Jan 14, 2022
@sigv
tls: avoid throw in onerror for bad TLSSocket obj
3f21973 
TLSWrap.onerror has a helpful debug() call built in to it. However in
case of a malformed TLSSocket object, where the `_tlsOptions` value is
an unexpected `undefined`, accessing `_tlsOptions.isServer` causes
a TypeError to be thrown.

This commit ensures that the debug() call properly logs the state as
'unknown', instead of the two 'server' and 'client' choices previously
available. Additionally, onerror branching is adjusted to allow such
`undefined` options object, by use of optional chaining.

Other methods are not being adjusted, as such a case of `undefined`
options is not viable during regular processing of the TLSSocket.

Fixes: nodejs#41501
@sigv sigv mentioned this issue Jan 14, 2022
Merged
sigv added a commit to sigv/node that referenced this issue Jan 14, 2022
@sigv
tls: avoid throw in onerror for bad TLSSocket obj
399d85c 
TLSWrap.onerror has a helpful debug() call built in to it. However in
case of a malformed TLSSocket object, where the `_tlsOptions` value is
an unexpected `undefined`, accessing `_tlsOptions.isServer` causes
a TypeError to be thrown.

This commit ensures that the debug() call properly logs the state as
'unknown', instead of the two 'server' and 'client' choices previously
available. Additionally, onerror branching is adjusted to allow such
`undefined` options object, by use of optional chaining.

Other methods are not being adjusted, as such a case of `undefined`
options is not viable during regular processing of the TLSSocket.

Fixes: nodejs#41501
sigv added a commit to sigv/node that referenced this issue Jan 14, 2022
@sigv
tls: avoid throw in onerror for bad TLSSocket obj
653c392 
TLSWrap.onerror has a helpful debug() call built in to it. However in
case of a malformed TLSSocket object, where the `_tlsOptions` value is
an unexpected `undefined`, accessing `_tlsOptions.isServer` causes
a TypeError to be thrown.

This commit ensures that the debug() call properly logs the state as
'unknown', instead of the two 'server' and 'client' choices previously
available. Additionally, onerror branching is adjusted to allow such
`undefined` options object, by use of optional chaining.

Other methods are not being adjusted, as such a case of `undefined`
options is not viable during regular processing of the TLSSocket.

Fixes: nodejs#41501
nodejs-github-bot pushed a commit that referenced this issue Feb 14, 2022
@sigv
tls: avoid throw in onerror for bad TLSSocket obj
a987ba1 
TLSWrap.onerror has a helpful debug() call built in to it. However in
case of a malformed TLSSocket object, where the `_tlsOptions` value is
an unexpected `undefined`, accessing `_tlsOptions.isServer` causes
a TypeError to be thrown.

This commit ensures that the debug() call properly logs the state as
'unknown', instead of the two 'server' and 'client' choices previously
available. Additionally, onerror branching is adjusted to allow such
`undefined` options object, by use of optional chaining.

Other methods are not being adjusted, as such a case of `undefined`
options is not viable during regular processing of the TLSSocket.

Fixes: #41501

PR-URL: #41523
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
bengl pushed a commit to bengl/node that referenced this issue Feb 21, 2022
@sigv @bengl
tls: avoid throw in onerror for bad TLSSocket obj
c498a72 
TLSWrap.onerror has a helpful debug() call built in to it. However in
case of a malformed TLSSocket object, where the `_tlsOptions` value is
an unexpected `undefined`, accessing `_tlsOptions.isServer` causes
a TypeError to be thrown.

This commit ensures that the debug() call properly logs the state as
'unknown', instead of the two 'server' and 'client' choices previously
available. Additionally, onerror branching is adjusted to allow such
`undefined` options object, by use of optional chaining.

Other methods are not being adjusted, as such a case of `undefined`
options is not viable during regular processing of the TLSSocket.

Fixes: nodejs#41501

PR-URL: nodejs#41523
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
bengl pushed a commit to bengl/node that referenced this issue Feb 21, 2022
@sigv @bengl
tls: avoid throw in onerror for bad TLSSocket obj
1cd5723 
TLSWrap.onerror has a helpful debug() call built in to it. However in
case of a malformed TLSSocket object, where the `_tlsOptions` value is
an unexpected `undefined`, accessing `_tlsOptions.isServer` causes
a TypeError to be thrown.

This commit ensures that the debug() call properly logs the state as
'unknown', instead of the two 'server' and 'client' choices previously
available. Additionally, onerror branching is adjusted to allow such
`undefined` options object, by use of optional chaining.

Other methods are not being adjusted, as such a case of `undefined`
options is not viable during regular processing of the TLSSocket.

Fixes: nodejs#41501

PR-URL: nodejs#41523
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
bengl pushed a commit that referenced this issue Feb 22, 2022
@sigv @bengl
tls: avoid throw in onerror for bad TLSSocket obj
b1505a1 
TLSWrap.onerror has a helpful debug() call built in to it. However in
case of a malformed TLSSocket object, where the `_tlsOptions` value is
an unexpected `undefined`, accessing `_tlsOptions.isServer` causes
a TypeError to be thrown.

This commit ensures that the debug() call properly logs the state as
'unknown', instead of the two 'server' and 'client' choices previously
available. Additionally, onerror branching is adjusted to allow such
`undefined` options object, by use of optional chaining.

Other methods are not being adjusted, as such a case of `undefined`
options is not viable during regular processing of the TLSSocket.

Fixes: #41501

PR-URL: #41523
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
danielleadams pushed a commit that referenced this issue Apr 21, 2022
@sigv @danielleadams
tls: avoid throw in onerror for bad TLSSocket obj
51fa723 
TLSWrap.onerror has a helpful debug() call built in to it. However in
case of a malformed TLSSocket object, where the `_tlsOptions` value is
an unexpected `undefined`, accessing `_tlsOptions.isServer` causes
a TypeError to be thrown.

This commit ensures that the debug() call properly logs the state as
'unknown', instead of the two 'server' and 'client' choices previously
available. Additionally, onerror branching is adjusted to allow such
`undefined` options object, by use of optional chaining.

Other methods are not being adjusted, as such a case of `undefined`
options is not viable during regular processing of the TLSSocket.

Fixes: #41501

PR-URL: #41523
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
danielleadams pushed a commit that referenced this issue Apr 24, 2022
@sigv @danielleadams
tls: avoid throw in onerror for bad TLSSocket obj
9649d65 
TLSWrap.onerror has a helpful debug() call built in to it. However in
case of a malformed TLSSocket object, where the `_tlsOptions` value is
an unexpected `undefined`, accessing `_tlsOptions.isServer` causes
a TypeError to be thrown.

This commit ensures that the debug() call properly logs the state as
'unknown', instead of the two 'server' and 'client' choices previously
available. Additionally, onerror branching is adjusted to allow such
`undefined` options object, by use of optional chaining.

Other methods are not being adjusted, as such a case of `undefined`
options is not viable during regular processing of the TLSSocket.

Fixes: #41501

PR-URL: #41523
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
@fragglebob fragglebob mentioned this issue Jun 13, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
tls Issues and PRs related to the tls subsystem.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

tls: avoid throw in onerror for bad TLSSocket obj sigv/node
2 participants
@sigv @Mesteery