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.

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

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

Closed
2 of 4 tasks
friedow opened this issue Feb 16, 2024 · 9 comments · Fixed by #1578
Closed
2 of 4 tasks

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

friedow opened this issue Feb 16, 2024 · 9 comments · Fixed by #1578

Comments

@friedow
Copy link
Contributor

friedow commented Feb 16, 2024

Sorting

  • I'm submitting a ...

    • bug report
    • feature request
    • support request
  • I confirm that I

    • used the search to make sure that a similar issue hasn't already been submit

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

  • Confirm you were using yarn not npm: [x]

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 🎉

Copy link

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
Copy link
Collaborator

WoH commented Feb 18, 2024

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

@friedow
Copy link
Contributor Author

friedow commented Feb 19, 2024

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

@maxuai
Copy link

maxuai commented Feb 20, 2024

@WoH @friedow I think there is another very similar behavior in this issue: #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
Copy link
Collaborator

WoH commented Feb 20, 2024

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

@maxuai
Copy link

maxuai commented Feb 22, 2024

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
Copy link
Contributor Author

friedow commented Feb 22, 2024

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
Copy link

maxuai commented Feb 23, 2024

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
Copy link
Contributor Author

friedow commented Mar 1, 2024

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants