Fix URL generation

[jessarcher]

There are several reports of issues with the URL returned by Inertia as a result of #333.

333 was created to support the X_FORWARDED_PREFIX header value which can be added by reverse proxies that serve a web request under a sub-path that isn't normally visible to the underlying webserver.

Unfortunately, it seems to have created a scenario where a sub-path can be doubled up. I haven't been able to figure out exactly what scenario triggers this. There are several proposed PRs to fix it, but nothing I can reproduce. In any case, I'm assuming both $request->baseUrl() and $request->getRequestUri() can contain the same sub path, which we concatenate.

Some solutions propose to use $request->fullUrl(), which constructs a full URL from the various pieces (without concatenating those specific methods). The problem is that this method includes the HTTP scheme, host, and port, and there may be users that depend on Inertia's URL not having those elements (e.g. styling active links). Other solutions combine various other methods to construct the URL, but they need to get into the weeds of adding query strings, etc.

This PR proposes to let Laravel/Symfony handle the URL construction intricacies. Unfortunately, there doesn't appear to be a method that gives us exactly what we need without the scheme and host, so I've just stripped those from the resulting URL.

Alternatives:

• 360 (This one manually constructs the URL but misses some logic from fullUrl(). It has a test that replicates the new issue, but not the potential regression.)

• 372 (This just restored the old behaviour, which had the proxy issue)

• 566 (this is very similar to mine, and given the existing tests pass, I'm happy to accept it)

• 576

Closes #359 Closes #421

[rojtjo]

Glad to see this is being addressed!

I'm just wondering why we can't just make the URL generation configurable with a sensible default (probably the implementation of this PR)? Currently we have to either override both the ResponseFactory and Response classes, or create a custom middleware to do this.

[jessarcher]

@rojtjo I'm happy to look at making it configurable. May I ask your use case for configuring it differently from how this PR does it?

[rojtjo]

The only project I ran into the double path issue was when I mounted the app on a subpath using an nginx location block, but that has since been migrated to a subdomain. So unfortunately I cannot confirm if this PR would've actually fixed the issue.

The main issue in my opinion is that there is currently no easy way to override the behavior on app level. At the time I created #503 which solved the issue for me using closures, but it would've probably been easier if we had the concept of a UrlResolver contract so we could just swap the implementation through the container.

Anyways, if this PR does actually fix the issue for everyone there is no need to go through all these hoops to retrieve the correct URL. So I guess it's best to just hold off of making it configurable for now.

[murilomagalhaes]

I can confirm that this PR fixes the issue. I'm no longer having duplicated URIs. But now i'm getting the requested URI with ASCII scaped forward slashes (%2F) inside a query string on any route.

My nginx conf:

[RobertBoes]

Can confirm the behaviour here is changed. Visiting /?value=te/st in current Inertia works, no redirect happens, the History is updated correctly. But with this change visiting above URL would perform the visit normally, however the GET parameters are now escaped, resulting in the history pushstate updating the URL to /?value=te%2Fst. And I'm testing this on Valet, so no special config or subdirectories. Don't think this is desired behaviour?

[jessarcher]

So the \Illuminate\Http\Request::fullUrl method uses the Symfony\Component\HttpFoundation::getQueryString method, which in turn passes the query string through Symfony\Component\HttpFoundation::normalizeQueryString.

The Symfony\Component\HttpFoundation::getRequestUri method we previously used returns the path and query string "raw". However, it doesn't include the X_FORWARDED_PREFIX, which is why it was prefixed with Symfony\Component\HttpFoundation::getBaseUrl, but that resulted in URL double-ups.

This feels like a game of whack-a-mole.

The only thing I can think of is to manually construct the URL, similar to how fullUrl does, but without the scheme and host and without query string encoding.

[driesvints]

People, just want to note that Laravel does not and will never support projects in sub directories and this should be avoided at all cost: https://laravel.com/docs/10.x/installation#directory-configuration

[RobertBoes]

People, just want to note that Laravel does not and will never support projects in sub directories and this should be avoided at all cost: laravel.com/docs/10.x/installation#directory-configuration

That's just talking about not using a document root on a Laravel app. For example you have a example.com pointed to /home/example and then your Laravel app located at /home/example/laravel-app, which would result in a URL like example.com/laravel-app, that's unsupported yes. But setting a document-root to /home/example/laravel-app or using proxy with the x-forwarded-prefix should work without issues. So with that you could host your app at example.com/laravel-app without issues.

[driesvints]

@RobertBoes nope, sorry not supported.

[RobertBoes]

@driesvints That's not what the text says tho? Along with https://github.com/laravel/framework/blob/10.x/src/Illuminate/Http/Middleware/TrustProxies.php#L22 it properly sets the prefix path.

[driesvints]

I'm sorry @RobertBoes but Laravel has never intended to host an app other than at the document root and root url. The routing system just isn't intended to be used with a prefix.

[RobertBoes]

So you're saying Request::HEADER_X_FORWARDED_PREFIX does in fact not work in TrustProxies? Why is it an option in the middleware then?

[driesvints]

@RobertBoes look, all I'm saying is that if you expect laravel-app of example.com/laravel-app to be the root of your Laravel app that that is not something we support. Url or server wise.

[rojtjo]

Supported by Laravel, or not, this PR fixed the double path issue. Only issue is that it introduced a bug where the query string is being encoded.