fix: properly validate $ref value escaping (#1760)

* add tests for $ref encoding

* use percent encoding and decoding for $ref values

* add validator for non-unreserved RFC3986 characters in $ref values

* unescape $ref values before checking for existence of the path

* don't use full unescaper for existential path testing

* use querystring-browser for consistent cross-platform escaping

* touch up error messaging

* update tests to match actual error messaging

Author: shockey

package.json

@@ -51,6 +51,7 @@
     "json-beautify": "^1.0.1",
     "json-refs": "^3.0.4",
     "prop-types": "15.6.0",
+    "querystring-browser": "^1.0.4",
     "react": "^15.6.2",
     "react-ace": "^4.1.6",
     "react-addons-css-transition-group": "^15.4.2",

src/plugins/refs-util.js

@@ -1,3 +1,5 @@
+import qs from "querystring-browser"
+
 /**
  * Unescapes a JSON pointer.
  * @api public
@@ -6,13 +8,13 @@ export function unescapeJsonPointerToken(token) {
   if (typeof token !== "string") {
     return token
   }
-  return token.replace(/~1/g, "/").replace(/~0/g, "~")
+  return qs.unescape(token.replace(/~1/g, "/").replace(/~0/g, "~"))
 }
 
 /**
  * Escapes a JSON pointer.
  * @api public
  */
 export function escapeJsonPointerToken(token) {
-  return token.replace(/~/g, "~0").replace(/\//g, "~1")
+  return qs.escape(token.replace(/~/g, "~0").replace(/\//g, "~1"))
 }

src/plugins/validate-semantic/validators/2and3/refs.js

@@ -1,5 +1,6 @@
 import get from "lodash/get"
 import { escapeJsonPointerToken } from "../../../refs-util"
+import qs from "querystring-browser"
 import { pathFromPtr } from "json-refs"
 
 export const validate2And3RefHasNoSiblings = () => system => {
@@ -89,7 +90,7 @@ export const validate2And3RefPointersExist = () => (system) => {
       const value = node.node
       if(typeof value === "string" && value[0] === "#") {
         // if pointer starts with "#", it is a local ref
-        const path = pathFromPtr(value)
+        const path = pathFromPtr(qs.unescape(value))
 
         if(json.getIn(path) === undefined) {
           errors.push({
@@ -104,3 +105,35 @@ export const validate2And3RefPointersExist = () => (system) => {
     return errors
   })
 }
+
+// from RFC3986: https://tools.ietf.org/html/rfc3986#section-2.2
+// plus "%", since it is needed for encoding.
+const RFC3986_UNRESERVED_CHARACTERS = /[A-Z|a-z|0-9|\-|_|\.|~|%]/g
+
+export const validate2And3RefPointersAreProperlyEscaped = () => (system) => {
+  return system.validateSelectors.all$refs()
+  .then((refs) => {
+    const errors = []
+
+    refs.forEach((node) => {
+      const value = node.node
+      const hashIndex = value.indexOf("#")
+      const fragment = hashIndex > -1 ? value.slice(hashIndex + 1) : null
+      if(typeof fragment === "string") {
+        const rawPath = fragment.split("/")
+        const hasReservedChars = rawPath
+          .some(p => p.replace(RFC3986_UNRESERVED_CHARACTERS, "").length > 0)
+
+        if(hasReservedChars) {
+          errors.push({
+            path: [...node.path.slice(0, -1), "$ref"],
+            message: "$ref values must be RFC3986-compliant percent-encoded URIs",
+            level: "error"
+          })
+        }
+      }
+    })
+
+    return errors
+  })
+}

test/plugins/validate-semantic/2and3/refs.js

@@ -340,6 +340,154 @@ describe("validation plugin - semantic - 2and3 refs", function() {
         expect(allSemanticErrors).toEqual([])
       })
     })
+    it("should return an error when a JSON pointer uses incorrect percent-encoding in Swagger 2", () => {
+      const spec = {
+        "swagger": "2.0",
+        "paths": {
+          "/foo": {
+            "get": {
+              "responses": {
+                "200": {
+                  "description": "Success",
+                  "schema": {
+                    "$ref": "#/definitions/foo bar"
+                  }
+                }
+              }
+            }
+          }
+        },
+        "definitions": {
+          "foo bar": {
+            "type": "string"
+          }
+        }
+      }
+
+      return validateHelper(spec)
+      .then(system => {
+        const allSemanticErrors = system.errSelectors.allErrors().toJS()
+          .filter(err => err.source !== "resolver")
+          expect(allSemanticErrors[0]).toInclude({
+            level: "warning",
+            message: "Definition was declared but never used in document",
+            path: ["definitions", "foo bar"]
+          })
+          expect(allSemanticErrors.length).toEqual(2)
+          expect(allSemanticErrors[1]).toInclude({
+            level: "error",
+            message: "$ref values must be RFC3986-compliant percent-encoded URIs",
+            path: ["paths", "/foo", "get", "responses", "200", "schema", "$ref"]
+          })
+       })
+    })
+    it("should return an error when a JSON pointer uses incorrect percent-encoding in OpenAPI 3", () => {
+      const spec = {
+        "openapi": "3.0.0",
+        "paths": {
+          "/foo": {
+            "get": {
+              "responses": {
+                "200": {
+                  "description": "Success",
+                  "schema": {
+                    "$ref": "#/components/schemas/foo bar"
+                  }
+                }
+              }
+            }
+          }
+        },
+        components: {
+          schemas: {
+            "foo bar": {
+              "type": "string"
+            }
+          }
+        }
+      }
+
+      return validateHelper(spec)
+      .then(system => {
+        const allSemanticErrors = system.errSelectors.allErrors().toJS()
+          .filter(err => err.source !== "resolver")
+          expect(allSemanticErrors[0]).toInclude({
+            level: "warning",
+            message: "Definition was declared but never used in document",
+            path: ["components", "schemas", "foo bar"]
+          })
+          expect(allSemanticErrors.length).toEqual(2)
+          expect(allSemanticErrors[1]).toInclude({
+            level: "error",
+            message: "$ref values must be RFC3986-compliant percent-encoded URIs",
+            path: ["paths", "/foo", "get", "responses", "200", "schema", "$ref"]
+          })
+      })
+    })
+    it("should return no errors when a JSON pointer uses correct percent-encoding in Swagger 2", () => {
+      const spec = {
+        "swagger": "2.0",
+        "paths": {
+          "/foo": {
+            "get": {
+              "responses": {
+                "200": {
+                  "description": "Success",
+                  "schema": {
+                    "$ref": "#/definitions/foo%20bar"
+                  }
+                }
+              }
+            }
+          }
+        },
+        "definitions": {
+          "foo bar": {
+            "type": "string"
+          }
+        }
+      }
+
+      return validateHelper(spec)
+      .then(system => {
+        const allSemanticErrors = system.errSelectors.allErrors().toJS()
+          .filter(err => err.source !== "resolver")
+        expect(allSemanticErrors).toEqual([])
+      })
+    })
+    it("should return no errors when a JSON pointer uses correct percent-encoding in OpenAPI 3", () => {
+      const spec = {
+        "openapi": "3.0.0",
+        "paths": {
+          "/foo": {
+            "get": {
+              "responses": {
+                "200": {
+                  "description": "Success",
+                  "schema": {
+                    "$ref": "#/components/schemas/foo%20bar"
+                  }
+                }
+              }
+            }
+          }
+        },
+        components: {
+          schemas: {
+            "foo bar": {
+              "type": "string"
+            }
+          }
+        }
+      }
+
+      return validateHelper(spec)
+      .then(system => {
+        const allSemanticErrors = system.errSelectors.allErrors().toJS()
+          .filter(err => err.source !== "resolver")
+        expect(allSemanticErrors).toEqual([])
+      })
+    })
 
   })
   describe("Nonexistent $ref pointers", () => {