From 865bb94ae30815840a526ab206f44b610bdd61ed Mon Sep 17 00:00:00 2001 From: Mike de Heij Date: Tue, 6 Sep 2016 12:21:35 +0200 Subject: [PATCH] PHP7 consistent ordering for splitHttpAcceptHeader In PHP7 sorting order is changed, splitHttpAcceptHeader was relying on unspecified behaviour of arsort(). Sorting order is now made explicit. --- .travis.yml | 11 ++++ composer.json | 43 ++++++++++++++-- lib/request/sfWebRequest.class.php | 63 ++++++++++++++--------- phpunit.xml.dist | 17 ++++++ test/lib/request/sfWebRequestTest.php | 74 +++++++++++++++++++++++++++ 5 files changed, 182 insertions(+), 26 deletions(-) create mode 100644 .travis.yml create mode 100644 phpunit.xml.dist create mode 100644 test/lib/request/sfWebRequestTest.php diff --git a/.travis.yml b/.travis.yml new file mode 100644 index 0000000000..d9a5bbeb7e --- /dev/null +++ b/.travis.yml @@ -0,0 +1,11 @@ +language: php +php: + - 7.0 + - 5.6 + - hhvm + +before_script: + - composer install --dev + +script: + - vendor/bin/phpunit diff --git a/composer.json b/composer.json index 719c3379b7..e39cca06ff 100644 --- a/composer.json +++ b/composer.json @@ -1,11 +1,48 @@ { - "name": "symfony/symfony1", + "name": "symfony/symfony1", "description": "Symfony is a complete framework designed to optimize the development of web applications by way of several key features. For starters, it separates a web application's business rules, server logic, and presentation views. It contains numerous tools and classes aimed at shortening the development time of a complex web application. Additionally, it automates common tasks so that the developer can focus entirely on the specifics of an application. The end result of these advantages means there is no need to reinvent the wheel every time a new web application is built!", - "authors": [ + "authors": [ { "name": "Fabien Potencier", "email": "fabien@symfony.com" } ], - "license": "MIT" + "license": "MIT", + "require-dev": { + "phpunit/phpunit": "^5.5" + }, + "autoload-dev": { + "classmap": [ + "lib/action", + "lib/addon", + "lib/autoload", + "lib/cache", + "lib/command", + "lib/config", + "lib/controller", + "lib/database", + "lib/debug", + "lib/event", + "lib/exception", + "lib/filter", + "lib/form", + "lib/generator", + "lib/helper", + "lib/i18n", + "lib/log", + "lib/plugin", + "lib/request", + "lib/response", + "lib/routing", + "lib/storage", + "lib/task", + "lib/user", + "lib/util", + "lib/validator", + "lib/vendor", + "lib/view", + "lib/widget", + "lib/yaml" + ] + } } diff --git a/lib/request/sfWebRequest.class.php b/lib/request/sfWebRequest.class.php index 4a386006a4..d3d666178a 100644 --- a/lib/request/sfWebRequest.class.php +++ b/lib/request/sfWebRequest.class.php @@ -927,34 +927,51 @@ public function setRelativeUrlRoot($value) $this->relativeUrlRoot = $value; } - /** - * Splits an HTTP header for the current web request. - * - * @param string $header Header to split - */ - public function splitHttpAcceptHeader($header) +/** + * Splits an HTTP header for the current web request. + * + * @param string $header Header to split + */ +public function splitHttpAcceptHeader($header) +{ + $i = 0; + $values = array(); + foreach (array_filter(explode(',', $header)) as $value) { - $values = array(); - foreach (array_filter(explode(',', $header)) as $value) + // Cut off any q-value that might come after a semi-colon + if ($pos = strpos($value, ';')) { - // Cut off any q-value that might come after a semi-colon - if ($pos = strpos($value, ';')) - { - $q = (float) trim(substr($value, $pos + 3)); - $value = trim(substr($value, 0, $pos)); - } - else - { - $q = 1; - } - - $values[$value] = $q; + $q = (float) trim(substr($value, $pos + 3)); + $value = trim(substr($value, 0, $pos)); + } + else + { + $q = 1; } - arsort($values); - - return array_keys($values); + $values[$value] = array($q, $i++); } + /** + * Implement reverse stable sort instead of arsort + */ + uasort($values, function ($a, $b) { + if ($a[0] == $b[0]) { + // Can not be same since we used $i++ + if ($a[1] < $b[1]) { + return 1; + } else { + return -1; + } + } elseif ($a[0] < $b[0]) { + return 1; + } else { + return -1; + } + }); + + return array_keys($values); +} + /** * Returns the array that contains all request information ($_SERVER or $_ENV). diff --git a/phpunit.xml.dist b/phpunit.xml.dist new file mode 100644 index 0000000000..f628c0b9fb --- /dev/null +++ b/phpunit.xml.dist @@ -0,0 +1,17 @@ + + + + + + ./test/ + + + + + ./lib + + + diff --git a/test/lib/request/sfWebRequestTest.php b/test/lib/request/sfWebRequestTest.php new file mode 100644 index 0000000000..4c0f4d4751 --- /dev/null +++ b/test/lib/request/sfWebRequestTest.php @@ -0,0 +1,74 @@ +request = new sfWebRequest($event_dispatcher); + } + + /** + * @dataProvider splitHttpAcceptHeaderProvider + * @covers sfWebRequest::splitHttpAcceptHeader + */ + public function testSplitHttpAcceptHeader($header, $expected) + { + self::assertSame($expected, $this->request->splitHttpAcceptHeader($header)); + } + + public function splitHttpAcceptHeaderProvider() + { + return [ + [ + "text/javascript, text/html, application/xml, text/xml, */*", + [ + ' */*', + ' text/xml', + ' application/xml', + ' text/html', + 'text/javascript', + ] + ], + [ + "text/javascript, text/html;q=2.9, application/xml, text/xml, */*", + [ + 'text/html', + ' */*', + ' text/xml', + ' application/xml', + 'text/javascript', + ] + ], + [ + "text/javascript;q=1.0, text/html;q=2.9, application/xml, text/xml, */*", + [ + 'text/html', + ' */*', + ' text/xml', + ' application/xml', + 'text/javascript', + ] + ], + [ + "text/javascript;q=1.0, text/html;q=1.0, application/xml, text/xml, */*", + [ + ' */*', + ' text/xml', + ' application/xml', + 'text/html', + 'text/javascript', + ] + ] + ]; + } +}