Skip to content
This repository was archived by the owner on Jan 8, 2020. It is now read-only.

Zend\Escaper integration into Zend\View (BC break since Escape helper is now EscapeHtml)#1750

Merged
weierophinney merged 7 commits into
zendframework:masterfrom
padraic:feature/zend-view-with-escaper
Jul 5, 2012
Merged

Zend\Escaper integration into Zend\View (BC break since Escape helper is now EscapeHtml)#1750
weierophinney merged 7 commits into
zendframework:masterfrom
padraic:feature/zend-view-with-escaper

Conversation

@padraic

@padraic padraic commented Jul 4, 2012

Copy link
Copy Markdown
Member

Integrates Zend\Escaper across Zend\View helpers. Replaces Escape helper with EscapeHtml, EscapeHtmlAttr, EscapeJs, EscapeCSS and EscapeUrl helpers instead. Tests checked for Zend\View and Zend\Escaper.

Other background:

  1. 2nd parameter to __invoke() on Escape* classes has been preserved.
  2. Rationale for Escape helper deprecation is to enforce selection of correct Escape* helper per agreed RFC.
  3. A Placeholder class had an inconsistent use of htmlspecialchars() directly (not via Escape helper). This was replaced with an instance of Zend\Escaper\Escaper.
  4. Helpers can likely be refactored a bit further into the Helper\Escaper\AbstractHelper class.
  5. Helpers likely still need a brief security review (there's a usage of Zend\Json instead of a true Javascript escaper but is likely safe - will review post-beta).

@travisbot

Copy link
Copy Markdown

This pull request fails (merged 39140db into f8e8120).

Comment thread library/Zend/View/Helper/EscapeCss.php Outdated

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd use byRef here to just remove $k

@Ocramius

Ocramius commented Jul 4, 2012

Copy link
Copy Markdown
Member

The logic of the AbstractHelper is very very similar to the one of its concrete implementations. You could just reduce the concrete implementations to the simple implementation of

public function doEscape($string);

That should reduce code introduced by this PR by a lot

Comment thread library/Zend/View/Helper/EscapeUrl.php Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Class is named incorrectly (EscapeCss instead of EscapeUrl)

@weierophinney weierophinney merged commit 10bd6bc into zendframework:master Jul 5, 2012
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants