inertiajs / inertia-laravel

The Laravel adapter for Inertia.js.
https://inertiajs.com
MIT License
2.01k stars 224 forks source link

Page URL: Fix handling of base url and reverse proxy forward prefix #566

Closed flexponsive closed 5 months ago

flexponsive commented 8 months ago

The url property of of the inertia HTTP response should contain the user-facing request URI, i.e. the absolute URL path and if applicable the query string.

Inertia currently has a bug where, if the laravel application has base url different from root, the url property contains the baseUrl twice (https://github.com/inertiajs/inertia-laravel/pull/372, https://github.com/inertiajs/inertia-laravel/issues/359, https://github.com/inertiajs/inertia-laravel/issues/421). This was introduced as a side-effect of a fix for a separate issue when the laravel application is deployed behind a reverse proxy, and where the reverse proxy has a forward prefix (https://github.com/inertiajs/inertia-laravel/pull/333).

This PR fixes the handling of the baseUrl case and preserves the behavior in case of reverse proxy with forward prefix. It also adds test cases for both cases.

Example: Base URL duplicated by Inertia response

There is a thread on laracasts, URL duplicates in shared hosting deployed Inertia App, which illustrates the base url problem:

However, the URL in the browser is rewritten whenever I access a page.

https://www.mydomainname.com/project turns into https://www.mydomainname.com/project/project

The same thing happens with the other route. https://www.mydomainname.com/project/github turns into https://www.mydomainname.com/project/project/github

Checking with vue devtools, Inertia.initialPage.url seems to be set to the incorrect value (the one with a duplicate)

This problem happens because the $request->getRequestUri() contains the full request URI as sent to the application server, which also includes the base url: https://github.com/inertiajs/inertia-laravel/blob/12e7937e99f6bdb114541c7f907bef1339b1c843/src/Response.php#L102

There are more users who experienced this problem, e.g. root directory is displayed twice in URL on load and reload

Example: Reverse Proxy Deployment with Forward Prefix

Suppose the application has a public URL https://intranet.example.org/to/project. This is served through a reverse proxy, which requests the content in the background from http://localhost:8080.

Using the X-Forwarded-Prefix header, the reverse proxy can instruct symfony to prepend the forward prefix to the baseUrl. This was the setting reported in https://github.com/inertiajs/inertia-laravel/pull/333.

Since the forward prefix is not included in the getRequestUri() the problem of a duplicated base url does not arise in this case.

flexponsive commented 8 months ago

Related PRs

I saw there are a couple of open pull requests here addressing the same issue:

User impact

Additionally to the examples above, the "inertia duplicates the base url" problem also came up in different forms on stackoverflow, e.g. Ziggy router modifying browser urls or root directory is displayed twice in URL on load and reload

Many thanks to the entire Inertia team for this amazing project! I hope this PR can contribute to solving one of the very few longstanding issues.

murilomagalhaes commented 8 months ago

At this point i've just given up for any PR that fixes this problem to be merged. Sadly, i've seen a lot of them being closed or just ignored.

flexponsive commented 8 months ago

Following the discussion with @RobertBoes, I changed the code to make explicit that - in line with the inertia protocol example - the url property should not contain the HTTP scheme and host name.

The code now reads:

 'url' => str_replace($request->getSchemeAndHttpHost(), '', $request->getUri()),

I would have liked use the fullUrl method, but this runs into the problem that when accessing the root URL, the fullUrl method does not return a trailing slash (so one would have to write str_replace($request->getSchemeAndHttpHost(), '', $request->fullUrl()) ?: '/', which is a bit ugly).

jessarcher commented 5 months ago

Thanks for the PR, and I'm sorry for the delay! I ended up going with a different but very similar approach (#592), but this PR was very helpful!