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

Allow DNS override #675

Merged
merged 5 commits into from
Mar 30, 2023
Merged

Allow DNS override #675

merged 5 commits into from
Mar 30, 2023

Conversation

dnadoba
Copy link
Collaborator

@dnadoba dnadoba commented Mar 29, 2023

Sometimes it can be useful to connect to one host e.g. x.example.com but request and validate the certificate chain as if we would connect to y.example.com. This is what this PR adds support for by adding a dnsOverride configuration to HTTPClient.Configuration. This is similar to curls —resolve option but only allows overriding host and not ports for now.

Sometimes it can be useful to connect to one host e.g. `x.example.com` but request and validate the certificate chain as if we would connect to `y.example.com`. This is what this PR adds support for by adding a `dnsOverride` configuration to `HTTPClient.Configuration`. This is similar to curls `—resolve-to` option but only allows overriding host and not ports for now.
@dnadoba dnadoba added the semver/minor For PRs that when merged cause a bump of the minor version, ie. 1.x.0 -> 1.(x+1).0 label Mar 29, 2023
Copy link
Collaborator

@Lukasa Lukasa left a comment

Choose a reason for hiding this comment

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

Mostly looks really good! One small note.

@@ -434,7 +434,7 @@ extension HTTPConnectionPool.ConnectionFactory {
}
#endif

let sslServerHostname = self.key.connectionTarget.sslServerHostname
let sslServerHostname = self.key.serverNameIndicatorOverride ?? self.key.connectionTarget.sslServerHostname
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should we hide this in a computed property on the key so as to centralise the knowledge of how to do this?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done. Actually found through this another place where I forgot to use the override, namely if we proxy:
f296aa4

@Lukasa Lukasa merged commit 98b45ed into swift-server:main Mar 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semver/minor For PRs that when merged cause a bump of the minor version, ie. 1.x.0 -> 1.(x+1).0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants