This repository has been archived by the owner. It is now read-only.

Remove splitting of path and path base in Kestrel #1050

Closed
JunTaoLuo opened this Issue Aug 17, 2016 · 11 comments

Comments

Projects
None yet
7 participants
@JunTaoLuo
Copy link
Contributor

JunTaoLuo commented Aug 17, 2016

This is now implemented as the UsePathBase Middleware in HttpAbstractions. We can now remove this feature in Kestrel.

@davidfowl

This comment has been minimized.

Copy link
Member

davidfowl commented Aug 17, 2016

👏

@brockallen

This comment has been minimized.

Copy link

brockallen commented Mar 17, 2017

Why add UsePathBase and not just use Map?

@Tratcher

This comment has been minimized.

Copy link
Member

Tratcher commented Mar 17, 2017

Map branches, UsePathBase trims (if it matches) and continues.

@brockallen

This comment has been minimized.

Copy link

brockallen commented Mar 17, 2017

So UsePathBase is the same as Map, if you had just taken everything in the Configure and moved it into the config inside the Map?

@Tratcher

This comment has been minimized.

Copy link
Member

Tratcher commented Mar 17, 2017

The path logic is the same, but for Map the other branch would always be empty and 404.

For the urls http://localhost/favicon.ico and http://localhost/PathBase/favicon.ico, if you use UsePathBase then the two both result in the same /favicon.ico for the next middleware, but if you Map then you get two pipelines that may have different middleware. If you only configure the Map pipeline, then the non-Map pipeline always 404s.

@brockallen

This comment has been minimized.

Copy link

brockallen commented Mar 17, 2017

I guess all I was getting at was that it seemed odd to have a new API when you can get the same effect with an existing API.

For the urls http://localhost/favicon.ico and http://localhost/PathBase/favicon.ico, if you use UsePathBase then the two both result in the same /favicon.ico for the next middleware

Wait, so when you said "strip" this is what you mean I guess... meaning if the URL starts with the ~/BaseUrl then it's stripped and then proceeds to the next middleware (so the next MW doesn't see toe original)... Is that what UseUrls used to do? Or am I still confused? Perhaps I guess I need to get the nightlys and test.

@Tratcher

This comment has been minimized.

Copy link
Member

Tratcher commented Mar 17, 2017

Yes, that's the same as what UseUrls used to do (in Kestrel).

@brockallen

This comment has been minimized.

Copy link

brockallen commented Mar 18, 2017

Ok, I setup a quick test sample on v1.1. In short UseUrls("http://localhost:123/foo") still accepts requests to "http://localhost:123/bar", and that's my confusion. This behavior (that I was previously unaware of) is pretty counter intuitive. Your explanation above now makes more sense, but I am left feeling like I'm missing some major context as to why the counter intuitive behavior existed in the first place. I guess it's a "kestrel thing" (or maybe libuv?), as opposed to how I'd expect http.sys reserved URLs to behave?

@Tratcher

This comment has been minimized.

Copy link
Member

Tratcher commented Mar 18, 2017

Yes, it was a Kestrel thing. Http.Sys routes requests using scheme, host, port, and path base, but Kestrel really only operates on ip and port. Once it has received a request it doesn't reject it if the host or path base are wrong.

@grantbowering

This comment has been minimized.

Copy link

grantbowering commented May 18, 2018

In 1.1, a path base could be set by appending it to an ASPNETCORE_URLS environment variable appropriately, which provided a quick way of differing this per deployment. Since that throws an exception in 2.0, is there an environment variable that can be leveraged equivalent to app.SetPathBase(path), or writing code to set this in Configure the only way to set this up?

@Tratcher

This comment has been minimized.

Copy link
Member

Tratcher commented May 18, 2018

@grantbowering this is only supported via code at the moment. Future improvements are tracked at https://github.com/aspnet/Hosting/issues/1120

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