Refactor deconstructURL and scheme parsing

[dnadoba]

Motivation

We want to reuse our URL parsing, validation and deconstruction logic for the upcoming new HTTPClientRequest from the async/await proposal.

Changes

• Introduce a new DeconstructedURL struct which hold a scheme, connectionTarget and uri.
• Introduce a new DeconstructedURL.Scheme enum which replaced the previous String in HTTPClient.Request and ConnectionPool.Scheme enum
• move deconstructURL logic to an initialiser of DeconstructedURL
• use DeconstructedURL instead of separate scheme, connectionTarget, and uri properties in HTTPClient.Request
• remove HTTPClient.Request.Kind because it is almost a replica of the DeconstructedURL.Scheme enum where the only difference is that http and https are represented as one case called host. As we now represent scheme as an enum instead of a String we can safely replace it with an exhaustive switch over scheme instead.
• refactor tests to use scheme instead of kind
• remove unused config(overriding:) from ConnectionPool.Key

[dnadoba]

@swift-server-bot test this please

[fabianfett]

All in all looks good. Few changes wanted.

[fabianfett]

NIT: Only import what is actually needed.

Suggested change

	import Foundation
	import struct Foundation.URL

[fabianfett]

He, she, it das s muss mit ;)

[dnadoba]

We will diverge from the public API on HTTPClient.Request but I’m okay with that

[fabianfett]

I'm not sure this should be namespaced in DeconstructedURL. Would you mind moving this out?

[fabianfett]

I think this should live in the Scheme file.

[fabianfett]

NIT: Move down to other internal var like redirect state. Mixing internal and public makes this hard to read.

[fabianfett]

Very cool, except for the tiny nits.

[fabianfett]

@swift-server-bot test this please

[karwa]

One thing that might be worth considering is if the scheme can just be dropped?

• http == httpUnix == unix, the only difference is the connection target (host/UDS)
• https == httpsUnix, the only difference is the connection target (host/UDS)

Because it now knows which kind of connection target it has, the only thing the scheme provides is whether to use TLS.

[dnadoba]

Sadly we expose the scheme as public API. Not as the Scheme enum but as a String on HTTPClient.Request and therefore need to preserve it for the time being.

[karwa]

Technically, it could get that from the URL. There's no need for ConnectionPool or most other things to know specifically which kind of scheme was used (also, http+unix and unix schemes should resolve to the same connection in the pool if they point to the same socket).

[Lukasa]

(also, http+unix and unix schemes should resolve to the same connection in the pool if they point to the same socket).

I disagree with this: the scheme is meaningfully part of both the origin and the network key for a request, and so should use different connections.

[fabianfett]

The problem here are http semantics. From an http standpoint unix:///some/path and http+unix:///some/path go to different places. While the server might be the same, the pure semantics say they are different (because of the different scheme). And since connections are trust boundaries, we need to make the differentiation based on the scheme.

[karwa]

I'm not sure to what extent HTTP really intends to define the semantics of non-http:/https: URLs. It sounds more likely that the semantics of application-defined schemes are also application-defined.

[Lukasa]

HTTP does not define them. I agree that in principle we can define them however we want. Given that schemes are explicitly part of the connection pooling pattern in fetch, the most prudent choice is to generalise that idea to these schemes, especially as there are very few practical problems caused by doing so.