typestack / class-validator

Decorator-based property validation for classes.
MIT License
10.93k stars 788 forks source link

fix: stopAtFirstError not working as expected, a custom decorator executes when previous validator fails #958

Open RRGT19 opened 3 years ago

RRGT19 commented 3 years ago

Description

I have a custom decorator as the last one that validates a string against my database. It's still being called and executed even if a previous validator has failed.

ValidationPipe configuration

app.useGlobalPipes(
    new ValidationPipe({
      stopAtFirstError: true, // <- Feature activated.
      whitelist: true,
      forbidNonWhitelisted: true,
      transform: true,
    }),
  );

Entity DTO

@IsString()
@IsNotEmpty() // <- Seems that not executed, did not throw error to the client.
@Length(11, 11) // <- Executed, If this fails, should stop and return error to the client.
@IsUnique({ table: 'users' }) // <-- Custom, validates against database.
readonly identification: string;

POST Request example

{
   "identification": "",
}

Response

{
  "statusCode": 400,
  "message": [
    "identification must be longer than or equal to 11 characters",
    "The value  of identification already exists." // <- Oops!, should not happen.
  ],
  "error": "Bad Request"
}

My custom validator and decorator

@ValidatorConstraint({
  name: 'isUnique',
  async: true,
})
@Injectable()
export class IsUniqueValidator implements ValidatorConstraintInterface {
  async validate(value: string, validationArgs?: ValidationArguments): Promise<boolean> {

    // "value" is empty here... I did this as a workaround.
    if (isEmpty(value)) {
      return false;
    }

    const results: any[] = []; // Represents the result of a query against database.

    return results.length === 0;
  }

  defaultMessage(validationArguments?: ValidationArguments): string {
    const value = validationArguments?.value;
    const property = validationArguments?.property;
    return `The value ${value} of ${property} already exists.`;
  }
}

export function IsUnique(params: any, validationOptions?: ValidationOptions) {
  return function (object: Object, propertyName: string) {
    registerDecorator({
      target: object.constructor,
      propertyName: propertyName,
      options: validationOptions,
      constraints: [params],
      validator: IsUniqueValidator,
    });
  };
}

Expected behavior

Just the result of a previous validation, example:

{
  ...
  "message": [
    "identification must be longer than or equal to 11 characters",
  ],
  ..
}

To be more precise, the @IsNotEmpty()should have stop the empty string as shown above, being the only error returned to the client.

Actual behavior

The custom validator, which is the last one, is still being called and executed.

class-validator: 0.13.1

feRpicoral commented 2 years ago

Any updates on this? I'm facing the same issue.

thiennhan2310 commented 2 years ago

same

secmohammed commented 2 years ago

Due to the nature of Typescript decorators, All of the decorators are going to run in reversed order as for: https://www.typescriptlang.org/docs/handbook/decorators.html#decorator-composition

Therefore, it's not really related to your custom rule as you probably thought. Since your custom decorator is the last decorator, it means that it's going to be the first decorator to be executed. That's why you are seeing its error message.

smentek commented 2 years ago

Due to the nature of Typescript decorators, All of the decorators (...)

This is correct, but it will not work in the way you describe (reversed from decoration) with async validators. Async validation will be triggered at the beginning. I described it better under: https://github.com/typestack/class-validator/issues/1688 I would propose to close this ticket and keep only #1688 since stopAtFirstError option has nothing to do with the origin of the problem.

nllahat commented 1 year ago

@smentek @RRGT19 @secmohammed @thiennhan2310 @feRpicoral Can any of you help me understand y this one passes when I send a req without this optional param (someString) ?

If it runs in the reversed order so I would expect it to fail on the IsNotEmpty decorator.

export class SomeDto {
  @IsOptional()
  @IsString()
  @IsNotEmpty()
  public someString?: string
  .
  .
  .
}

Thanks a lot ! 🙏

manojkmishra commented 1 year ago

Is stopAtFirstError really working for someone, I am trying to configure it as below and its not working at all. Is there something I am missing in configuration.

app.useGlobalPipes( new ValidationPipe({ transform: true, stopAtFirstError: true, }), );

chapeee commented 10 months ago

i am facing the same issue

dhtmdgkr123 commented 9 months ago

+1

kavyantic commented 4 months ago

Is stopAtFirstError really working for someone, I am trying to configure it as below and its not working at all. Is there something I am missing in configuration.

app.useGlobalPipes( new ValidationPipe({ transform: true, stopAtFirstError: true, }), );

I am facing the same issue. Even after enabling the option nothing seems to change and class-validator generates too many errors as before.

kavyantic commented 4 months ago

Is this active?

sebasaenz commented 2 months ago

I'm facing the same issue and is a bit concerning since this feature is quite necessary to prevent from potential ReDoS attacks (if a long string is catched earlier with @Max() then there's no need for the @Matches() validator to be executed).

cracker-source commented 2 months ago

Anyone has solution for this issue?