From 3948abb06a4dfca0864bc87783857ec73aa5cd72 Mon Sep 17 00:00:00 2001
From: Erayd <steve@erayd.net>
Date: Fri, 23 Jun 2017 23:42:48 +1200
Subject: [PATCH] [BUGFIX] Ignore $ref siblings & abort on infinite-loop
 references (#437)

* Add tests for $ref enforcement

* Overriding schema properties from $ref is not allowed

From the spec [1]:
    An object schema with a "$ref" property MUST be interpreted as a
    "$ref" reference.  The value of the "$ref" property MUST be a URI
    Reference.  Resolved against the current URI base, it identifies the
    URI of a schema to use.  All other properties in a "$ref" object MUST
    be ignored.

[1] https://tools.ietf.org/html/draft-wright-json-schema-01#section-8

* Add test for infinite-loop circular reference

* Return dereferenced schema directly and check for infinite-loops

Schemas containing $ref MUST ignore all other properties, so just return
the reference target directly.

Because some circular references may result in an infinite loop, keep
track of the objects that have been dereferenced during the current call
to SchemaStorage and abort if the same object is encountered more than
once.

* Code style fix (remove blank line)
---
 src/JsonSchema/SchemaStorage.php | 20 +++++---
 tests/RefTest.php                | 78 ++++++++++++++++++++++++++++++++
 tests/SchemaStorageTest.php      |  2 +-
 3 files changed, 92 insertions(+), 8 deletions(-)
 create mode 100644 tests/RefTest.php

diff --git a/src/JsonSchema/SchemaStorage.php b/src/JsonSchema/SchemaStorage.php
index 9a1caeb4..de2a1cb2 100644
--- a/src/JsonSchema/SchemaStorage.php
+++ b/src/JsonSchema/SchemaStorage.php
@@ -94,7 +94,7 @@ public function getSchema($id)
     /**
      * {@inheritdoc}
      */
-    public function resolveRef($ref)
+    public function resolveRef($ref, $resolveStack = array())
     {
         $jsonPointer = new JsonPointer($ref);
 
@@ -111,9 +111,9 @@ public function resolveRef($ref)
         $refSchema = $this->getSchema($fileName);
         foreach ($jsonPointer->getPropertyPaths() as $path) {
             if (is_object($refSchema) && property_exists($refSchema, $path)) {
-                $refSchema = $this->resolveRefSchema($refSchema->{$path});
+                $refSchema = $this->resolveRefSchema($refSchema->{$path}, $resolveStack);
             } elseif (is_array($refSchema) && array_key_exists($path, $refSchema)) {
-                $refSchema = $this->resolveRefSchema($refSchema[$path]);
+                $refSchema = $this->resolveRefSchema($refSchema[$path], $resolveStack);
             } else {
                 throw new UnresolvableJsonPointerException(sprintf(
                     'File: %s is found, but could not resolve fragment: %s',
@@ -129,12 +129,18 @@ public function resolveRef($ref)
     /**
      * {@inheritdoc}
      */
-    public function resolveRefSchema($refSchema)
+    public function resolveRefSchema($refSchema, $resolveStack = array())
     {
         if (is_object($refSchema) && property_exists($refSchema, '$ref') && is_string($refSchema->{'$ref'})) {
-            $newSchema = $this->resolveRef($refSchema->{'$ref'});
-            $refSchema = (object) (get_object_vars($refSchema) + get_object_vars($newSchema));
-            unset($refSchema->{'$ref'});
+            if (in_array($refSchema, $resolveStack, true)) {
+                throw new UnresolvableJsonPointerException(sprintf(
+                    'Dereferencing a pointer to %s results in an infinite loop',
+                    $refSchema->{'$ref'}
+                ));
+            }
+            $resolveStack[] = $refSchema;
+
+            return $this->resolveRef($refSchema->{'$ref'}, $resolveStack);
         }
 
         return $refSchema;
diff --git a/tests/RefTest.php b/tests/RefTest.php
new file mode 100644
index 00000000..b67852c1
--- /dev/null
+++ b/tests/RefTest.php
@@ -0,0 +1,78 @@
+<?php
+
+/*
+ * This file is part of the JsonSchema package.
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace JsonSchema\Tests;
+
+use JsonSchema\Validator;
+
+class RefTest extends \PHPUnit_Framework_TestCase
+{
+    public function dataRefIgnoresSiblings()
+    {
+        return array(
+            // #0 check that $ref is resolved and the instance is validated against
+            // the referenced schema
+            array(
+                '{
+                    "definitions":{"test": {"type": "integer"}},
+                    "properties": {
+                        "propertyOne": {"$ref": "#/definitions/test"}
+                    }
+                }',
+                '{"propertyOne": "not an integer"}',
+                false
+            ),
+            // #1 check that sibling properties of $ref are ignored during validation
+            array(
+                '{
+                    "definitions":{
+                        "test": {"type": "integer"}
+                    },
+                    "properties": {
+                        "propertyOne": {
+                            "$ref": "#/definitions/test",
+                            "maximum": 5
+                        }
+                    }
+                }',
+                '{"propertyOne": 10}',
+                true
+            ),
+            // #2 infinite-loop / unresolveable circular reference
+            array(
+                '{
+                    "definitions": {
+                        "test1": {"$ref": "#/definitions/test2"},
+                        "test2": {"$ref": "#/definitions/test1"}
+                    },
+                    "properties": {"propertyOne": {"$ref": "#/definitions/test1"}}
+                }',
+                '{"propertyOne": 5}',
+                true,
+                '\JsonSchema\Exception\UnresolvableJsonPointerException'
+            )
+        );
+    }
+
+    /** @dataProvider dataRefIgnoresSiblings */
+    public function testRefIgnoresSiblings($schema, $document, $isValid, $exception = null)
+    {
+        $document = json_decode($document);
+        $schema = json_decode($schema);
+
+        $v = new Validator();
+        if ($exception) {
+            $this->setExpectedException($exception);
+        }
+
+        $v->validate($document, $schema);
+
+        $this->assertEquals($isValid, $v->isValid());
+    }
+}
diff --git a/tests/SchemaStorageTest.php b/tests/SchemaStorageTest.php
index 0e440ac8..9fd27bae 100644
--- a/tests/SchemaStorageTest.php
+++ b/tests/SchemaStorageTest.php
@@ -79,7 +79,7 @@ public function testSchemaWithLocalAndExternalReferencesWithCircularReference()
         );
 
         // local ref with overriding
-        $this->assertNotEquals(
+        $this->assertEquals(
             $schemaStorage->resolveRef("$mainSchemaPath#/definitions/house/additionalProperties"),
             $schemaStorage->resolveRef("$mainSchemaPath#/properties/house/additionalProperties")
         );