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.

Sign up for GitHub

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

Jump to bottom

[twig] do not escape filter url. #314

Closed
wants to merge 2 commits into from
Closed

[twig] do not escape filter url. #314

wants to merge 2 commits into from

Conversation

makasim
Copy link
Collaborator

@makasim makasim commented Feb 3, 2014

@havvg havvg added this to the v0.18.x milestone Feb 5, 2014
@havvg
Copy link
Contributor

havvg commented Feb 6, 2014

The change seems reasonable, did you verify it in any way?

@makasim
Copy link
Collaborator Author

makasim commented Feb 6, 2014

Let's keep it open I will play with it a bit more

@makasim
Copy link
Collaborator Author

makasim commented Feb 7, 2014

@havvg I've just checked it. Router escape parameters very well, Tried:

  • hack img "/>Text<div - <img src="/appp.php/media/cache/thumbnail_web_path/%22/%3EText%3Cdiv%20" />
  • script tag: <script> - <img src="/appp.php/media/cache/thumbnail_web_path/<script>" />
  • urls element ?& - <img src="/appp.php/media/cache/thumbnail_web_path/?&" />

looks good

@havvg
Copy link
Contributor

havvg commented Feb 7, 2014

Did you remove the double encoding? (It's not part of the PR) ;-)

@makasim
Copy link
Collaborator Author

makasim commented Feb 7, 2014

@havvg hm, I removed it in develop (here #320) but not in master.

fixed now, unfortunately I cannot quickly switch sandbox to ensure it works

@makasim
Copy link
Collaborator Author

makasim commented Feb 17, 2014

@havvg ping

@makasim
Copy link
Collaborator Author

makasim commented Mar 5, 2014

I am going to merge this to master tomorrow.

@makasim
Copy link
Collaborator Author

makasim commented Mar 6, 2014

close in favor #337. I decided to commit this change to develop since I am not sure it does not break anything in master.

@makasim makasim closed this Mar 6, 2014
@makasim makasim deleted the twig-helper-not-escape-filter-url branch March 6, 2014 15:05
@makasim makasim mentioned this pull request Jul 5, 2015
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
0.20.x
Development

Successfully merging this pull request may close these issues.
2 participants
@makasim @havvg