Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix(oas3): validate xml and x-www-form-urlencoded request body #9705

Open
wants to merge 1 commit into
base: issue-9673
Choose a base branch
from
Open

fix(oas3): validate xml and x-www-form-urlencoded request body #9705

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 3 additions & 1 deletion src/core/components/execute.jsx
View file
Open in desktop
Expand Up @@ -35,6 +35,7 @@ export default class Execute extends Component {
let oas3ValidateBeforeExecuteSuccess = oas3Selectors.validateBeforeExecute([path, method])
let oas3RequestContentType = oas3Selectors.requestContentType(path, method)
let oas3RequestBody = specSelectors.getOAS3RequestBody([path, method])
let oas3RequestBodyInclusionSetting = oas3Selectors.requestBodyInclusionSetting(path, method)

if (!oas3ValidateBeforeExecuteSuccess) {
validationErrors.missingBodyValue = true
Expand All @@ -52,7 +53,8 @@ export default class Execute extends Component {
let valueErrors = oas3Selectors.validateValues({
oas3RequestBody,
oas3RequestContentType,
oas3RequestBodyValue
oas3RequestBodyValue,
oas3RequestBodyInclusionSetting
})
if ((!missingRequiredKeys || missingRequiredKeys.length < 1)
&& (!valueErrors || valueErrors.length < 1)) {
Expand Down
16 changes: 10 additions & 6 deletions src/core/plugins/oas3/reducers.js
View file
Open in desktop
Expand Up @@ -72,21 +72,21 @@ export default {
// context: is application/json or application/xml, where typeof (missing) bodyValue = String
return state.setIn(["requestData", path, method, "errors"], fromJS(errors))
}
let newState = state
if (validationErrors.missingRequiredKeys && validationErrors.missingRequiredKeys.length > 0) {
// context: is application/x-www-form-urlencoded, with list of missing keys
const { missingRequiredKeys } = validationErrors
return state.updateIn(["requestData", path, method, "bodyValue"], fromJS({}), missingKeyValues => {
newState = newState.updateIn(["requestData", path, method, "bodyValue"], fromJS({}), missingKeyValues => {
return missingRequiredKeys.reduce((bodyValue, currentMissingKey) => {
return bodyValue.setIn([currentMissingKey, "errors"], fromJS(errors))
}, missingKeyValues)
})
}
if (validationErrors.valueErrors && validationErrors.valueErrors.length > 0) {
// context: is application/json, with list of errors
return state.setIn(["requestData", path, method, "errors"], fromJS(validationErrors.valueErrors))
// context: is application/json, application/xml or application/x-www-form-urlencoded, with list of errors
newState = newState.setIn(["requestData", path, method, "errors"], fromJS(validationErrors.valueErrors))
}
console.warn("unexpected result: SET_REQUEST_BODY_VALIDATE_ERROR")
return state
return newState
},
[CLEAR_REQUEST_BODY_VALIDATE_ERROR]: (state, { payload: { path, method } }) => {
const requestBodyValue = state.getIn(["requestData", path, method, "bodyValue"])
Expand All @@ -97,11 +97,15 @@ export default {
if (!valueKeys) {
return state
}
return state.updateIn(["requestData", path, method, "bodyValue"], fromJS({}), bodyValues => {
const newState = state.updateIn(["requestData", path, method, "bodyValue"], fromJS({}), bodyValues => {
return valueKeys.reduce((bodyValue, curr) => {
return bodyValue.setIn([curr, "errors"], fromJS([]))
}, bodyValues)
})
if (newState.getIn(["requestData", path, method, "errors"], fromJS([])).size !== 0) {
return newState.setIn(["requestData", path, method, "errors"], fromJS([]))
}
return newState
},
[CLEAR_REQUEST_BODY_VALUE]: (state, { payload: { pathMethod }}) => {
let [path, method] = pathMethod
Expand Down
37 changes: 32 additions & 5 deletions src/core/plugins/oas3/selectors.js
View file
Open in desktop
Expand Up @@ -6,6 +6,7 @@ import constant from "lodash/constant"

import { getDefaultRequestBodyValue } from "./components/request-body"
import { stringify, validateParam } from "core/utils"
import { parseXML } from "core/utils/xmlParse"

// Helpers

Expand Down Expand Up @@ -310,15 +311,41 @@ export const validateShallowRequired = (

export const validateValues = (
state,
{
oas3RequestBody,
oas3RequestContentType,
oas3RequestBodyValue
{
oas3RequestBody,
oas3RequestContentType,
oas3RequestBodyValue,
oas3RequestBodyInclusionSetting,
}
) => {
if (oas3RequestContentType !== "application/json") {
const supportedContentType = [
"application/json",
"application/xml",
"application/x-www-form-urlencoded",
]

if (!supportedContentType.includes(oas3RequestContentType)) {
return []
}

if (oas3RequestContentType === "application/xml") {
oas3RequestBodyValue = parseXML(oas3RequestBodyValue)
if (oas3RequestBodyValue.xmlError) {
return [oas3RequestBodyValue.xmlError]
}
}

if (oas3RequestContentType === "application/x-www-form-urlencoded") {
oas3RequestBodyValue = oas3RequestBodyValue
.map((value, key) => {
value = value.get("value")
return value === "" && !oas3RequestBodyInclusionSetting.includes(key)
? undefined
: value
})
.toJS()
}

return validateParam(oas3RequestBody, oas3RequestBodyValue, {
bypassRequiredCheck: false,
isOAS3: true,
Expand Down
45 changes: 45 additions & 0 deletions src/core/utils/xmlParse.js
View file
Open in desktop
@@ -0,0 +1,45 @@
/**
* @prettier
*/

const traverseXML = (node, result = {}) => {
if (node.nodeType === 1) {
// ELEMENT_NODE - parameter
const name = node.nodeName

if (node.nodeName === "parsererror") {
throw new Error("Parameter string value must be valid XML")
}

const childNodes = Array.from(node.childNodes)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We might need to get rid of nodes with only whitespace characters here, as at this point our result is this:

{
    "0": "\n",
    "1": "\t",
    "id": "10",
    "name": "doggie",
    "category": {
        "0": "\n",
        "1": "\t",
        "2": "\t",
        "id": "1",
        "name": "Dogs"
    }
}

The validation for this will pass but perhaps it might create issues later on.


if (childNodes.length === 1) {
// only one child node - value of the parameter
result[name] = traverseXML(childNodes[0])
} else if (childNodes.length > 0) {
// more than one child node - the parameter is an object or an array
const traversedChildren = childNodes.map((child) => traverseXML(child))
result[name] = traversedChildren.reduce((paramObj, child) => {
Object.entries(child).forEach(([key, value]) => {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As I mentioned in a comment

We might need to get rid of nodes with only whitespace characters

Perhaps we could just add if (typeof child !== "string") here, as traversedChildren will look something like this:

[
    "\n\t\t",
    {
        "id": "1"
    },
    "\n\t\t",
    {
        "name": "Dogs"
    },
    "\n\t"
]

It seems like the actual parameters should be { key: value } objects.

paramObj[key] = value
Copy link
Contributor Author

@glowcloud glowcloud Mar 18, 2024 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is fine if the type of our parameter is an object but if we have an array, ex.

	<photoUrls>
		<photoUrl>a</photoUrl>
		<photoUrl>b</photoUrl>
		<photoUrl>c</photoUrl>
	</photoUrls>

the JSON result for it will be

{
  "photoUrls": {
      "photoUrl": "c"
  }
}

and that's the only thing we'll be validating.

Perhaps we could do something like this

  let isArray = false
  Object.entries(child).forEach(([key, value]) => {
    if (paramObj[key] === undefined) {
      paramObj[key] = value
    } else if (isArray) {
      paramObj[key].push(value)
    } else {
      paramObj[key] = [paramObj[key], value]
      isArray = true
    }
  })
  if (isArray) {
    paramObj = Object.values(paramObj)[0]
  }

In this case, arrays of length 0 or 1 would still be seen as an object.

})
return paramObj
}, {})
}
return result
} else if (node.nodeType === 3) {
// TEXT_NODE - value of a parameter (parent node)
return node.nodeValue
}
return undefined
}

export const parseXML = (xml) => {
Copy link
Contributor Author

@glowcloud glowcloud Mar 18, 2024 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure about the name of this function, perhaps it should be xmlToJson to better reflect what it does.

try {
xml = new DOMParser().parseFromString(xml, "application/xml")
xml = traverseXML(xml.childNodes[0])
return Object.values(xml)[0]
} catch (e) {
return { xmlError: e.message }
}
}
137 changes: 137 additions & 0 deletions test/unit/core/plugins/oas3/reducers.js
View file
Open in desktop
Expand Up @@ -450,6 +450,143 @@ describe("oas3 plugin - reducer", function () {
})
})

describe("missingRequiredKeys and valueErrors exist with length, e.g. application/x-www-form-urlencoded", () => {
it("should set errors", () => {
const state = fromJS({
requestData: {
"/pet": {
post: {
bodyValue: {
id: {
value: "test",
},
name: {
value: "",
},
},
requestContentType: "application/x-www-form-urlencoded"
}
}
}
})

const result = setRequestBodyValidateError(state, {
payload: {
path: "/pet",
method: "post",
validationErrors: {
missingBodyValue: null,
missingRequiredKeys: ["name"],
valueErrors: [
{
propKey: "id",
error: "Value must be an integer"
}
]
},
}
})

const expectedResult = {
requestData: {
"/pet": {
post: {
bodyValue: {
id: {
value: "test",
},
name: {
value: "",
errors: ["Required field is not provided"],
},
},
requestContentType: "application/x-www-form-urlencoded",
errors: [
{
propKey: "id",
error: "Value must be an integer"
}
]
}
}
}
}

expect(result.toJS()).toEqual(expectedResult)
})

it("should overwrite errors", () => {
const state = fromJS({
requestData: {
"/pet": {
post: {
bodyValue: {
id: {
value: "",
errors: ["some fake error"],
},
name: {
value: 123,
},
},
requestContentType: "application/json",
errors: [
{
propKey: "name",
error: "some fake error"
}
]
}
}
}
})

const result = setRequestBodyValidateError(state, {
payload: {
path: "/pet",
method: "post",
validationErrors: {
missingBodyValue: null,
missingRequiredKeys: ["id"],
valueErrors: [
{
propKey: "name",
error: "Value must be a string"
}
]
},
}
})

const expectedResult = {
requestData: {
"/pet": {
post: {
bodyValue: {
id: {
value: "",
errors: ["Required field is not provided"],
},
name: {
value: 123,
},
},
requestContentType: "application/json",
errors: [
{
propKey: "name",
error: "Value must be a string"
}
]
}
}
}
}

expect(result.toJS()).toEqual(expectedResult)
})
})

describe("other unexpected payload, e.g. no missingBodyValue or missingRequiredKeys", () => {
it("should not throw error if receiving unexpected validationError format. return state unchanged", () => {
const state = fromJS({
Expand Down