Skip to content

Conversation

@gguichard
Copy link
Contributor

Symfony HttpFoundation supports X-Forwarded-Prefix HTTP header since version 5.3.
This header is used by Traefik reverse proxy for example.

This ensures that the links rendered by Laravel application deployed behind LB are valid even if the application is deployed via prefix URL.

Example routing setup:
route /admin/(.*) => Laravel backend /$1
in this case links rendered by Laravel backend must start with /admin/

c.f. symfony/symfony#37734

@derekmd
Copy link
Contributor

derekmd commented Jan 5, 2022

This change has been made for 9.x (#40014) being released this month since it was deemed a breaking change (#40003).

Although having your test additions on the master branch might be nice since that previous PR didn't touch the test suite.

@GrahamCampbell
Copy link
Collaborator

This is a breaking change, and cannot be made to 8.x.

@gguichard
Copy link
Contributor Author

gguichard commented Jan 5, 2022

Okay sorry I didn't see the PR for 9.x.

After this patch (and the other patch for 9.x which is almost the same), non-absolute URLs generated by the router (RouteUrlGenerator) are still not correctly prefixed with the value provided by X-Forwarded-Prefix.

The prefix provided by X-Forwarded-Prefix is handled in getBaseUrl() function from Symfony HttpFoundation (see https://github.com/symfony/http-foundation/blob/ce952af52877eaf3eab5d0c08cc0ea865ed37313/Request.php#L903)
but in the following code:

$uri = preg_replace('#^'.$base.'#i', '', $uri);

The base url is removed from generated URI with a preg_replace call. After this call the generated URI becomes invalid.

@taylorotwell
Copy link
Member

Closing this on 8.x since we are attempting to solve this on the master branch (which is just a few weeks from release). @gguichard are you able to fix the other problem you mentioned?

@gguichard gguichard deleted the feat_http_x_forwarded_prefix branch February 4, 2022 16:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants