Fix JsonManipulator handling of package links, fixes #1465

src/Composer/Json/JsonManipulator.php

@@ -49,37 +49,47 @@ class JsonManipulator
 
     public function addLink($type, $package, $constraint)
     {
-        // no link of that type yet
+        $data = @json_decode($this->contents, true);
-        if (!preg_match('#"'.$type.'":\s*\{#', $this->contents)) {
-            $this->addMainKey($type, array($package => $constraint));
 
-            return true;
+        // abort if the file is not parseable
+        if (null === $data) {
+            return false;
+        }
+
+        // no link of that type yet
+        if (!isset($data[$type])) {
+            return $this->addMainKey($type, array($package => $constraint));
         }
 
-        $linksRegex = '#("'.$type.'":\s*\{)([^}]+)(\})#s';
+        $regex = '{^(\s*\{\s*(?:'.self::$JSON_STRING.'\s*:\s*'.self::$JSON_VALUE.'\s*,\s*)*?)'.
-        if (!preg_match($linksRegex, $this->contents, $match)) {
+            '('.preg_quote(JsonFile::encode($type)).'\s*:\s*)('.self::$JSON_VALUE.')(.*)}s';
+        if (!preg_match($regex, $this->contents, $matches)) {
             return false;
         }
 
-        $links = $match[2];
+        $links = $matches[3];
-        $packageRegex = str_replace('/', '\\\\?/', preg_quote($package));
 
-        // link exists already
+        if (isset($data[$type][$package])) {
-        if (preg_match('{"'.$packageRegex.'"\s*:}i', $links)) {
+            // update existing link
-            $links = preg_replace('{"'.$packageRegex.'"(\s*:\s*)"[^"]+"}i', addcslashes(JsonFile::encode($package).'${1}"'.$constraint.'"', '\\'), $links);
+            $packageRegex = str_replace('/', '\\\\?/', preg_quote($package));
-        } elseif (preg_match('#[^\s](\s*)$#', $links, $match)) {
+            $links = preg_replace('{"'.$packageRegex.'"(\s*:\s*)'.self::$JSON_STRING.'}i', addcslashes(JsonFile::encode($package).'${1}"'.$constraint.'"', '\\'), $links);
-            // link missing but non empty links
-            $links = preg_replace(
-                '#'.$match[1].'$#',
-                addcslashes(',' . $this->newline . $this->indent . $this->indent . JsonFile::encode($package).': '.JsonFile::encode($constraint) . $match[1], '\\'),
-                $links
-            );
         } else {
-            // links empty
+            if (preg_match('#^\s*\{\s*\S+.*?(\s*\}\s*)$#', $links, $match)) {
-            $links = $this->newline . $this->indent . $this->indent . JsonFile::encode($package).': '.JsonFile::encode($constraint) . $links;
+                // link missing but non empty links
+                $links = preg_replace(
+                    '{'.preg_quote($match[1]).'$}',
+                    addcslashes(',' . $this->newline . $this->indent . $this->indent . JsonFile::encode($package).': '.JsonFile::encode($constraint) . $match[1], '\\'),
+                    $links
+                );
+            } else {
+                // links empty
+                $links = '{' . $this->newline .
+                    $this->indent . $this->indent . JsonFile::encode($package).': '.JsonFile::encode($constraint) . $this->newline .
+                    $this->indent . '}';
+            }
         }
 
-        $this->contents = preg_replace($linksRegex, addcslashes('${1}'.$links.'$3', '\\'), $this->contents);
+        $this->contents = $matches[1] . $matches[2] . $links . $matches[4];
 
         return true;
     }
@@ -127,7 +137,7 @@ class JsonManipulator
         $children = $match[2];
 
         // invalid match due to un-regexable content, abort
-        if (!json_decode('{'.$children.'}')) {
+        if (!@json_decode('{'.$children.'}')) {
             return false;
         }
 
@@ -190,7 +200,7 @@ class JsonManipulator
         $children = $match[2];
 
         // invalid match due to un-regexable content, abort
-        if (!json_decode('{'.$children.'}')) {
+        if (!@json_decode('{'.$children.'}')) {
             return false;
         }
 
@@ -256,7 +266,7 @@ class JsonManipulator
             '('.preg_quote(JsonFile::encode($key)).'\s*:\s*'.self::$JSON_VALUE.')(.*)}s';
         if (preg_match($regex, $this->contents, $matches)) {
             // invalid match due to un-regexable content, abort
-            if (!json_decode('{'.$matches[2].'}')) {
+            if (!@json_decode('{'.$matches[2].'}')) {
                 return false;
             }
 

tests/Composer/Test/Json/JsonManipulatorTest.php

@@ -126,6 +126,67 @@ class JsonManipulatorTest extends \PHPUnit_Framework_TestCase
         "vendor/baz": "qux"
     }
 }
+'
+            ),
+            array(
+                '{
+    "require": {
+        "foo": "bar"
+    },
+    "repositories": [{
+        "type": "package",
+        "package": {
+            "require": {
+                "foo": "bar"
+            }
+        }
+    }]
+}',
+                'require',
+                'foo',
+                'qux',
+                '{
+    "require": {
+        "foo": "qux"
+    },
+    "repositories": [{
+        "type": "package",
+        "package": {
+            "require": {
+                "foo": "bar"
+            }
+        }
+    }]
+}
+'
+            ),
+            array(
+                '{
+    "repositories": [{
+        "type": "package",
+        "package": {
+            "require": {
+                "foo": "bar"
+            }
+        }
+    }]
+}',
+                'require',
+                'foo',
+                'qux',
+                '{
+    "repositories": [{
+        "type": "package",
+        "package": {
+            "require": {
+                "foo": "bar"
+            }
+        }
+    }],
+    "require": {
+        "foo": "qux"
+    }
+}
 '
             ),
         );