[#ZF-3781] URL view helper does not escape html special chars - Zend Framework Issue Tracker

Details

Type: Bug
Status: Open
Priority: Major
Resolution: Unresolved
Affects Version/s: 1.5.0RC1, 1.5.0RC2, 1.5.0RC3, 1.5.0, 1.5.1, 1.5.2, 1.6.0, 1.6.1, 1.6.2, 1.7 Preview Release, 1.7.3, 1.8.0, Next Major Release
Fix Version/s: Next Major Release
Component/s: Zend_View
Labels:
- zf-crteam-padraic
- zf-crteam-priority

Description

The URL view helper currently does automatically escape html special chars, so it has to be done manually yet. Since it is only intended for the view, it actually should.

Activity

Ascending order - Click to sort in descending order

Hide

Permalink

Jon Whitcraft added a comment - 23/Oct/08 6:38 PM

Can you provide a use case to for this? To me since it just uses the router to assemble the url you should have to escape the data before you pass it in. But if you can make a good case for this it can be considered for a change.

Show

Jon Whitcraft added a comment - 23/Oct/08 6:38 PM Can you provide a use case to for this? To me since it just uses the router to assemble the url you should have to escape the data before you pass it in. But if you can make a good case for this it can be considered for a change.

Hide

Permalink

Ben Scholzen added a comment - 24/Oct/08 12:11 AM

Yeah sure (and hi Jon, btw, hope you remember me ).

When creating a URL in a view, which creates an ampersand, double-quotes or other characters (which are valid for URLs), the generated source code isn't valid anymore. Since this is a view helper, it should escape it's output with the escape() view helper.

Show

Ben Scholzen added a comment - 24/Oct/08 12:11 AM Yeah sure (and hi Jon, btw, hope you remember me

). When creating a URL in a view, which creates an ampersand, double-quotes or other characters (which are valid for URLs), the generated source code isn't valid anymore. Since this is a view helper, it should escape it's output with the escape() view helper.

Hide

Permalink

Jon Whitcraft added a comment - 24/Oct/08 7:42 AM

Yes i do remember you. And What view helper are you using? I'm using The Rewirte Router (urls are formated like /help/1/foo/bar) so i don't get the ? and & on my urls. I think this may be a possibility to change but it has to be specific to a router...

Show

Jon Whitcraft added a comment - 24/Oct/08 7:42 AM Yes i do remember you. And What view helper are you using? I'm using The Rewirte Router (urls are formated like /help/1/foo/bar) so i don't get the ? and & on my urls. I think this may be a possibility to change but it has to be specific to a router...

Hide

Permalink

Jon Whitcraft added a comment - 24/Oct/08 7:53 AM

Can you also produce a test case for this issue?

Show

Jon Whitcraft added a comment - 24/Oct/08 7:53 AM Can you also produce a test case for this issue?

Hide

Permalink

Ben Scholzen added a comment - 24/Oct/08 8:28 AM

I could even do the changes myself, I just didn't to it yet cause I was too lazy.

Zend_Controller_Router_Route: /article/:name
$route->assemble(array('name' => 'foo & bar'));

Actual Output:
/article/foo+&+bar

Expected Output (with default escape() settings):
/article/foo+&+bar

Show

Ben Scholzen added a comment - 24/Oct/08 8:28 AM I could even do the changes myself, I just didn't to it yet cause I was too lazy. Zend_Controller_Router_Route: /article/:name $route->assemble(array('name' => 'foo & bar')); Actual Output: /article/foo+&+bar Expected Output (with default escape() settings): /article/foo+&+bar

Hide

Permalink

Ben Scholzen added a comment - 24/Oct/08 8:29 AM

Argh, wrongly escape...

Expected Output (with default escape() settings):

/article/foo+&+bar{/code}

Show

Ben Scholzen added a comment - 24/Oct/08 8:29 AM Argh, wrongly escape... Expected Output (with default escape() settings):

/article/foo+&+bar{/code}

Hide

Permalink

Ben Scholzen added a comment - 24/Oct/08 8:30 AM

Ok now it gets stupid...

Expected output:

/article/foo+&amp;+bar

Show

Ben Scholzen added a comment - 24/Oct/08 8:30 AM Ok now it gets stupid... Expected output:

/article/foo+&amp;+bar

Hide

Permalink

Jon Whitcraft added a comment - 24/Oct/08 8:46 AM

Doing this would break BC as most people are already using the $this->escape() and it would double escape anything that they have already escaped.

Show

Jon Whitcraft added a comment - 24/Oct/08 8:46 AM Doing this would break BC as most people are already using the $this->escape() and it would double escape anything that they have already escaped.

Hide

Permalink

Ben Scholzen added a comment - 24/Oct/08 10:21 AM

Then I mark this as fix for 2.0, as there we are able to break BC.

Show

Ben Scholzen added a comment - 24/Oct/08 10:21 AM Then I mark this as fix for 2.0, as there we are able to break BC.

Hide

Permalink

Jon Whitcraft added a comment - 10/Nov/08 6:15 PM

Moving this issue to be Unassigned as it's posponed till the 2.0 cycle as it would break BC.

Show

Jon Whitcraft added a comment - 10/Nov/08 6:15 PM Moving this issue to be Unassigned as it's posponed till the 2.0 cycle as it would break BC.

Hide

Permalink

Ben Scholzen added a comment - 13/Nov/08 2:11 AM

I found a solution to not have it as a BC-break, but instead as additional feature (until we clearify the default behaviour for 2.0):

I'll add a static method Zend_View_Helper_Url::setEscape(boolean $enable). By default it will have the old behaviour (of not escaping ULRs), but those who want that feature can enable it (e.g. in their bootstrap). I would also suggest an additional parameter in the Zend_View_Helper_Url::url() method to set the escaping for a single call (null = default behaviour set by setEscape, false = don't escape, true = escape).

I'll let Matthew review this before working on it.

Show

Ben Scholzen added a comment - 13/Nov/08 2:11 AM I found a solution to not have it as a BC-break, but instead as additional feature (until we clearify the default behaviour for 2.0): I'll add a static method Zend_View_Helper_Url::setEscape(boolean $enable). By default it will have the old behaviour (of not escaping ULRs), but those who want that feature can enable it (e.g. in their bootstrap). I would also suggest an additional parameter in the Zend_View_Helper_Url::url() method to set the escaping for a single call (null = default behaviour set by setEscape, false = don't escape, true = escape). I'll let Matthew review this before working on it.

Hide

Permalink

Matthew Weier O'Phinney added a comment - 18/Nov/08 10:55 AM

I see three solutions:

Pass variables through escape() as you pass them to the helper (current method)
Add an optional argument to the url() signature for "escape", allowing per-call escaping
Have a static flag for setting the default behavior

All of these are BC.

Show

Matthew Weier O'Phinney added a comment - 18/Nov/08 10:55 AM I see three solutions:

Pass variables through escape() as you pass them to the helper (current method)
Add an optional argument to the url() signature for "escape", allowing per-call escaping
Have a static flag for setting the default behavior

All of these are BC.

Hide

Permalink

Ralph Schindler added a comment - 18/Feb/11 2:33 PM

Should we postpone or move forward with one of Matthews proposed solutions?

Show

Ralph Schindler added a comment - 18/Feb/11 2:33 PM Should we postpone or move forward with one of Matthews proposed solutions?

Hide

Permalink

Ben Scholzen added a comment - 19/Feb/11 3:57 AM

I'd say postpone it for 2.0 and make escaping be the default, but have an option to disable it.

Show

Ben Scholzen added a comment - 19/Feb/11 3:57 AM I'd say postpone it for 2.0 and make escaping be the default, but have an option to disable it.

Hide

Permalink

Pádraic Brady added a comment - 13/Aug/11 7:50 PM

Added Next Major Release as an affected version for filtering

Show

Pádraic Brady added a comment - 13/Aug/11 7:50 PM Added Next Major Release as an affected version for filtering

Hide

Permalink

Adam Lundrigan added a comment - 17/Oct/11 7:33 PM

Should I resolve this ticket as 'Postponed' and clone it into ZF2, then?

Show

Adam Lundrigan added a comment - 17/Oct/11 7:33 PM Should I resolve this ticket as 'Postponed' and clone it into ZF2, then?

People

Assignee:

Ben Scholzen

Reporter:

Ben Scholzen

Vote (0)

Watch (2)

Dates

Created:

28/Jul/08 10:28 AM

Updated:

17/Oct/11 7:33 PM