-
-
Notifications
You must be signed in to change notification settings - Fork 947
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
Allow overriding TRUSTED_HOSTS and MERCURE_PUBLIC_URL variables #2666
Conversation
…C_URL variables Fix issue with invalid TRUSTED_HOSTS and MERCURE_PUBLIC_URL when using http://localhost or :80 as SERVER_NAME
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Could you update the docs to mention these new variables (the current example is still broken, even with your changes, because the new variables must be explicitly set) and also update Symfony Docker accordingly? https://github.com/dunglas/symfony-docker/blob/main/compose.yaml
Of course. In which branch this change will be merged?
|
Target the 3.3 branch for the docs. We don't have branches for the distribution. |
Modifying the SERVER_NAME to disable HTTPS, as outlined in the API Platform documentation, leads to improper configurations for TRUSTED_HOSTS and MERCURE_PUBLIC_URL when using formats like
http://localhost
or:80
.This results in values like
^:80|php$
or^http://localhost|php$
forTRUSTED_HOSTS
, and URLs such ashttps://:80/.well-known/mercure
orhttps://http://localhost/.well-known/mercure
forMERCURE_PUBLIC_URL
.This PR adopts "nested interpolation" from the Docker documentation to allow precise overrides, introducing
TRUSTED_PROXIES
andCADDY_MERCURE_PUBLIC_URL
as new, adjustable variables, with room for naming discussions.