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

Add support for specifying the PathBase for all requests #5898

Open
davidfowl opened this issue Jun 29, 2017 · 25 comments
Open

Add support for specifying the PathBase for all requests #5898

davidfowl opened this issue Jun 29, 2017 · 25 comments
Labels
affected-few This issue impacts only small number of customers area-hosting Includes Hosting area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions enhancement This issue represents an ask for new feature or an enhancement to an existing one severity-minor This label is used by an internal tool
Milestone

Comments

@davidfowl
Copy link
Member

As a follow up from aspnet/Hosting#815, we want to allow specifying the path base as a host setting. Hosting should use this to call UsePathBase as the first middleware in the pipeline. IISIntegration does something similar with the APPL_PATH env variable : https://github.com/aspnet/IISIntegration/blob/51554566532237ef76cb105ec5b3f7a5050b9ba4/src/Microsoft.AspNetCore.Server.IISIntegration/WebHostBuilderIISExtensions.cs#L19.

@bklooste
Copy link

You want to be able to switch between WebListener ( or any host) and Kestrel which share the same startup before startup is loaded.. Having If (KestralLoaded) in startup is pretty damn ugly .. and makes testing a pain , i dont think there should be any Host ( including Kestrel) specific stuff in Startup .

@bklooste
Copy link

Using IStartupFilter is pretty ugly and having 2 methods for the 2 web hosts is pretty poor . As I mentioned before other systems may do a lot of work before startup eg console hosts , hosted by SF etc etc . Core and web listener supports this nicely but Kestrel does not. eg Hosting is a ASP.Net concept in startup I think the port/url hosting needs to be done before Startup .. Sure Startup should have access to this but it should be set before.

@davidfowl
Copy link
Member Author

I'm not sure what you're referring to but there will be a use path base on the IWebHostBuilder like the method suggests. I don't know what more you're asking for.

@jkotalik
Copy link
Contributor

@davidfowl For testing this change, I believe I need to send a request to the application to verify that the HttpContext properly contains the PathBase.

Firstly, is there a better way to test this?

If this is correct, what type of test would this be? It seems to extend past the WebHosts tests because it requires that a request be sent to a server.

@davidfowl
Copy link
Member Author

@jkotalik Whats the issue with sending a request?

@Tratcher
Copy link
Member

I assume you'd just be calling UsePathBase. https://github.com/aspnet/HttpAbstractions/blob/87cd79d6fc54bb4abf07c1e380cd7a9498a78612/src/Microsoft.AspNetCore.Http.Abstractions/Extensions/UsePathBaseExtensions.cs#L21
You can mimic existing hosting and UsePathBase tests.

@ygoe
Copy link

ygoe commented Feb 3, 2018

Maybe it's a good idea to also make the use of proxy forwarding HTTP headers configurable for the hosting environment. Because these headers are provided by the hosting environment and could be different per host. The application shouldn't need to care about what headers the proxy provides.

My applications currently draw the application path base from an environment variable provided by my own hosting environment and put it into UsePathBase, and they also call UseForwardedHeaders because the hosting environment works that way.

But I can't influence third-party applications. They might stop working in some hosting environments.

@poke
Copy link
Contributor

poke commented Mar 24, 2018

I just stumbled on this issue again; what is the actual goal of this? To move the configuration of the path base into the WebHostBuilder, to ensure that it is always done at the very beginning of the middleware pipeline? And so that people can configure it through the host builder configuration (e.g. using environment variables, command line arguments, etc.?)

@davidfowl
Copy link
Member Author

@poke yes that was the intent

@poke
Copy link
Contributor

poke commented Mar 24, 2018

I see, I might have the chance to work on this then, seeing that this is not planned by someone else at this point.

Does the solution include any support for allowing to dynamically set the path base when sitting behaind a reverse proxy? In one of my applications, I currently use a custom X-Forwarded-Path HTTP header to dynamically configure the path base (along with the other X-Forwarded headers) to completely uncouple the configuration from the application, moving it into the reverse proxy configuration.

I don’t assume that we would want to move such a functionality into the webhost but keep it as custom middleware then, right (Just like the UseForwardedHeaders middleware)? In that case, we would have to make sure that the stuff in the webhost will not conflict here and that you could still overwrite the path base properly in user middleware.

@Tratcher
Copy link
Member

X-Forwarded-Path would be an interesting add to UseForwardedHeaders, though it appears to be far less common.

The ask for Hosting is only for a static config based mapping for basic reverse proxy scenarios.

@Tratcher
Copy link
Member

That said, I've seen some variants of this request in forums that UsePathBase does not cover. If the proxy trims or prepends the path then UsePathBase doesn't work, it relies on an unchanged path.

Examples:

A basic pass through:
Public Path: /foo/api/1
Proxy forwards: /foo/api/1
Fixup needed: UsePathBase(/foo), Path becomes: /api/1 and PathBase becomes: /foo

Variant 1: Proxy trims the path
Public Path: /foo/api/1
Proxy forwards /api/1
Fixup needed: Set PathBase to /foo, leave Path alone

Variant 2: Proxy prepends path
Public Path: /api/1
Proxy forwards /foo/api/1
Fixup needed: Remove /foo from Path, don't set PathBase

UsePathBase might need some overloads to handle these: Transfer, Set, Trim. I don't know how easily we could get those all into hosting config unless they were all separate parameters.

@damianh
Copy link

damianh commented Sep 13, 2018

Just got burned by this, specifically proxy trimming the path. Sorry I have nothing more valuable to add other than feedback from the wild.

@aspnet-hello aspnet-hello transferred this issue from aspnet/Hosting Dec 18, 2018
@aspnet-hello aspnet-hello added this to the Backlog milestone Dec 18, 2018
@aspnet-hello aspnet-hello added area-hosting enhancement This issue represents an ask for new feature or an enhancement to an existing one labels Dec 18, 2018
@shanselman
Copy link
Contributor

I'm pretty sure this is the inverse of PathBase. I'm having to add it back in order to get ~ to work.

I'm taking http://staging.hanselman.com/blog and pointing it to http://hanselmanblog.azurewebsites.net so I need to add it BACK not strip it. Per https://docs.microsoft.com/en-us/aspnet/core/host-and-deploy/proxy-load-balancer?view=aspnetcore-2.2#deal-with-path-base-and-proxies-that-change-the-request-path

app.Use((context, next) =>
{
     context.Request.PathBase = new PathString(/blog);
     return next.Invoke();
});

@robertlestak
Copy link

I don't have a solution for the underlying AspNetCore issue, however I found a viable server-side workaround until a DotNet Core solution is found.

If a unique proxy_pass location block is created in the proxy server for every single DotNet Route, DotNet will be able to respond as expected.

This of course requires close coordination between server admins and app devs, or a script which dynamically modifies location blocks with each iterative update of the backing DotNet application.

While certainly is not a viable long-term solution, it does ensure that in the interim, while this bug is being addressed, DotNet APIs will "play nice" behind path-rewriting proxies.

@Andreas-Hjortland
Copy link

Andreas-Hjortland commented May 25, 2020

I encountered this issue today, and I solved it by adding a custom middleware which looks at the X-Path-Base header that I configured my reverse proxy to set, which sets the PathBase and Path based on the value in that header (shamelessly copied the implementation from UsePathBaseMiddleware.cs). Would it be possible to extend the ForwardedHeadersMiddleware and its corresponding Options class with something like this? I can create a pull request if this looks like a sound solution to the problem.

app.Use(async (context, next) =>
{
    // TODO Check if host is valid based on hosts in ForwardedHeadersOptions
    if (context.Request.Headers.TryGetValue("X-Path-Base", out var pathBase) &&
        context.Request.Path.StartsWithSegments(pathBase.Last(), out var matchedPath, out var remainingPath))
    {
        try
        {
            var originalPath = context.Request.Path;
            var originalPathBase = context.Request.PathBase;
            context.Request.PathBase = matchedPath;
            context.Request.Path = remainingPath;
            await next();
        }
        finally
        {
            context.Request.PathBase = new PathString(matchedPath);
            context.Request.Path = new PathString(remainingPath);
        }
    }
    else
    {
        await next();
    }
});

@Tratcher Tratcher added affected-few This issue impacts only small number of customers severity-minor This label is used by an internal tool labels Nov 6, 2020 — with ASP.NET Core Issue Ranking
@ericis
Copy link

ericis commented Jul 30, 2021

Just spent a few hours toward discovering this issue.

I'm using Kubernetes + Traefik reverse proxy, which strips the prefix path for two dockerized .NET sample apps. Of course, the static assets are attempting to use absolute paths at the root path, but when those URLs bounce through the proxy, they're not recognized.

e.g. "/app-1" on traefik strips the prefix to "/" for .NET and the sample app loves it, so it responds with a 200, but pukes out absolute URLs to the root "/", because it doesn't understand that it's running behind a proxy.

Naturally, I thought... "this is a ubiquitous problem that has been solved using environment variables!" ...

I "could" start coding, but why should we have to code this? I want to demo the awesome work of .NET Core teams out-of-the-box! 😉

Is there support yet for something like ASPNETCORE_APPL_PATH that I can pass to the docker container or for a header that I can inject using Traefik?

p.s. (rant) no solution for reverse-proxy path rewrites since 2017?! Posted by @davidfowl and high-fived by @shanselman in 2019 ?? I'm so confused why this isn't solved with an environment variable, configuration and or command line argument...

@ericis
Copy link

ericis commented Jul 30, 2021

I'd love to see this:
docker run --name aspnet-sample -d -p 8080:80 -e ASPNETCORE_APPL_PATH=/dotnet mcr.microsoft.com/dotnet/sample s:aspnetapp

... result in:

@nsmithdev
Copy link

https://docs.microsoft.com/en-us/aspnet/core/host-and-deploy/proxy-load-balancer?view=aspnetcore-3.1#deal-with-path-base-and-proxies-that-change-the-request-path

var appPath = Environment.GetEnvironmentVariable("ASPNETCORE_APPL_PATH");
if (!string.IsNullOrEmpty(appPath))
{
    app.Use((context, next) =>
    {
        context.Request.PathBase = new PathString(appPath);
        return next();
    });
}

@cslovell
Copy link

cslovell commented Feb 4, 2022

I'm genuinely surprised this isn't implemented. This is standard 12 factor app stuff and for those of us in decoupled settings, where software built by one team is deployed internally in several locations, we need to be able to configure this from the environment in a self-service manner without rebuilding the application or asking another team to rewrite their software to look for the environment variable. In other words, if the app is already pulling environment variables in, there should be a standard environment variable that we can use to set the base path. For instance:

A .NET application built by Team One is set to receive traffic at the root path, example.com. However, Team Two will deploy this application at example.com/team1/app, so they set up a proxy that forwards traffic and set the environment variable that instructs the application to preface all requests with this base path. Team Two does not need to bother Team One about ensuring that the code to look for environment variables to set Base Path is set in all their applications, since this would be a standard part of the .NET library.

@cslovell
Copy link

cslovell commented Feb 4, 2022

https://docs.microsoft.com/en-us/aspnet/core/host-and-deploy/proxy-load-balancer?view=aspnetcore-3.1#deal-with-path-base-and-proxies-that-change-the-request-path

var appPath = Environment.GetEnvironmentVariable("ASPNETCORE_APPL_PATH");
if (!string.IsNullOrEmpty(appPath))
{
    app.Use((context, next) =>
    {
        context.Request.PathBase = new PathString(appPath);
        return next();
    });
}

Yes - but this will need to be done for every application, despite being a standard component of 12 factor app development. The nature of organizational bureaucracy may imply that this change is not possible by the team deploying the software. Setting this through an environment variable that .NET recognizes by default is much more appropriate.

@adityamandaleeka adityamandaleeka removed this from the Backlog milestone Feb 23, 2022
@adityamandaleeka
Copy link
Member

Cleared the milestone so this shows up in our next triage discussion.

@adityamandaleeka
Copy link
Member

Related: #23263 (doing that would give people another way to set the path base)

@adityamandaleeka adityamandaleeka added this to the .NET 7 Planning milestone Feb 23, 2022
@ghost
Copy link

ghost commented Feb 23, 2022

Thanks for contacting us.

We're moving this issue to the .NET 7 Planning milestone for future evaluation / consideration. We would like to keep this around to collect more feedback, which can help us with prioritizing this work. We will re-evaluate this issue, during our next planning meeting(s).
If we later determine, that the issue has no community involvement, or it's very rare and low-impact issue, we will close it - so that the team can focus on more important and high impact issues.
To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

@ghost
Copy link

ghost commented Jul 27, 2022

We've moved this issue to the Backlog milestone. This means that it is not going to be worked on for the coming release. We will reassess the backlog following the current release and consider this item at that time. To learn more about our issue management process and to have better expectation regarding different types of issues you can read our Triage Process.

@amcasey amcasey added area-hosting Includes Hosting and removed feature-hosting labels Jun 1, 2023
@amcasey amcasey added area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions and removed area-runtime labels Aug 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affected-few This issue impacts only small number of customers area-hosting Includes Hosting area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions enhancement This issue represents an ask for new feature or an enhancement to an existing one severity-minor This label is used by an internal tool
Projects
None yet
Development

No branches or pull requests