-
Notifications
You must be signed in to change notification settings - Fork 7
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
Propagate the authority pseudoheader #33
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -52,6 +52,10 @@ package final class GRPCChannel: ClientTransport { | |
/// A factory for connections. | ||
private let connector: any HTTP2Connector | ||
|
||
/// The server authority. If `nil`, a value will be computed based on the endpoint being | ||
/// connected to. | ||
private let authority: String? | ||
|
||
/// The connection backoff configuration used by the subchannel when establishing a connection. | ||
private let backoff: ConnectionBackoff | ||
|
||
|
@@ -82,6 +86,12 @@ package final class GRPCChannel: ClientTransport { | |
self.input = AsyncStream.makeStream() | ||
self.connector = connector | ||
|
||
if let authority = config.http2.authority ?? resolver.authority { | ||
self.authority = PercentEncoding.encodeAuthority(authority) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why does the percent-encoding happen at this level of the hierarchy rather than when it is first obtained? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If it remains sometimes percent-encoded and sometimes not when stored as a property on objects I think that should be documented. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is effectively where it's first obtained: If we don't have a value at this point then we have to derive it from the address of each connection the You're right about the documentation though, I'll add that and some comments explaining this. |
||
} else { | ||
self.authority = nil | ||
} | ||
|
||
self.backoff = ConnectionBackoff( | ||
initial: config.backoff.initial, | ||
max: config.backoff.max, | ||
|
@@ -446,6 +456,7 @@ extension GRPCChannel { | |
state.changeLoadBalancerKind(to: loadBalancerConfig) { | ||
let loadBalancer = RoundRobinLoadBalancer( | ||
connector: self.connector, | ||
authority: self.authority, | ||
backoff: self.backoff, | ||
defaultCompression: self.defaultCompression, | ||
enabledCompression: self.enabledCompression | ||
|
@@ -463,6 +474,7 @@ extension GRPCChannel { | |
state.changeLoadBalancerKind(to: loadBalancerConfig) { | ||
let loadBalancer = PickFirstLoadBalancer( | ||
connector: self.connector, | ||
authority: self.authority, | ||
backoff: self.backoff, | ||
defaultCompression: self.defaultCompression, | ||
enabledCompression: self.enabledCompression | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here you use
self.authority ?? self.address.sniHostname
, but further down the file you useself.authority ?? self.address.authority
- is that intentional?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it's intentional here. The authority here is used as part of the TLS handshake, but valid hostnames for that don't include raw IP addresses, so
sniHostname
returnsnil
ifaddress
is an IPv4/IPv6 address.Further down the authority being used is for the
:authority
pseudo-header.I'll add some comments explaining this.