8000 [DI] Add "force_parameters_escape" option to dumpers by fancyweb · Pull Request #34541 · symfony/symfony · GitHub
[go: up one dir, main page]

Skip to content

[DI] Add "force_parameters_escape" option to dumpers #34541

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

Closed

Conversation

fancyweb
Copy link
Contributor
@fancyweb fancyweb commented Nov 23, 2019
Q A
Branch? 3.4
Bug fix? yes
New feature? no
Deprecations? no
Tickets #34526
License MIT
Doc PR -

In ContainerBuilderDebugDumpPass we dump an uncompiled container. That means the dumpers do not escape parameters. I guess we need to add an easy way to force this escaping. That fixes the problem with the new lint command. It was not detected before because the debug:container command never compiles the container (contrary to the new one).

If it's too complicated, we could just rebuild a fresh container everytime in the lint command.

#SymfonyHackday

@fancyweb fancyweb force-pushed the di-force-parameters-escape-in-dumpers branch from aec766c to 72e361f Compare November 23, 2019 13:31
@nicolas-grekas
Copy link
Member
nicolas-grekas commented Nov 24, 2019

I don't think this is the right approach. This will break scripts that expect the current escaping strategy that varies by compiled/not-compiled.
Instead, we could deal with this at the loader level, on 4.4.

@nicolas-grekas nicolas-grekas added this to the 3.4 milestone Nov 24, 2019
@nicolas-grekas
Copy link
Member

Closing as explained, let's continue on 4.4

@fancyweb fancyweb deleted the di-force-parameters-escape-in-dumpers branch November 28, 2019 10:30
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.

3 participants
0