lukeautry / tsoa

Build OpenAPI-compliant REST APIs using TypeScript and Node
MIT License
3.33k stars 481 forks source link

Array validation does not check if the provided value is an array #1563

Closed friedow closed 3 months ago

friedow commented 5 months ago

Sorting

Expected Behavior

If a request body contains a parameter typed as an array, it should be validated that the provided value is actually an array.

Current Behavior

If a request body contains a parameter typed as an array:

interface SomeRequestBody {
  some_parameter: string[];
}

You can actually provide a Request having the parameter set to the arrays generic type without the validation catching it:

// This won't return a validation error response but it should:
fetch('/some/endpoint', {
  method: 'POST',
  headers: {
    "Content-Type": "application/json",
  },
  body: JSON.stringify({
    // notice how we provide a string instead of a string array here
    some_parameter: "this should be invalid as it is a string and not an array"
  })
})

Possible Solution

I'll provide a fix for this in the code :).

Steps to Reproduce

See above

Context (Environment)

Version of the library: v6.0.1 Version of NodeJS: v18.18.2

Detailed Description

I'll use the Array.isArray() function to validate that the input to validateArray() is in fact an array. If not, a validation error will be produced by the function.

Breaking change?

Nope :tada:

github-actions[bot] commented 5 months ago

Hello there friedow 👋

Thank you for opening your very first issue in this project.

We will try to get back to you as soon as we can.👀

WoH commented 4 months ago

We are coercing to an array, so removing that would certainly be a breaking change.

friedow commented 4 months ago

I agree. Was there a reason why values were siently coerced to an array though?

maxuai commented 4 months ago

@WoH @friedow I think there is another very similar behavior in this issue: https://github.com/lukeautry/tsoa/issues/1444

Overall to maybe not make it a breaking change I would suggest to allow the implicit coercion to be disabled via config and to be enabled by default. WDYT?

WoH commented 4 months ago

Depends on the context. For bodies, I agree that it's weird, query params less so.

maxuai commented 4 months ago

Depends on the context. For bodies, I agree that it's weird, query params less so.

Agreed, for query params this makes more sense

friedow commented 4 months ago

To me it is really weird that validation changes inputs (body and query params alike). I think this should be a breaking change and validation should not implicitly coerce values.

maxuai commented 4 months ago

Just another sample that we currently facing: when the TS type allows number and string a value will be coerced to number if possible, so e.g. "1" will become 1

friedow commented 4 months ago

@WoH and I just had a talk offline and we agreed to make the coercion of request bodies configurable by adding a config flag bodyCoercion: boolean. I'll implement this next week.