Fix issues when reading recursive specs

.gitignore

@@ -1,5 +1,6 @@
 /vendor
 /composer.lock
 
+/node_modules
 
 /.php_cs.cache

.travis.yml

@@ -32,6 +32,7 @@ matrix:
 
 install: make install
 script:
+  - make lint
   - make test
   - if [[ $TRAVIS_PHP_VERSION = "7.3" || $TRAVIS_PHP_VERSION = "nightly" ]]; then true; else make check-style; fi
   - make coverage

Makefile

@@ -14,10 +14,17 @@ fix-style:
 
 install:
 	composer install --prefer-dist --no-interaction
+	yarn install
 
 test:
 	php $(PHPARGS) vendor/bin/phpunit $(TESTCASE)
 
+lint:
+	php $(PHPARGS) bin/php-openapi validate tests/spec/data/reference/playlist.json
+	php $(PHPARGS) bin/php-openapi validate tests/spec/data/recursion.json
+	node_modules/.bin/speccy lint tests/spec/data/reference/playlist.json
+	node_modules/.bin/speccy lint tests/spec/data/recursion.json
+
 # copy openapi3 json schema
 schemas/openapi-v3.0.json: vendor/oai/openapi-specification/schemas/v3.0/schema.json
 	cp $< $@
@@ -35,5 +42,5 @@ coverage: .php-openapi-covA .php-openapi-covB
 .php-openapi-covB:
 	grep -rhPo '^class \w+' src/spec/ | awk '{print $$2}' |grep -v '^Type$$' | sort > $@
 
-.PHONY: all check-style fix-style install test coverage
+.PHONY: all check-style fix-style install test lint coverage
 

bin/php-openapi

@@ -8,6 +8,8 @@
  * @license https://github.com/cebe/php-openapi/blob/master/LICENSE
  */
 
+use cebe\openapi\ReferenceContext;
+
 $composerAutoload = [
     __DIR__ . '/../vendor/autoload.php', // standalone with "composer install" run
     __DIR__ . '/../../../autoload.php',  // script is installed as a composer binary
@@ -97,12 +99,17 @@ foreach($argv as $k => $arg) {
 switch ($command) {
     case 'validate':
 
+        $errors = [];
+
         $openApi = read_input($inputFile, $inputFormat);
+        $referenceContext = new ReferenceContext($openApi, $inputFile ? realpath($inputFile) : '');
+        $referenceContext->throwException = false;
+        $openApi->resolveReferences($referenceContext);
 
         // Validate
 
         $openApi->validate();
-        $errors = $openApi->getErrors();
+        $errors = array_merge($errors, $openApi->getErrors());
 
         $validator = new JsonSchema\Validator;
         $validator->validate($openApi->getSerializableData(), (object)['$ref' => 'file://' . dirname(__DIR__) . '/schemas/openapi-v3.0.json']);
@@ -115,13 +122,20 @@ switch ($command) {
         if (!empty($errors)) {
             print_formatted("\BErrors found while reading the API Description:\C\n", STDERR);
             foreach ($errors as $error) {
-                fwrite(STDERR, "- $error\n");
+                if (($openPos = strpos($error, '[')) !== false && ($closePos = strpos($error, ']')) !== false && $openPos < $closePos) {
+                    $error = escape_formatted(substr($error, 0, $openPos + 1)) . '\Y'
+                           . escape_formatted(substr($error, $openPos + 1, $closePos - $openPos - 1)) . '\C'
+                           . escape_formatted(substr($error, $closePos));
+                } else {
+                    $error = escape_formatted($error);
+                }
+                print_formatted("- " . $error . "\n", STDERR);
             }
         }
         if (!$validator->isValid()) {
             print_formatted("\BOpenAPI v3.0 schema violations:\C\n", STDERR);
             foreach ($validator->getErrors() as $error) {
-                print_formatted(sprintf("- [\Y%s\C] %s\n", $error['property'], $error['message']), STDERR);
+                print_formatted(sprintf("- [\Y%s\C] %s\n", escape_formatted($error['property']), escape_formatted($error['message'])), STDERR);
             }
         }
         exit(2);
@@ -130,6 +144,7 @@ switch ($command) {
     case 'convert':
 
         $openApi = read_input($inputFile, $inputFormat);
+        $openApi->resolveReferences();
 
         if ($outputFile === null) {
             if ($outputFormat === null) {
@@ -202,11 +217,12 @@ function read_input($inputFile, $inputFormat)
                 }
             }
             if ($inputFormat === 'json') {
-                $openApi = \cebe\openapi\Reader::readFromJsonFile(realpath($inputFile));
+                $openApi = \cebe\openapi\Reader::readFromJsonFile(realpath($inputFile), \cebe\openapi\spec\OpenApi::class, false);
             } else {
-                $openApi = \cebe\openapi\Reader::readFromYamlFile(realpath($inputFile));
+                $openApi = \cebe\openapi\Reader::readFromYamlFile(realpath($inputFile), \cebe\openapi\spec\OpenApi::class, false);
             }
         }
+        $openApi->setDocumentContext($openApi, new \cebe\openapi\json\JsonPointer(''));
     } catch (Symfony\Component\Yaml\Exception\ParseException $e) {
         error($e->getMessage());
         exit(1);
@@ -269,7 +285,7 @@ EOF
  * @return void
  */
 function error($message, $callback = null) {
-    print_formatted("\B\RError\C: " . $message . "\n", STDERR);
+    print_formatted("\B\RError\C: " . escape_formatted($message) . "\n", STDERR);
     if (is_callable($callback)) {
         call_user_func($callback);
     }
@@ -283,5 +299,10 @@ function print_formatted($string, $stream) {
             '\\R' => "\033[31m", // green
             '\\B' => "\033[1m", // bold
             '\\C' => "\033[0m", // clear
+            '\\\\' => '\\',
     ]));
 }
+
+function escape_formatted($string) {
+    return strtr($string, ['\\' => '\\\\']);
+}

package.json

@@ -0,0 +1,5 @@
+{
+  "dependencies": {
+    "speccy": "^0.11.0"
+  }
+}

src/ReferenceContext.php

@@ -14,6 +14,11 @@
  */
 class ReferenceContext
 {
+    /**
+     * @var bool whether to throw UnresolvableReferenceException in case a reference can not
+     * be resolved. If `false` errors are added to the Reference Objects error list instead.
+     */
+    public $throwException = true;
     /**
      * @var SpecObjectInterface
      */

src/SpecBaseObject.php

@@ -10,6 +10,7 @@
 use cebe\openapi\exceptions\TypeErrorException;
 use cebe\openapi\exceptions\UnknownPropertyException;
 use cebe\openapi\json\JsonPointer;
+use cebe\openapi\json\JsonReference;
 use cebe\openapi\spec\Reference;
 use cebe\openapi\spec\Type;
 
@@ -23,6 +24,7 @@ abstract class SpecBaseObject implements SpecObjectInterface, DocumentContextInt
 {
     private $_properties = [];
     private $_errors = [];
+    private $_recursing = false;
 
     private $_baseDocument;
     private $_jsonPointer;
@@ -145,6 +147,12 @@ private function instantiate($type, $data)
      */
     public function getSerializableData()
     {
+        if ($this->_recursing) {
+            // return a reference
+            return (object) ['$ref' => JsonReference::createFromUri('', $this->getDocumentPosition())->getReference()];
+        }
+        $this->_recursing = true;
+
         $data = $this->_properties;
         foreach ($data as $k => $v) {
             if ($v instanceof SpecObjectInterface) {
@@ -165,6 +173,9 @@ public function getSerializableData()
                 }
             }
         }
+
+        $this->_recursing = false;
+
         return (object) $data;
     }
 
@@ -175,19 +186,36 @@ public function getSerializableData()
      */
     public function validate(): bool
     {
+        // avoid recursion to get stuck in a loop
+        if ($this->_recursing) {
+            return true;
+        }
+        $this->_recursing = true;
+        $valid = true;
         foreach ($this->_properties as $v) {
             if ($v instanceof SpecObjectInterface) {
-                $v->validate();
+                if (!$v->validate()) {
+                    $valid = false;
+                }
             } elseif (is_array($v)) {
                 foreach ($v as $item) {
                     if ($item instanceof SpecObjectInterface) {
-                        $item->validate();
+                        if (!$item->validate()) {
+                            $valid = false;
+                        }
                     }
                 }
             }
         }
+        $this->_recursing = false;
+
         $this->performValidation();
-        return \count($this->getErrors()) === 0;
+
+        if (!empty($this->_errors)) {
+            $valid = false;
+        }
+
+        return $valid;
     }
 
     /**
@@ -196,6 +224,12 @@ public function validate(): bool
      */
     public function getErrors(): array
     {
+        // avoid recursion to get stuck in a loop
+        if ($this->_recursing) {
+            return [];
+        }
+        $this->_recursing = true;
+
         if (($pos = $this->getDocumentPosition()) !== null) {
             $errors = [
                 array_map(function($e) use ($pos) {
@@ -216,6 +250,9 @@ public function getErrors(): array
                 }
             }
         }
+
+        $this->_recursing = false;
+
         return array_merge(...$errors);
     }
 

src/exceptions/UnresolvableReferenceException.php

@@ -7,10 +7,16 @@
 
 namespace cebe\openapi\exceptions;
 
+use cebe\openapi\json\JsonPointer;
+
 /**
- *
- *
+ * This exception is thrown on attempt to resolve a reference which points to a non-existing target.
  */
 class UnresolvableReferenceException extends \Exception
 {
+    /**
+     * @var JsonPointer|null may contain context information in form of a JSON pointer to the position
+     * of the broken reference in the document.
+     */
+    public $context;
 }

src/spec/Reference.php

@@ -177,7 +177,15 @@ public function resolve(ReferenceContext $context = null)
 
             return $referencedObject;
         } catch (NonexistentJsonPointerReferenceException $e) {
-            throw new UnresolvableReferenceException("Failed to resolve Reference '$this->_ref' to $this->_to Object: " . $e->getMessage(), 0, $e);
+            $message = "Failed to resolve Reference '$this->_ref' to $this->_to Object: " . $e->getMessage();
+            if ($context->throwException) {
+                $exception = new UnresolvableReferenceException($message, 0, $e);
+                $exception->context = $this->getDocumentPosition();
+                throw $exception;
+            }
+            $this->_errors[] = $message;
+            $this->_jsonReference = null;
+            return $this;
         }
     }
 
@@ -195,11 +203,13 @@ private function fetchReferencedFile($uri)
                 return Yaml::parse($content);
             }
         } catch (\Throwable $e) {
-            throw new UnresolvableReferenceException(
+            $exception = new UnresolvableReferenceException(
                 "Failed to resolve Reference '$this->_ref' to $this->_to Object: " . $e->getMessage(),
                 $e->getCode(),
                 $e
             );
+            $exception->context = $this->getDocumentPosition();
+            throw $exception;
         }
     }
 

tests/spec/data/recursion.json

@@ -0,0 +1,49 @@
+{
+  "openapi": "3.0.1",
+  "info": {
+    "title": "test",
+    "version": "1.0",
+    "contact": {
+      "email": "[email protected]"
+    }
+  },
+  "paths": {
+    "/product": {
+      "description": "default"
+    }
+  },
+  "tags": [
+    {"name": "Products"}
+  ],
+  "components": {
+    "schemas": {
+      "Product": {
+        "title": "Product",
+        "description": "A product.",
+        "type": "object",
+        "properties": {
+          "isBundle": {
+            "description": "Whether this product is a bundle of multiple products.",
+            "type": "boolean"
+          },
+          "bundleProducts": {
+            "description": "If `isBundle` is `true`, `bundleProducts` will contain an array of products.",
+            "oneOf": [
+              {
+                "title": "Market Product",
+                "type": "array",
+                "items": {
+                  "$ref": "#/components/schemas/Product"
+                }
+              },
+              {
+                "description": "If this is not a bundle of products, `bundleProducts` will be `false`.",
+                "type": "boolean"
+              }
+            ]
+          }
+        }
+      }
+    }
+  }
+}