From 6c971c302848a81a93dd626e53e8769dcd1ff4b7 Mon Sep 17 00:00:00 2001 From: Jordi Boggiano Date: Mon, 9 Feb 2015 19:11:32 +0000 Subject: [PATCH] Fix regex matching and add more tests for addSubNode, refs #3721, fixes #3716 --- src/Composer/Json/JsonManipulator.php | 56 ++++++++----- .../Test/Json/JsonManipulatorTest.php | 80 +++++++++++++++++-- 2 files changed, 110 insertions(+), 26 deletions(-) diff --git a/src/Composer/Json/JsonManipulator.php b/src/Composer/Json/JsonManipulator.php index e719e3d6d..d30a68385 100644 --- a/src/Composer/Json/JsonManipulator.php +++ b/src/Composer/Json/JsonManipulator.php @@ -31,7 +31,7 @@ class JsonManipulator if (!self::$RECURSE_BLOCKS) { self::$RECURSE_BLOCKS = '(?:[^{}]*|\{(?:[^{}]*|\{(?:[^{}]*|\{(?:[^{}]*|\{[^{}]*\})*\})*\})*\})*'; self::$RECURSE_ARRAYS = '(?:[^\]]*|\[(?:[^\]]*|\[(?:[^\]]*|\[(?:[^\]]*|\[[^\]]*\])*\])*\])*\]|'.self::$RECURSE_BLOCKS.')*'; - self::$JSON_STRING = '"(?:\\\\["bfnrt/\\\\]|\\\\u[a-fA-F0-9]{4}|[^\0-\x09\x0a-\x1f\\\\"])*"'; + self::$JSON_STRING = '"(?:[^\0-\x09\x0a-\x1f\\\\"]+|\\\\["bfnrt/\\\\]|\\\\u[a-fA-F0-9]{4})*"'; self::$JSON_VALUE = '(?:[0-9.]+|null|true|false|'.self::$JSON_STRING.'|\['.self::$RECURSE_ARRAYS.'\]|\{'.self::$RECURSE_BLOCKS.'\})'; } @@ -139,12 +139,20 @@ class JsonManipulator } // main node content not match-able - $nodeRegex = '#("'.$mainNode.'":\s*\{)('.self::$RECURSE_BLOCKS.')(\})#s'; - if (!$this->pregMatch($nodeRegex, $this->contents, $match)) { - return false; + $nodeRegex = '{^(\s*\{\s*(?:'.self::$JSON_STRING.'\s*:\s*'.self::$JSON_VALUE.'\s*,\s*)*?)'. + '('.preg_quote(JsonFile::encode($mainNode)).'\s*:\s*\{)('.self::$RECURSE_BLOCKS.')(\})(.*)}s'; + try { + if (!$this->pregMatch($nodeRegex, $this->contents, $match)) { + return false; + } + } catch (\RuntimeException $e) { + if ($e->getCode() === PREG_BACKTRACK_LIMIT_ERROR) { + return false; + } + throw $e; } - $children = $match[2]; + $children = $match[3]; // invalid match due to un-regexable content, abort if (!@json_decode('{'.$children.'}')) { @@ -184,7 +192,7 @@ class JsonManipulator $children = $this->newline . $this->indent . $this->indent . JsonFile::encode($name).': '.$this->format($value, 1) . $children; } - $this->contents = preg_replace($nodeRegex, addcslashes('${1}'.$children.'$3', '\\'), $this->contents); + $this->contents = preg_replace($nodeRegex, addcslashes('${1}${2}'.$children.'${4}${5}', '\\'), $this->contents); return true; } @@ -199,15 +207,23 @@ class JsonManipulator } // no node content match-able - $nodeRegex = '#("'.$mainNode.'":\s*\{)('.self::$RECURSE_BLOCKS.')(\})#s'; - if (!$this->pregMatch($nodeRegex, $this->contents, $match)) { - return false; + $nodeRegex = '{^(\s*\{\s*(?:'.self::$JSON_STRING.'\s*:\s*'.self::$JSON_VALUE.'\s*,\s*)*?)'. + '('.preg_quote(JsonFile::encode($mainNode)).'\s*:\s*\{)('.self::$RECURSE_BLOCKS.')(\})(.*)}s'; + try { + if (!$this->pregMatch($nodeRegex, $this->contents, $match)) { + return false; + } + } catch (\RuntimeException $e) { + if ($e->getCode() === PREG_BACKTRACK_LIMIT_ERROR) { + return false; + } + throw $e; } - $children = $match[2]; + $children = $match[3]; // invalid match due to un-regexable content, abort - if (!@json_decode('{'.$children.'}')) { + if (!@json_decode('{'.$children.'}', true)) { return false; } @@ -245,7 +261,7 @@ class JsonManipulator // no child data left, $name was the only key in if (!trim($childrenClean)) { - $this->contents = preg_replace($nodeRegex, '$1'.$this->newline.$this->indent.'}', $this->contents); + $this->contents = preg_replace($nodeRegex, '$1$2'.$this->newline.$this->indent.'$4$5', $this->contents); // we have a subname, so we restore the rest of $name if ($subName !== null) { @@ -260,12 +276,12 @@ class JsonManipulator $that = $this; $this->contents = preg_replace_callback($nodeRegex, function ($matches) use ($that, $name, $subName, $childrenClean) { if ($subName !== null) { - $curVal = json_decode('{'.$matches[2].'}', true); + $curVal = json_decode('{'.$matches[3].'}', true); unset($curVal[$name][$subName]); $childrenClean = substr($that->format($curVal, 0), 1, -1); } - return $matches[1] . $childrenClean . $matches[3]; + return $matches[1] . $matches[2] . $childrenClean . $matches[4] . $matches[5]; }, $this->contents); return true; @@ -352,17 +368,17 @@ class JsonManipulator if ($count === false) { switch (preg_last_error()) { case PREG_NO_ERROR: - throw new \RuntimeException('Failed to execute regex: PREG_NO_ERROR'); + throw new \RuntimeException('Failed to execute regex: PREG_NO_ERROR', PREG_NO_ERROR); case PREG_INTERNAL_ERROR: - throw new \RuntimeException('Failed to execute regex: PREG_INTERNAL_ERROR'); + throw new \RuntimeException('Failed to execute regex: PREG_INTERNAL_ERROR', PREG_INTERNAL_ERROR); case PREG_BACKTRACK_LIMIT_ERROR: - throw new \RuntimeException('Failed to execute regex: PREG_BACKTRACK_LIMIT_ERROR'); + throw new \RuntimeException('Failed to execute regex: PREG_BACKTRACK_LIMIT_ERROR', PREG_BACKTRACK_LIMIT_ERROR); case PREG_RECURSION_LIMIT_ERROR: - throw new \RuntimeException('Failed to execute regex: PREG_RECURSION_LIMIT_ERROR'); + throw new \RuntimeException('Failed to execute regex: PREG_RECURSION_LIMIT_ERROR', PREG_RECURSION_LIMIT_ERROR); case PREG_BAD_UTF8_ERROR: - throw new \RuntimeException('Failed to execute regex: PREG_BAD_UTF8_ERROR'); + throw new \RuntimeException('Failed to execute regex: PREG_BAD_UTF8_ERROR', PREG_BAD_UTF8_ERROR); case PREG_BAD_UTF8_OFFSET_ERROR: - throw new \RuntimeException('Failed to execute regex: PREG_BAD_UTF8_OFFSET_ERROR'); + throw new \RuntimeException('Failed to execute regex: PREG_BAD_UTF8_OFFSET_ERROR', PREG_BAD_UTF8_OFFSET_ERROR); default: throw new \RuntimeException('Failed to execute regex: Unknown error'); } diff --git a/tests/Composer/Test/Json/JsonManipulatorTest.php b/tests/Composer/Test/Json/JsonManipulatorTest.php index fdbc90af2..d8e564346 100644 --- a/tests/Composer/Test/Json/JsonManipulatorTest.php +++ b/tests/Composer/Test/Json/JsonManipulatorTest.php @@ -576,18 +576,22 @@ class JsonManipulatorTest extends \PHPUnit_Framework_TestCase ), 'fails on deep arrays with borked texts' => array( '{ - "repositories": [{ - "package": { "bar": "ba[z" } - }] + "repositories": [ + { + "package": { "bar": "ba[z" } + } + ] }', 'bar', false ), 'fails on deep arrays with borked texts2' => array( '{ - "repositories": [{ - "package": { "bar": "ba]z" } - }] + "repositories": [ + { + "package": { "bar": "ba]z" } + } + ] }', 'bar', false @@ -603,6 +607,9 @@ class JsonManipulatorTest extends \PHPUnit_Framework_TestCase "package": { "require": { "this/should-not-end-up-in-root-require": "~2.0" + }, + "require-dev": { + "this/should-not-end-up-in-root-require-dev": "~2.0" } } } @@ -611,16 +618,23 @@ class JsonManipulatorTest extends \PHPUnit_Framework_TestCase "package/a": "*", "package/b": "*", "package/c": "*" + }, + "require-dev": { + "package/d": "*" } }'); $this->assertTrue($manipulator->removeSubNode('require', 'package/c')); + $this->assertTrue($manipulator->removeSubNode('require-dev', 'package/d')); $this->assertEquals('{ "repositories": [ { "package": { "require": { "this/should-not-end-up-in-root-require": "~2.0" + }, + "require-dev": { + "this/should-not-end-up-in-root-require-dev": "~2.0" } } } @@ -628,6 +642,60 @@ class JsonManipulatorTest extends \PHPUnit_Framework_TestCase "require": { "package/a": "*", "package/b": "*" + }, + "require-dev": { + } +} +', $manipulator->getContents()); + } + + public function testAddSubNodeInRequire() + { + $manipulator = new JsonManipulator('{ + "repositories": [ + { + "package": { + "require": { + "this/should-not-end-up-in-root-require": "~2.0" + }, + "require-dev": { + "this/should-not-end-up-in-root-require-dev": "~2.0" + } + } + } + ], + "require": { + "package/a": "*", + "package/b": "*" + }, + "require-dev": { + "package/d": "*" + } +}'); + + $this->assertTrue($manipulator->addSubNode('require', 'package/c', '*')); + $this->assertTrue($manipulator->addSubNode('require-dev', 'package/e', '*')); + $this->assertEquals('{ + "repositories": [ + { + "package": { + "require": { + "this/should-not-end-up-in-root-require": "~2.0" + }, + "require-dev": { + "this/should-not-end-up-in-root-require-dev": "~2.0" + } + } + } + ], + "require": { + "package/a": "*", + "package/b": "*", + "package/c": "*" + }, + "require-dev": { + "package/d": "*", + "package/e": "*" } } ', $manipulator->getContents());