Skip to content

grpc-js: Add security connector, rework connection establishment#2855

Merged
murgatroid99 merged 2 commits intogrpc:masterfrom
murgatroid99:grpc-js_credentials_secure_connector
Nov 23, 2024
Merged

grpc-js: Add security connector, rework connection establishment#2855
murgatroid99 merged 2 commits intogrpc:masterfrom
murgatroid99:grpc-js_credentials_secure_connector

Conversation

@murgatroid99
Copy link
Member

This modifies the ChannelCredentials API so that instead of providing the connection options which are used to establish a TLS connection, it provides a SecurityConnector, which is created using information about a specific subchannel, and is responsible for establishing TLS connections on top of TCP connections.

With this change, the steps of establishing an HTTP/2 session are more cleanly divided:

  • The transport establishes the TCP connection, potentially delegating to the proxy connection code.
  • The security connector establishes the TLS connection on top of the TCP connection (or passes it through untouched, in the case of insecure credentials)
  • The transport creates the HTTP/2 session on top of the socket provided by the security connector.

@murgatroid99 murgatroid99 merged commit 8f08bbe into grpc:master Nov 23, 2024
* disable enforcement of the limit. Some testing indicates that Node's
* behavior degrades badly when this limit is reached, so we solve that
* by disabling the check entirely. */
connectionOptions.maxSessionMemory = Number.MAX_SAFE_INTEGER;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@murgatroid99 why was this removed?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should still be there. It looks like it got lost in the shuffle, along with the maxSendHeaderBlockLength setting on line 688.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@murgatroid99 do you plan to restore it?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is now restored in version 1.14.1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants