schemaloader: Fix expandRefs not to crash if $ref'ed schema also has …

…another $ref to self internally

If a $ref'ed schema contains another $ref referring to somewhere inside
of it, and if that second $ref lives in an array container, expandRefs
was wrongly processing that second link, and, after emitting a warning
was invoking expandSchema(schema=undefined) which was further crashing
on trying to access undefined.type.

The following example demonstrates this situation:

    ---- 8< ---- (schema.json)
    {
      "$ref": "main.json"
    }

    ---- 8< ---- (main.json)
    {
      "$schema": "http://json-schema.org/draft-07/schema#",
      "type": "object",
      "definitions": {
        "tcpv4port": {
          "type": "integer"
        }
      },
      "properties": {
        "port": {
          "anyOf": [
            {
              "$ref": "#/definitions/tcpv4port"
            }
          ]
        }
      }
    }

SchemaLoader.load initially handles this schema ok, but later
SchemaLoader.expandSchema, which is used by JSONEditor during form
building, is crashing when invoked with schema of "port":

    Uncaught (in promise) TypeError: Cannot read properties of undefined (reading 'type')
        at eval (schemaloader.js:219:1)
        at Array.forEach (<anonymous>)
        at SchemaLoader.expandSchema (schemaloader.js:218:1)
        at SchemaLoader.expandRefs (schemaloader.js:186:1)
        at SchemaLoader.expandSchema (schemaloader.js:232:1)
        at eval (schemaloader.js:68:1)
        at Array.forEach (<anonymous>)
        at SchemaLoader.anyOf (schemaloader.js:67:1)
        at eval (schemaloader.js:220:1)
        at Array.forEach (<anonymous>)
    eval	@	schemaloader.js:219
    expandSchema	@	schemaloader.js:218
    expandRefs	@	schemaloader.js:186
    expandSchema	@	schemaloader.js:232
    eval	@	schemaloader.js:68
    anyOf	@	schemaloader.js:67
    eval	@	schemaloader.js:220
    expandSchema	@	schemaloader.js:218
    expandSchema	@	core.js:67
    getEditorClass	@	core.js:211
    addObjectProperty	@	object.js:1014
    eval	@	object.js:423
    preBuild	@	object.js:422
    _callee$	@	core.js:81
    tryCatch	@	core.js:2
    eval	@	core.js:2
    eval	@	core.js:2
    asyncGeneratorStep	@	core.js:2
    _next	@	core.js:2
    Promise.then (async)
    asyncGeneratorStep	@	core.js:2
    _next	@	core.js:2
    eval	@	core.js:2
    eval	@	core.js:2
    load	@	core.js:102
    JSONEditor	@	core.js:61
    (anonymous)	@	k.html:98

The problem here is that upon loading SchemaLoader.load returns schema
with the following part for "port":

    { anyOf: [ { $ref: '#/counter/2' } ] }

which expandRefs will need to further dereference. expandRefs, when run
on that part, internally reconstructs ref for that $ref as

    ref = 'main.json#/definitions/tcpv4port'

because after loading

    loader.refs['#/counter/2'] = { fetchUrl: 'main.json', $ref: '#/definitions/tcpv4port' }

and further tries to lookup that ref in loader.refs registry.

The first bug is that loader.refs registry needs to be looked up only
with base part of the ref, because that's how loader.refs is
initialized:

    https://github.com/json-editor/json-editor/blob/06f0117aa166f3fca095f4554fba22c83f1f38cb/src/schemaloader.js#L375-L378
    https://github.com/json-editor/json-editor/blob/06f0117aa166f3fca095f4554fba22c83f1f38cb/src/schemaloader.js#L283-L292
    https://github.com/json-editor/json-editor/blob/06f0117aa166f3fca095f4554fba22c83f1f38cb/src/schemaloader.js#L465

The second bug is that the decision whether to dereference JSON Pointer
part of the URI was being taken based only on the original form of the
ref. In this case it was leading to main.json#/definitions/tcpv4port not
being followed to /definitions/tcpv4port even after schema of main.json
becomes correctly looked up with change of ref to refBase.

-> Fix both bugs:

1) lookup loader.refs only with base part of a ref;
2) after retriving looked up schema, also dereference it via
   JSON-Pointer, if that part is present in looked up ref.

Fixes: #1384

src/schemaloader.js

@@ -24,6 +24,7 @@ export class SchemaLoader {
      *    "fully/realized/path/to/schema.json": { ... }
      *    "mylocalschema.json": { ... }
      *  }
+     *  NOTE: keys are base part of original $ref without fragment
      */
     this.refs = this.options.refs || {}
 
@@ -154,7 +155,7 @@ export class SchemaLoader {
     // exemple #/counter/1#/definition/address +
     // [1] -> /counter/1
     // [2] -> /definition/address
-    const refWithPointerSplit = _schema.$ref.split('#')
+    let refWithPointerSplit = _schema.$ref.split('#')
     // If local ref
     if (refWithPointerSplit.length === 2 && !this.refs_with_info[_schema.$ref]) {
       const sub = this.expandRecursivePointer(this.schema, refWithPointerSplit[1])
@@ -171,20 +172,30 @@ export class SchemaLoader {
       : ''
     const ref = this._getRef(fetchUrl, refObj)
 
-    if (!this.refs[ref]) { /* if reference not found */
+    refWithPointerSplit = ref.split('#')
+    let refBase = ref
+    let refPointer = null
+    if (refWithPointerSplit.length > 1) {
+      refBase = refWithPointerSplit[0]
+      refPointer = refWithPointerSplit[1]
+    }
+
+    if (!this.refs[refBase]) { /* if reference not found */
       // eslint-disable-next-line no-console
       console.warn(`reference:'${ref}' not found!`)
-    } else if (recurseAllOf && hasOwnProperty(this.refs[ref], 'allOf')) {
+    }
+
+    let refSchema = this.refs[refBase]
+    if (refPointer !== null) {
+      refSchema = this.expandRecursivePointer(refSchema, refPointer)
+    }
+    if (recurseAllOf && hasOwnProperty(refSchema, 'allOf')) {
       const allOf = this.refs[ref].allOf
       Object.keys(allOf).forEach(key => {
         allOf[key] = this.expandRefs(allOf[key], true)
       })
     }
-    if (refWithPointerSplit.length > 2) {
-      return this.extendSchemas(_schema, this.expandSchema(this.expandRecursivePointer(this.refs[ref], refWithPointerSplit[2])))
-    } else {
-      return this.extendSchemas(_schema, this.expandSchema(this.refs[ref]))
-    }
+    return this.extendSchemas(_schema, this.expandSchema(refSchema))
   }
 
   /**

tests/unit/schemaloader.spec.js

@@ -429,6 +429,56 @@ describe('SchemaLoader', () => {
       expect(loader.refs['/main.json'].properties.timeout).toEqual({ default: null })
       server.restore()
     })
+
+    it('should not crash on expanding result with self reference', async () => {
+      // https://github.com/json-editor/json-editor/issues/1384
+      const main = {
+        $schema: 'http://json-schema.org/draft-07/schema#',
+        type: 'object',
+        definitions: {
+          tcpv4port: {
+            type: 'integer'
+          }
+        },
+        properties: {
+          port: {
+            allOf: [
+              {
+                $ref: '#/definitions/tcpv4port'
+              }
+            ]
+          }
+        }
+      }
+      const schema = {
+        $ref: '/main.json'
+      }
+
+      const server = createFakeServer()
+      server.autoRespond = true
+      window.XMLHttpRequest = server.xhr
+      server.respondWith('/main.json', [
+        200,
+        { 'Content-Type': 'application/json' },
+        JSON.stringify(main)
+      ])
+      fetchUrl =
+        document.location.origin + document.location.pathname.toString()
+
+      loader = new SchemaLoader({ ajax: true })
+      fileBase = loader._getFileBase(document.location.toString())
+
+      let loadedSchema = await loader.load(
+        schema,
+        fetchUrl,
+        fileBase
+      )
+
+      const port = loader.expandSchema(loadedSchema.properties.port)
+      expect(port.allOf).toBe(undefined)   // merged into parent
+      expect(port.$ref).toBe(undefined)    // $ref expaned
+      expect(port.type).toEqual('integer')
+    })
   })
 
   describe('when resolving undeclared URN $ref', () => {