aspnet / HttpAbstractions

[Archived] HTTP abstractions such as HttpRequest, HttpResponse, and HttpContext, as well as common web utilities. Project moved to https://github.com/aspnet/AspNetCore
Apache License 2.0
382 stars 193 forks source link

Copy ServerAddress from Kestrel to HttpAbstractions #977

Closed jkotalik closed 6 years ago

jkotalik commented 6 years ago

BasicMiddleware needs to parse server address, however we don't want to have BasicMiddleware depend on Kestrel. We also can't delete the ServerAddress from Kestrel because it is public https://github.com/aspnet/KestrelHttpServer/blob/dev/src/Kestrel.Core/ServerAddress.cs. We will first create this new class in HttpAbstractions, have Kestrel wrap ServerAddressTemplate with ServerAddress, and then modify HttpSysServer to use this type.

halter73 commented 6 years ago

Why do we want ServerAddress here instead of a shared source package in the Common repo?

Tratcher commented 6 years ago

An API for this is better than shared source here because it applies generally to anyone consuming the IServerAddressesFeature.

jkotalik commented 6 years ago

Also ServerAddress is a public type. We can't change the namespace on it (moving it to a shared package) without breaking people.

jkotalik commented 6 years ago

@Tratcher @halter73 and I discussed this and he originally wanted ServerAddress to be an internal type rather than public to begin with. I'm am okay with either option, depending on how reusable we want to make this.

Tratcher commented 6 years ago

The travis failures look like issues that need to be addressed.

Tratcher commented 6 years ago

Offline agreement is to call it BindingAddress and leave it pubternal.

jkotalik commented 6 years ago

🆙 📅

jkotalik commented 6 years ago

@halter73 if we change kestrel to consume this type, it will have to throw if the PathBase is set. I'll make that change once this is in.