fix(validateParam): validate JSON parameter values + support Parameter.content (swagger-api#5657)

shockey · web-flow · commit 75a0e5d5dcbf · 2019-10-11T11:20:23.000-07:00
* improve(getParameterSchema): ParameterSchemaDescriptor pattern

* chore: update usage of `getParameterSchema`

* consider `Parameter.content` media type when validating JSON values
diff --git a/src/core/components/parameter-row.jsx b/src/core/components/parameter-row.jsx
@@ -41,7 +41,7 @@ export default class ParameterRow extends Component {
     let enumValue
 
     if(isOAS3) {
-      let schema = getParameterSchema(parameterWithMeta, { isOAS3 })
+      let { schema } = getParameterSchema(parameterWithMeta, { isOAS3 })
       enumValue = schema.get("enum")
     } else {
       enumValue = parameterWithMeta ? parameterWithMeta.get("enum") : undefined
@@ -98,7 +98,7 @@ export default class ParameterRow extends Component {
 
     const paramWithMeta = specSelectors.parameterWithMetaByIdentity(pathMethod, rawParam) || Map()
 
-    const schema = getParameterSchema(paramWithMeta, { isOAS3: specSelectors.isOAS3() })
+    const { schema } = getParameterSchema(paramWithMeta, { isOAS3: specSelectors.isOAS3() })
 
     const parameterMediaType = paramWithMeta
       .get("content", Map())
@@ -209,9 +209,10 @@ export default class ParameterRow extends Component {
     const ExamplesSelectValueRetainer = getComponent("ExamplesSelectValueRetainer")
     const Example = getComponent("Example")
 
+    let { schema } = getParameterSchema(param, { isOAS3 })
     let paramWithMeta = specSelectors.parameterWithMetaByIdentity(pathMethod, rawParam) || Map()
+
     let format = param.get("format")
-    let schema = getParameterSchema(param, { isOAS3 })
     let type = schema.get("type")
     let isFormData = inType === "formData"
     let isFormDataSupported = "FormData" in win
diff --git a/src/core/utils.js b/src/core/utils.js
@@ -501,12 +501,14 @@ export const validatePattern = (val, rxPattern) => {
 export const validateParam = (param, value, { isOAS3 = false, bypassRequiredCheck = false } = {}) => {
   
   let errors = []
-  let required = param.get("required")
 
-  let paramDetails = getParameterSchema(param, { isOAS3 })
+  let paramRequired = param.get("required")
+
+  let { schema: paramDetails, parameterContentMediaType } = getParameterSchema(param, { isOAS3 })
 
   if(!paramDetails) return errors
 
+  let required = paramDetails.get("required")
   let maximum = paramDetails.get("maximum")
   let minimum = paramDetails.get("minimum")
   let type = paramDetails.get("type")
@@ -520,7 +522,7 @@ export const validateParam = (param, value, { isOAS3 = false, bypassRequiredChec
     then we should do our validation routine.
     Only bother validating the parameter if the type was specified.
   */
-  if ( type && (required || value) ) {
+  if ( type && (paramRequired || required || value) ) {
     // These checks should evaluate to true if there is a parameter
     let stringCheck = type === "string" && value
     let arrayCheck = type === "array" && Array.isArray(value) && value.length
@@ -533,29 +535,32 @@ export const validateParam = (param, value, { isOAS3 = false, bypassRequiredChec
     let objectCheck = type === "object" && typeof value === "object" && value !== null
     let objectStringCheck = type === "object" && typeof value === "string" && value
 
-    // if(type === "object" && typeof value === "string") {
-    //   // Disabled because `validateParam` doesn't consider the MediaType of the 
-    //   // `Parameter.content` hint correctly.
-    //   try {
-    //     JSON.parse(value)
-    //   } catch(e) {
-    //     errors.push("Parameter string value must be valid JSON")
-    //     return errors
-    //   }
-    // }
-
     const allChecks = [
       stringCheck, arrayCheck, arrayListCheck, arrayStringCheck, fileCheck, 
       booleanCheck, numberCheck, integerCheck, objectCheck, objectStringCheck,
     ]
 
     const passedAnyCheck = allChecks.some(v => !!v)
 
-    if (required && !passedAnyCheck && !bypassRequiredCheck ) {
+    if ((paramRequired || required) && !passedAnyCheck && !bypassRequiredCheck ) {
       errors.push("Required field is not provided")
       return errors
     }
 
+    if (
+      type === "object" &&
+      typeof value === "string" &&
+      (parameterContentMediaType === null ||
+        parameterContentMediaType === "application/json")
+    ) {
+      try {
+        JSON.parse(value)
+      } catch (e) {
+        errors.push("Parameter string value must be valid JSON")
+        return errors
+      }
+    }
+
     if (pattern) {
       let err = validatePattern(value, pattern)
       if (err) errors.push(err)
diff --git a/src/helpers/get-parameter-schema.js b/src/helpers/get-parameter-schema.js
@@ -23,6 +23,12 @@ const swagger2SchemaKeys = Im.Set.of(
   "multipleOf"
 )
 
+/**
+ * @typedef {Object} ParameterSchemaDescriptor
+ * @property {Immutable.Map} schema - the parameter schema
+ * @property {string|null} parameterContentMediaType - the effective media type, for `content`-based OpenAPI 3.0 Parameters, or `null` otherwise
+ */
+
 /**
  * Get the effective schema value for a parameter, or an empty Immutable.Map if
  * no suitable schema can be found.
@@ -35,18 +41,29 @@ const swagger2SchemaKeys = Im.Set.of(
  * @param {object} config
  * @param {boolean} config.isOAS3 Whether the parameter is from an OpenAPI 2.0
  * or OpenAPI 3.0 definition
- * @return {Immutable.Map} The desired schema
+ * @return {ParameterSchemaDescriptor} Information about the parameter schema
  */
 export default function getParameterSchema(parameter, { isOAS3 } = {}) {
   // Return empty Map if `parameter` isn't a Map
-  if (!Im.Map.isMap(parameter)) return Im.Map()
+  if (!Im.Map.isMap(parameter)) {
+    return {
+      schema: Im.Map(),
+      parameterContentMediaType: null,
+    }
+  }
 
   if (!isOAS3) {
     // Swagger 2.0
     if (parameter.get("in") === "body") {
-      return parameter.get("schema", Im.Map())
+      return {
+        schema: parameter.get("schema", Im.Map()),
+        parameterContentMediaType: null,
+      }
     } else {
-      return parameter.filter((v, k) => swagger2SchemaKeys.includes(k))
+      return {
+        schema: parameter.filter((v, k) => swagger2SchemaKeys.includes(k)),
+        parameterContentMediaType: null,
+      }
     }
   }
 
@@ -57,11 +74,19 @@ export default function getParameterSchema(parameter, { isOAS3 } = {}) {
       .get("content", Im.Map({}))
       .keySeq()
 
-    return parameter.getIn(
-      ["content", parameterContentMediaTypes.first(), "schema"],
-      Im.Map()
-    )
+    const parameterContentMediaType = parameterContentMediaTypes.first()
+
+    return {
+      schema: parameter.getIn(
+        ["content", parameterContentMediaType, "schema"],
+        Im.Map()
+      ),
+      parameterContentMediaType,
+    }
   }
 
-  return parameter.get("schema", Im.Map())
+  return {
+    schema: parameter.get("schema", Im.Map()),
+    parameterContentMediaType: null,
+  }
 }
diff --git a/test/mocha/core/helpers/get-parameter-schema.js b/test/mocha/core/helpers/get-parameter-schema.js
@@ -3,20 +3,22 @@
  */
 
 import expect from "expect"
-import Im, { fromJS } from "immutable"
+import { fromJS } from "immutable"
 import getParameterSchema from "../../../../src/helpers/get-parameter-schema"
 
 describe("getParameterSchema", () => {
   it("should return an empty Map when given no parameters", () => {
     const result = getParameterSchema()
 
-    expect(result).toEqual(fromJS({}))
+    expect(result.schema.toJS()).toEqual({})
+    expect(result.parameterContentMediaType).toEqual(null)
   })
 
   it("should return an empty Map when given an empty Map", () => {
     const result = getParameterSchema(fromJS({}))
 
-    expect(result).toEqual(fromJS({}))
+    expect(result.schema.toJS()).toEqual({})
+    expect(result.parameterContentMediaType).toEqual(null)
   })
 
   it("should return a schema for a Swagger 2.0 query parameter", () => {
@@ -34,12 +36,13 @@ describe("getParameterSchema", () => {
       })
     )
 
-    expect(result.toJS()).toEqual({
+    expect(result.schema.toJS()).toEqual({
       type: "array",
       items: {
         type: "string",
       },
     })
+    expect(result.parameterContentMediaType).toEqual(null)
   })
 
   it("should return a schema for a Swagger 2.0 body parameter", () => {
@@ -58,12 +61,13 @@ describe("getParameterSchema", () => {
       })
     )
 
-    expect(result.toJS()).toEqual({
+    expect(result.schema.toJS()).toEqual({
       type: "array",
       items: {
         type: "string",
       },
     })
+    expect(result.parameterContentMediaType).toEqual(null)
   })
 
   it("should return a schema for an OpenAPI 3.0 query parameter", () => {
@@ -87,12 +91,13 @@ describe("getParameterSchema", () => {
       }
     )
 
-    expect(result.toJS()).toEqual({
+    expect(result.schema.toJS()).toEqual({
       type: "array",
       items: {
         type: "string",
       },
     })
+    expect(result.parameterContentMediaType).toEqual(null)
   })
 
   it("should return a schema for an OpenAPI 3.0 query parameter with `content`", () => {
@@ -126,7 +131,7 @@ describe("getParameterSchema", () => {
       }
     )
 
-    expect(result.toJS()).toEqual({
+    expect(result.schema.toJS()).toEqual({
       type: "object",
       required: ["lat", "long"],
       properties: {
@@ -138,5 +143,6 @@ describe("getParameterSchema", () => {
         },
       },
     })
+    expect(result.parameterContentMediaType).toEqual(`application/json`)
   })
 })
diff --git a/test/mocha/core/utils.js b/test/mocha/core/utils.js
