Skip to content

[FrameworkBundle] Deprecate container parameters router.request_context.scheme and .host #61457

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

Open
wants to merge 1 commit into
base: 7.4
Choose a base branch
from

Conversation

stollr
Copy link

@stollr stollr commented Aug 19, 2025

Q A
Branch? 7.4
Bug fix? no
New feature? no
Deprecations? yes
Issues Fix #53919
License MIT

The framework.router.default_uri config is used to define the request context as fallback for the case that no HTTP request is available. But it does not affect the container params

  • router.request_context.base_url
  • router.request_context.host
  • router.request_context.scheme

They still use the defaults, which are defined in the routing.php config file.

This leads to inconsistency and confusion if a third party bundle relies on the container parameters instead of the request context service.

This PR deprecates those container params like proposed in #53919.

@carsonbot carsonbot added this to the 7.4 milestone Aug 19, 2025
@carsonbot carsonbot changed the title Deprecate container params router.request_context.* Deprecate container params router.request_context.* Aug 19, 2025
@stollr stollr force-pushed the issue53919/deprecate_request_context_params branch from 3b8c492 to 6aad228 Compare August 19, 2025 07:41
@carsonbot carsonbot changed the title Deprecate container params router.request_context.* [FrameworkBundle] Deprecate container params router.request_context.* Aug 19, 2025
@stollr stollr force-pushed the issue53919/deprecate_request_context_params branch from 6aad228 to ec2dade Compare August 19, 2025 07:50
@stof
Copy link
Member

stof commented Aug 19, 2025

This is not marking the parameters as deprecated to warn places using them.

@stollr
Copy link
Author

stollr commented Aug 19, 2025

You mean that the !isset($config['default_uri']) should be removed from the condition? Or is there a better way?

@stof
Copy link
Member

stof commented Aug 19, 2025

@stollr stollr force-pushed the issue53919/deprecate_request_context_params branch from ec2dade to ffdc268 Compare August 19, 2025 10:37
@stollr
Copy link
Author

stollr commented Aug 19, 2025

Thanks for the hint. I have updated the patch.

@nicolas-grekas
Copy link
Member

I wouldn't deprecate router.request_context.base_url (the others I would).
Instead, I'd fix it this way:

--- a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php
+++ b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php
@@ -1334,8 +1334,7 @@ class FrameworkExtension extends Extension
         $container->setParameter('request_listener.https_port', $config['https_port']);
 
         if (null !== $config['default_uri']) {
-            $container->getDefinition('router.request_context')
-                ->replaceArgument(0, $config['default_uri']);
+            $container->setParameter('router.request_context.base_url', $config['default_uri']);
         }
     }

@stollr
Copy link
Author

stollr commented Aug 20, 2025

Wouldn't this be a potential source of confusion? If a user sets router.request_context.base_url to "example.com", the default_url (maybe via an ENV variable) to something else and at the end, base_url will have another value than what he defined?

And what would be the advantage?

@nicolas-grekas
Copy link
Member

nicolas-grekas commented Aug 20, 2025

The advantage is not breaking things (deprecating first but still) and allowing bundles to read the value also.
The confusion would be the responsibility of the app, not ours.

@stollr
Copy link
Author

stollr commented Aug 20, 2025

Historically the router.request_context.base_url was used to specify only the URL path (for example here or in the older version of the symfony doc).

Imho overwriting the base_url with the default_uri, which is always an absolute URI containing scheme and host, could lead to a hidden BC break.

@nicolas-grekas
Copy link
Member

This was later improved to support full URI, see #36651
This also matches the fact that the config value is injected at the same position as the param, which means that today already, one can set router.request_context.base_url and basically configure the default_uri

@stollr stollr force-pushed the issue53919/deprecate_request_context_params branch from ffdc268 to 3493bcc Compare August 20, 2025 08:05
@stollr
Copy link
Author

stollr commented Aug 20, 2025

Okay, I have applied your suggestion ;-)

@nicolas-grekas nicolas-grekas changed the title [FrameworkBundle] Deprecate container params router.request_context.* [FrameworkBundle] Deprecate container parameters router.request_context.scheme and .host Aug 20, 2025
@nicolas-grekas nicolas-grekas force-pushed the issue53919/deprecate_request_context_params branch from 3493bcc to 01ec276 Compare August 20, 2025 08:20
@nicolas-grekas nicolas-grekas force-pushed the issue53919/deprecate_request_context_params branch from 01ec276 to 8034510 Compare August 20, 2025 09:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Configuring framework.router.default_uri has no impact on request context parameters
5 participants