fix: allow $ref pointers to point to a null value

[erunion]

I've run into a curious issue where the following schema throws a MissingPointerError exception:

components:
  examples:
    product:
      value:
        data:
          admission:
            $ref: "#/components/examples/product/value/data/pas"
          pas: null

I'm having a hard time understanding why the Pointer class is running this substring lookup for if a token does exist but it's null, but because the token we're looking for is the last in the list in this block it never ends up running the for loop below this line to look for a substring -- resulting in it falling through to throw a MissingPointerError.

json-schema-ref-parser/lib/pointer.ts

Lines 108 to 122 in 5303da9

	if (this.value[token] === undefined || (this.value[token] === null && i === tokens.length - 1)) {
	// one final case is if the entry itself includes slashes, and was parsed out as a token - we can join the remaining tokens and try again
	let didFindSubstringSlashMatch = false;
	for (let j = tokens.length - 1; j > i; j--) {
	const joinedToken = tokens.slice(i, j + 1).join("/");
	if (this.value[joinedToken] !== undefined) {
	this.value = this.value[joinedToken];
	i = j;
	didFindSubstringSlashMatch = true;
	break;
	}
	}
	if (didFindSubstringSlashMatch) {
	continue;
	}

I've managed to fix this by doing a secondary "does this token exist and is it null? ok return that." check and that has resolved this problem in the dereferencer but I also don't know if this is the best way to fix this, or if it will have downstream effects (no tests break with it), or would be considered as a breaking change.

Changing line 108 to only check this.value[token] === undefined also fixes this issue but even though no tests break with that I don't know if it's the right fix.

Looking for guidance here!

[coveralls]

Pull Request Test Coverage Report for Build 13576865917

Details

• 12 of 14 (85.71%) changed or added relevant lines in 2 files are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage decreased (-0.1%) to 92.654%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
lib/ref.ts	7	9	77.78%

Totals
Change from base Build 13555619823:	-0.1%
Covered Lines:	1634
Relevant Lines:	1734

---

💛 - Coveralls

[jonluca]

This is a great point.

I think we use this.value = null as an explicit check, and don't support the value actually being null.

Maybe if we change null into a Symbol('null') and check for that on lines 102 and 124, that's a better solution?

[jonluca]

Actually wait, is that a valid schema? It might not be a valid schem. In openapi 3.x, to make a field nullable you set the schema to be nullable: true. Does it resolve correctly if you change it to not just be a null value?

[erunion]

Yeah that is a valid schema. In this case the thing isn't formally nullable, but it's a null example value. If I were designing the actual schema for pas it'd need to have nullable to make this real yeah.

As far as resolving the resolver is able to handle this case without my fix if pas is null as it just leaves it alone, and changing pas to "dog" it also splices it into the schema as a normal $ref as well.

It's just when this value we want to splice in is null that it breaks down.

MissingPointerError: Missing $ref pointer "#/components/examples/product/value/data/pas". Token "pas" does not exist.
 ❯ Pointer.resolve lib/pointer.ts:140:15
    138|         const parentPath = pathFromRoot?.replace(path, "");
    139| 
    140|         throw new MissingPointerError(token, decodeURI(this.originalPath), targetRef, targetFound, parentPath);
       |               ^
    141|       } else {
    142|         this.value = this.value[token];
 ❯ $Ref.resolve lib/ref.ts:122:22
 ❯ $Refs._resolve lib/refs.ts:156:17
 ❯ dereference$Ref lib/dereference.ts:236:25
 ❯ crawl lib/dereference.ts:112:28
 ❯ crawl lib/dereference.ts:155:30
 ❯ crawl lib/dereference.ts:155:30
 ❯ crawl lib/dereference.ts:155:30
 ❯ crawl lib/dereference.ts:155:30
 ❯ crawl lib/dereference.ts:155:30

[jonluca]

Interesting, that makes sense.

If we swap out the null checks with a null symbol just for internal tracking that might work. I like your solution for now, I'm fairly open to merging this in. I'll think about it but it feels low risk

[erunion]

Doing it as Symbol('null') might be tough because when fed through JSON.stringify() it becomes undefined and consumers will have to recursively parse their schemas to convert them to a true null value.

[erunion]

This Symbol solution works but definitely feels a little on the messy side:

diff --git a/lib/pointer.ts b/lib/pointer.ts
index f1660a5..d7fa143 100644
--- a/lib/pointer.ts
+++ b/lib/pointer.ts
@@ -125,7 +125,7 @@ class Pointer<S extends object = JSONSchema, O extends ParserOptions<S> = Parser
         // actually instead pointing to an existing `null` value then we should use that
         // `null` value.
         if (token in this.value && this.value[token] === null) {
-          this.value = null;
+          this.value = Symbol('null');
           continue;
         }
 
diff --git a/lib/ref.ts b/lib/ref.ts
index 0ad40b4..f84f8f1 100644
--- a/lib/ref.ts
+++ b/lib/ref.ts
@@ -119,7 +119,12 @@ class $Ref<S extends object = JSONSchema, O extends ParserOptions<S> = ParserOpt
   resolve(path: string, options?: O, friendlyPath?: string, pathFromRoot?: string) {
     const pointer = new Pointer<S, O>(this, path, friendlyPath);
     try {
-      return pointer.resolve(this.value, options, pathFromRoot);
+      const resolved = pointer.resolve(this.value, options, pathFromRoot);
+      if (typeof resolved.value === 'symbol' && resolved.value.toString() === 'Symbol(null)') {
+        resolved.value = null;
+      }
+
+      return resolved;
     } catch (err: any) {
       if (!options || !options.continueOnError || !isHandledError(err)) {
         throw err;
@@ -148,6 +153,9 @@ class $Ref<S extends object = JSONSchema, O extends ParserOptions<S> = ParserOpt
   set(path: string, value: any) {
     const pointer = new Pointer(this, path);
     this.value = pointer.set(this.value, value);
+    if (typeof this.value === 'symbol' && this.value.toString() === 'Symbol(null)') {
+      this.value = null;
+    }
   }
 
   /**

[jonluca]

For this one I was thinking of having an explicit symbol like

export const nullSymbol = Symbol('null')

and then checking

if (resolved.value === nullSymbol) {
    resolved.value = null;
}

[erunion]

🙃 of course, time for afternoon coffee apparently. I've just pushed that refactor up!

[jonluca]

Awesome, this is great. Merged in, thanks for the great PR!

[github-actions]

🎉 This PR is included in version 11.9.3 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀