APIDevTools / json-schema-ref-parser

Parse, Resolve, and Dereference JSON Schema $ref pointers in Node and browsers
https://apitools.dev/json-schema-ref-parser
MIT License
955 stars 227 forks source link

JSON Pointer parsing #296

Closed char0n closed 7 months ago

char0n commented 1 year ago

Hi,

I've noticed that implementation of JSON Pointer parsing is not compliant with RFC 6901, nor it's safe (throws error). The issue is with decodeURIComponent, which trows error in certain character sequences which are valid JSON Pointers.

Steps to Reproduce

const Pointer = require('@apidevtools/json-schema-ref-parser/lib/pointer.js');

Pointer.parse('#/c%d');

/c%d - is valid JSON Pointer according to the RFC 6901 (section 5), which has a nice example suitable for creating a test suite for the implementation:

image

Remediation

Remediation might be introduced by safe variant of decodeURIComponent that will look as follows:

const safeDecodeURIComponent = (encodedURIComponent: string): string => {
  try {
    return decodeURIComponent(encodedURIComponent);
  } catch {
    return encodedURIComponent;
  }
};

This safe version at least gives us chance to evaluate the JSON Pointer against the structure, even though not all character sequences might get decoded.

char0n commented 1 year ago

It's also worth considering if encodeURIComponent and decodeURIComponent should be used at all as the specification says the following:

Evaluation of each reference token begins by decoding any escaped character sequence. This is performed by first transforming any occurrence of the sequence '~1' to '/', and then transforming any occurrence of the sequence '~0' to '~'.

This is little ambiguous as it's first tells us to that it begins by decoding any escaped character sequence but then it enumerates exactly two decoding operations that needs to be performed.

jonluca commented 7 months ago

Agreed, I think we do a bit too much string encoding in general. Would be better to fallback to explicit implementations of these schemas. For now I've implemented it in v11.2.4

char0n commented 7 months ago

Thanks a lot!