Skip to content
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

[DependencyInjection] Unescape parameters for all types of injection #16295

Merged
merged 1 commit into from Nov 19, 2015

Conversation

Projects
None yet
6 participants
@Nicofuma
Copy link
Contributor

commented Oct 20, 2015

Q A
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets
License MIT
Doc PR

The parameters must be escaped when injected in the container.
But they are only unescaped when the container is dumped and when these parameters are used in the service constructor.
We need to unescape them every time their are injected (constructor, setter and property injection)

@Nicofuma Nicofuma force-pushed the Nicofuma:patch-1 branch from b7ac104 to d7f3864 Oct 20, 2015

@stof

This comment has been minimized.

Copy link
Member

commented Oct 20, 2015

I don't understand what you are trying to fix here

@Nicofuma

This comment has been minimized.

Copy link
Contributor Author

commented Oct 20, 2015

Migth be clearer with a simple example: https://gist.github.com/Nicofuma/a7672bf0776efb2eaa5f
the output is:

"%%unescape_it%%"
"%unescape_it%"

while we are expecting:

"%unescape_it%"
"%unescape_it%"
@Nicofuma

This comment has been minimized.

Copy link
Contributor Author

commented Nov 2, 2015

Bump, this is a real issue for us as it throws an error to some of our users - because they have a % in their database password - each time they clear the cache.

@Nicofuma Nicofuma closed this Nov 2, 2015

@Nicofuma Nicofuma reopened this Nov 2, 2015

@Oyabun1

This comment has been minimized.

Copy link

commented Nov 2, 2015

this is a real issue for us as it throws an error to some of our users - because they have a % in their database password

I've had a few users of phpBB, experiencing the issue with the % character, try the changes in the commit and it resolved the problem for them.

@Nicofuma

This comment has been minimized.

Copy link
Contributor Author

commented Nov 10, 2015

Bump again

@stof could you give this PR another look please?

@Nicofuma

This comment has been minimized.

Copy link
Contributor Author

commented Nov 16, 2015

@stof @fabpot @Tobion anyone, could it be possible to have some feedbacks please?

@Nicofuma Nicofuma force-pushed the Nicofuma:patch-1 branch from d7f3864 to c7f100e Nov 19, 2015

@Nicofuma Nicofuma force-pushed the Nicofuma:patch-1 branch from c7f100e to 331a046 Nov 19, 2015

@nicolas-grekas

This comment has been minimized.

Copy link
Member

commented Nov 19, 2015

👍
Status: reviewed

@fabpot

This comment has been minimized.

Copy link
Member

commented Nov 19, 2015

Thank you @Nicofuma.

@fabpot fabpot merged commit 331a046 into symfony:2.3 Nov 19, 2015

2 of 3 checks passed

continuous-integration/travis-ci/pr The Travis CI build could not complete due to an error
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
fabbot.io Your code looks good.
Details

fabpot added a commit that referenced this pull request Nov 19, 2015

bug #16295 [DependencyInjection] Unescape parameters for all types of…
… injection (Nicofuma)

This PR was merged into the 2.3 branch.

Discussion
----------

[DependencyInjection] Unescape parameters for all types of injection

| Q             | A
| ------------- | ---
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets |
| License       | MIT
| Doc PR        |

The parameters must be escaped when injected in the container.
But they are only unescaped when the container is dumped and when these parameters are used in the service constructor.
We need to unescape them every time their are injected (constructor, setter and property injection)

Commits
-------

331a046 [DependencyInjection] Unescape parameters for all types of injection

This was referenced Nov 23, 2015

This was referenced Nov 30, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.