Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Anonymous function rewritten as static one to maintain compatibility with PHP 5.2 #293

Merged
merged 1 commit into from

4 participants

@mhujer

Fixes #292

@Ocramius
Collaborator

@mhujer I know this is kinda urgent, but it doesn't look like the tests pass on 5.2: https://travis-ci.org/zendframework/zf1/jobs/20264719#L1408

The build will most probably fail anyway because of missing phar

@mhujer

No, build on PHP 5.2 for release tag failed mainly because of this anonymous function called from many ZF classes. (It passed before recent security fixes: https://travis-ci.org/zendframework/zf1/builds)
The error in Zend/Xml is not appearing on other versions. But I dare say, that it was there before my fix, but not visible: https://travis-ci.org/zendframework/zf1/jobs/20225723#L1068

Executing Zend/Xml/AllTests.php
Parse error: syntax error, unexpected T_FUNCTION, expecting ')' in /home/travis/build/zendframework/zf1/library/Zend/Xml/Security.php on line 76
Finished executing Zend/Xml/AllTests.php
@Ocramius
Collaborator

@mhujer yeah, the fatal is obvious. Looking into the failing test...

@froschdesign froschdesign merged commit abc9a8e into from
@mhujer mhujer deleted the branch
@weierophinney weierophinney commented on the diff
library/Zend/Xml/Security.php
@@ -73,12 +88,8 @@ public static function scan($xml, DOMDocument $dom = null)
// Load XML with network access disabled (LIBXML_NONET)
// error disabled with @ for PHP-FPM scenario
- set_error_handler(function ($errno, $errstr) {
- if (substr_count($errstr, 'DOMDocument::loadXML()') > 0) {
- return true;
- }
- return false;
- }, E_WARNING);
+ set_error_handler(array('Zend_Xml_Security', '_loadXmlErrorHandler'), E_WARNING);
@weierophinney Owner

Not sure how this can possibly work if _loadXmlErrorHandler is declared with protected visibility. Going to push through an update that makes it public.

@mhujer
mhujer added a note

:+1:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@weierophinney weierophinney referenced this pull request from a commit
@weierophinney weierophinney [#293] Make new method public
- Since it's being invoked as a static callback, the method needs to be
  public. Renamed to remove the underscore prefix, and marked as public.
c675b9d
@weierophinney weierophinney referenced this pull request from a commit
@weierophinney weierophinney [#293] Ensure Zend\Xml tests pass on PHP 5.2
- Prior to 5.3, you needed to cast a simplexml node to a scalar in order
  to do comparisons.
eca162a
@weierophinney weierophinney referenced this pull request from a commit
@weierophinney weierophinney [#293] Update .gitignore rules
- Discovered on installing PHPUnit to run tests that .gitignore was not setup to
  ignore composer.lock, nor the files installed in the bin/ directory.
4d75497
@weierophinney weierophinney referenced this pull request from a commit
@weierophinney weierophinney Merge branch 'hotfix/293'
Fixes issues seen in #293
8281917
@weierophinney weierophinney added this to the 1.12.5 milestone
@xopherdeep xopherdeep referenced this pull request from a commit in xopherdeep/Zend-Framework-v1
@weierophinney weierophinney [#293] Make new method public
- Since it's being invoked as a static callback, the method needs to be
  public. Renamed to remove the underscore prefix, and marked as public.
0cca53e
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Mar 7, 2014
  1. @mhujer
This page is out of date. Refresh to see the latest.
Showing with 17 additions and 6 deletions.
  1. +17 −6 library/Zend/Xml/Security.php
View
23 library/Zend/Xml/Security.php
@@ -45,6 +45,21 @@ protected static function heuristicScan($xml)
}
/**
+ * @param integer $errno
+ * @param string $errstr
+ * @param string $errfile
+ * @param integer $errline
+ * @return bool
+ */
+ protected static function _loadXmlErrorHandler($errno, $errstr, $errfile, $errline)
+ {
+ if (substr_count($errstr, 'DOMDocument::loadXML()') > 0) {
+ return true;
+ }
+ return false;
+ }
+
+ /**
* Scan XML string for potential XXE and XEE attacks
*
* @param string $xml
@@ -73,12 +88,8 @@ public static function scan($xml, DOMDocument $dom = null)
// Load XML with network access disabled (LIBXML_NONET)
// error disabled with @ for PHP-FPM scenario
- set_error_handler(function ($errno, $errstr) {
- if (substr_count($errstr, 'DOMDocument::loadXML()') > 0) {
- return true;
- }
- return false;
- }, E_WARNING);
+ set_error_handler(array('Zend_Xml_Security', '_loadXmlErrorHandler'), E_WARNING);
@weierophinney Owner

Not sure how this can possibly work if _loadXmlErrorHandler is declared with protected visibility. Going to push through an update that makes it public.

@mhujer
mhujer added a note

:+1:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+
$result = $dom->loadXml($xml, LIBXML_NONET);
restore_error_handler();
Something went wrong with that request. Please try again.