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

Internal error in http2 with checkServerIdentity undefined #49839

Closed
martenrichter opened this issue Sep 24, 2023 · 3 comments · Fixed by #49896
Closed

Internal error in http2 with checkServerIdentity undefined #49839

martenrichter opened this issue Sep 24, 2023 · 3 comments · Fixed by #49896
Labels
http2 Issues or PRs related to the http2 subsystem.

Comments

@martenrichter
Copy link
Contributor

Version

v18.18.0

Platform

Linux 3eaa088c9827 5.15.90.1-microsoft-standard-WSL2 #1 SMP Fri Jan 27 02:56:13 UTC 2023 x86_64 GNU/Linux

Subsystem

http2

What steps will reproduce the bug?

It happened with:

  this.clientInt = connect('https://' + +this.hostname + ':' + this.port, {
      settings: {
        enableConnectProtocol: true
      },
      checkServerIdentity,
      localPort: this.localPort
    })

Important is that checkServerIdentity is undefined.

It says then:

Error [ERR_INTERNAL_ASSERTION]: This is caused by either a bug in Node.js or incorrect usage of Node.js internals.
Please open an issue with this stack trace at https://github.com/nodejs/node/issues

    at new NodeError (node:internal/errors:405:5)
    at assert (node:internal/assert:14:11)
    at Object.connect (node:_tls_wrap:1684:3)
    at connect (node:internal/http2/core:3298:22)
    at Http2WebTransportClient.createTransport (file:///workspaces/webtransport/lib/http2/client.js:57:22)
    at HttpClient.transportIntSwitchToReliable (file:///workspaces/webtransport/lib/transport.js:129:35)
    at file:///workspaces/webtransport/lib/webtransport.js:70:18 {
  code: 'ERR_INTERNAL_ASSERTION'
}

and I am just filling it, since node.js asked me to.
I assume it can be easily worked around by not having checkServerIdentity in the object, when not used.

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

Always.

What is the expected behavior? Why is that the expected behavior?

Ignore the setting

What do you see instead?

The trace above.

Additional information

No response

@ishan-18
Copy link

Try doing this:
`const options = {
settings: {
enableConnectProtocol: true
},
localPort: this.localPort
};

if (checkServerIdentity) {
options.checkServerIdentity = checkServerIdentity;
}

this.clientInt = connect('https://' + this.hostname + ':' + this.port, options);
`

@martenrichter
Copy link
Contributor Author

martenrichter commented Sep 27, 2023

Thanks, of course, I did something like this. But as I wrote, I just filed the issue, as node.js asked me too. ("Please open an issue with this stack trace at") I assume, that the intention is, that it is either correctly handled or an meaningful error is thrown.

deokjinkim added a commit to deokjinkim/node that referenced this issue Sep 27, 2023
If user uses invalid type for `options.checkServerIdentity`
in tls.connect(), it's not internal issue of Node.js. So
validateFunction() is more proper than assert().

Fixes: nodejs#49839
@deokjinkim
Copy link
Contributor

@martenrichter Thank you for your report. As you mentioned, need to throw ERR_INVALID_ARG_TYPE instead of ERR_INTERNAL_ASSERTION for this case. I created PR(#49896).

@mertcanaltin mertcanaltin added the http2 Issues or PRs related to the http2 subsystem. label Sep 30, 2023
nodejs-github-bot pushed a commit that referenced this issue Sep 30, 2023
If user uses invalid type for `options.checkServerIdentity`
in tls.connect(), it's not internal issue of Node.js. So
validateFunction() is more proper than assert().

Fixes: #49839
PR-URL: #49896
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
GeoffreyBooth pushed a commit to GeoffreyBooth/node that referenced this issue Oct 1, 2023
If user uses invalid type for `options.checkServerIdentity`
in tls.connect(), it's not internal issue of Node.js. So
validateFunction() is more proper than assert().

Fixes: nodejs#49839
PR-URL: nodejs#49896
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
alexfernandez pushed a commit to alexfernandez/node that referenced this issue Nov 1, 2023
If user uses invalid type for `options.checkServerIdentity`
in tls.connect(), it's not internal issue of Node.js. So
validateFunction() is more proper than assert().

Fixes: nodejs#49839
PR-URL: nodejs#49896
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
debadree25 pushed a commit to debadree25/node that referenced this issue Apr 15, 2024
If user uses invalid type for `options.checkServerIdentity`
in tls.connect(), it's not internal issue of Node.js. So
validateFunction() is more proper than assert().

Fixes: nodejs#49839
PR-URL: nodejs#49896
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
http2 Issues or PRs related to the http2 subsystem.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants