Class-validator: Null value passes when skipMissingProperties set to true

Created on 24 Jan 2019  路  11Comments  路  Source: typestack/class-validator

When skipMissingProperties is set to true, if property has null value, it will not validate with IsNotEmpty decorator and null value will be forwarded to controller/service/db repo etc. and cause a crash
Source of error

Example

Json input

{
    "startDate":null 
}

Part of UpdateProjectDto

 @IsNotEmpty()
 @IsDateString()
 readonly startDate: Date;

Nest.js framework controller example of code which will receive poData with startDate set to null and crash at DB query...

@Patch('/:id')
@UsePipes(new ValidationPipe({ skipMissingProperties: true }))
    updateById(@Param('id', new ParseIntPipe()) id, @Body() poData: UpdateProjectDto) {
        //#update route
        return this.projectsService.updateById(id, poData);
   }
feature

Most helpful comment

I'm having a similar issue.
Why would we consider null as equivalent to undefined in this case?
I believe this behavior breaks partial object validation for non-nullable fields.

For example:

    @IsString()
    @IsNotEmpty()
    @IsDefined()
    name: string;

Those two objects would both pass validation when the skipMissingProperties is applied

validate({}, { skipMissingProperties: true})
validate({ "name": null }, { skipMissingProperties: true})

Which, in my case, triggers a database Column 'name' cannot be null error.

Wouldn't it make more sense if skipMissingProperties and the IsDefined decorators relied solely on whether the property is, well, defined instead of also checking if it is null?

All 11 comments

any idea why this might be anyone?

Just search the source code :-)

// ValidationExecutor#execute
...
            if ((value === null || value === undefined) && this.validatorOptions && this.validatorOptions.skipMissingProperties === true) {
                return;
            }
...

https://github.com/typestack/class-validator/blob/master/src/validation/ValidationExecutor.ts#L100

I'm having a similar issue.
Why would we consider null as equivalent to undefined in this case?
I believe this behavior breaks partial object validation for non-nullable fields.

For example:

    @IsString()
    @IsNotEmpty()
    @IsDefined()
    name: string;

Those two objects would both pass validation when the skipMissingProperties is applied

validate({}, { skipMissingProperties: true})
validate({ "name": null }, { skipMissingProperties: true})

Which, in my case, triggers a database Column 'name' cannot be null error.

Wouldn't it make more sense if skipMissingProperties and the IsDefined decorators relied solely on whether the property is, well, defined instead of also checking if it is null?

Some properties of my model can be null, some can't. None can be undefined.
In case of PUT / PATCH HTTP requests, I'm using { skipMissingProperties: true} to validate the body of my request because it contains only updated properties.

However:

  • { skipMissingProperties: true} doesn't validate null values
  • @IsDefined() makes no differences between null and undefined
    So, it is not possible to validate correctly the request body.

Solutions:

  • skipMissingProperties could skip only undefined values (but it would introduce breaking changes I guess)
  • Add a skipUndefinedProperties option (quite easy from what I read)

"skipUndefinedProperties " is right

I really also think that handling null and undefined the same doesn't make sense! That is like handling numbers and strings the same 馃槃

People in this thread go to see the PR that could possibly solve this issue and give it a review! https://github.com/typestack/class-validator/pull/353

temporary solution

turn off skipMissingProperties: {skipMissingProperties: false}

and use @ValidateIf decorator:

@ValidateIf(o => 'name' in o)
@IsString()
@IsNotEmpty()
readonly name?: string;

I agree skipMissingProperties is little bit misleading. We should introduce skipUndefinedProperties and also skipNullProperties. skipMissingProperties should be deprecated and removed in 1.0 version. What do you think about skipNullProperties? Is it useable option, too?

Yes, in some APIs you want to delete null values, and i some you want to save them. These two options would give us the diversity we need.

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

jerradpatch picture jerradpatch  路  4Comments

twittwer picture twittwer  路  6Comments

st-clair-clarke picture st-clair-clarke  路  3Comments

MistyKuu picture MistyKuu  路  3Comments

yantrab picture yantrab  路  3Comments