cloudflare / chanfana

OpenAPI 3 and 3.1 schema generator and validator for Hono, itty-router and more!
https://chanfana.pages.dev
MIT License
288 stars 38 forks source link

Fix: Allow non required properties on obj params to be undefined #23

Closed william1616 closed 1 year ago

william1616 commented 1 year ago

Currently the request body validation will attempt to validate all properties on an object in the request body regardless of if that property is set or not

For example with this schema definition

const Task = {
  name: new Str({ example: 'lorem' }),
  slug: String,
  description: new Str({ required: false }),
  completed: Boolean,
  due_date: new DateOnly(),
}

You would expect that the request body below would be valid

{
  "name": "lorem",
  "slug": "string",
  "completed": true,
  "due_date": "2022-09-15"
}

However it's not and the below is returned from the validator

{
  "errors": {
    "body.description": "Cannot read properties of undefined (reading 'toString')"
  },
  "success": false,
  "result": {}
}

The problem is that the validate() function on the Str parameter first performs the below logic

if (typeof value !== 'string') {
  value = value.toString()
}

Which doesn't work if the value is undefined

Interesting at the top level the Body class (which is used to validate the request body) checks the below logic before validating

if (value === undefined || value === null) {
  if (this.params.default !== undefined && this.params.default !== null) {
    value = this.params.default
  } else {
    if (this.params.required) {
      throw new ValidationError('is required')
    } else {
      return null
    }
  }
}

Which, if a value is undefined (or null) will set it to it's default value (assuming that's set) and then if the value is still undefined or null throw an is required error rather than attempting to validate it

I've implemented similar logic in the Obj class's validate function however unlike in the Body class if there a value is undefined and has a default value is not changed and remains undefined. Whilst for Parameters setting a default value if the value is undefined makes sense it does not make sense for an object. I originally encountered this issue building a PATCH endpoint into an API. If you consider the below

First a request body definition with a default value

const Task = {
  name: new Str({ example: 'lorem' }),
  slug: String,
  description: new Str({ required: false }),
  completed: new Bool({ default: false }),
  due_date: new DateOnly(),
}

First a POST Request to create a Task

{
  "name": "lorem",
  "slug": "string",
  "completed": true,
  "due_date": "2022-09-15"
}

Then a PATCH Request to update the Task

{
  "description": "hello world!"
}

You would expect the Task to look like

{
  "name": "lorem",
  "description": "hello world!"
  "slug": "string",
  "completed": true,
  "due_date": "2022-09-15"
}

However if the default setting logic was implemented then actually the Task would look like this

{
  "name": "lorem",
  "description": "hello world!"
  "slug": "string",
  "completed": false,
  "due_date": "2022-09-15"
}

With this change undefined non required values will not be validated and undefined required values will return an is required rather than Cannot read properties of undefined (reading 'toString') error. (I've used Str as an example in this PR but the concept applies to bools & numbers as well

G4brym commented 1 year ago

Hey @william1616 thanks for fixing this issue I will take a look into this pr next week, when I'm back from vacations

G4brym commented 1 year ago

Hey @william1616 I've just pushed a new release (v0.0.13) with your fix

Thanks for contributing to this project

william1616 commented 1 year ago

Thanks @G4brym