nestjs / nest

A progressive Node.js framework for building efficient, scalable, and enterprise-grade server-side applications with TypeScript/JavaScript 🚀
https://nestjs.com
MIT License
66.91k stars 7.55k forks source link

ParseArrayPipe is not validated when used with separator. #7408

Closed CatsMiaow closed 2 years ago

CatsMiaow commented 3 years ago

Related issues https://github.com/nestjs/nest/issues/5467

Code

// http://localhost:3000/hello/pipe?ids=1,a,null,3
@Get('hello/pipe')
public helloPipe(@Query('ids', new ParseArrayPipe({ items: Number, separator: ',' })) ids: number[]): number[] {
  return ids;
}

https://docs.nestjs.com/techniques/validation#parsing-and-validating-arrays

Result

[1,"a",null,3]

Trace https://github.com/nestjs/nest/blob/40271286d125f137e06a6619ee33cefc31da5e43/packages/common/pipes/parse-array.pipe.ts#L89 validationPipe.transform is executed in ParseArrayPipe and

https://github.com/nestjs/nest/blob/40271286d125f137e06a6619ee33cefc31da5e43/packages/common/pipes/validation.pipe.ts#L87 transformPrimitive is executed in validationPipe.transform

https://github.com/nestjs/nest/blob/40271286d125f137e06a6619ee33cefc31da5e43/packages/common/pipes/validation.pipe.ts#L154-L158

transformPrimitive(value, metadata) {
  console.log(value, metadata);

image Since metadata.data is not passed to the transformPrimitive method, the value is returned as it is.

However, ignoring(comment out) the return value; and proceeding with the next process does not throw a validate error.

[1,null,0,3]

Should get a validation error like ParseIntPipe.

{
  "statusCode": 400,
  "message": "Validation failed (blahblahblah)",
  "error": "Bad Request"
}
Tony133 commented 3 years ago

I recommend that you leave a minimal replay of a clonable git repository so that the NestJS core team can evaluate the issue you reported.

I don't remember if this problem will be solved in v8 of Nest, we'll see.

kamilmysliwiec commented 3 years ago

Please provide a minimum reproduction repository.

chris9740 commented 3 years ago

This issue was created after I asked about it in the NestJS Discord server, so I was able to create a reproduction repository on behalf of CatsMiaow: https://github.com/chris9740/nestjs-repro

When a request is sent to http://localhost:3000/?id=1,2,3,a, the expected result is that the comma separated values should be validated as a Number, and throw an exception if any of them isn't since that's what the items property is set to, but instead it parses the ones it can and ignores the other ones. The return value will be an array with [1, 2, 3, "a"]

CatsMiaow commented 3 years ago

https://github.com/CatsMiaow/nest-issues-7408

I made a sample code using nestjs/typescript-starter. Added commit. https://github.com/CatsMiaow/nest-issues-7408/commit/e148367cf0536898e0f6ff3b231f5e9fd8fb6748

image

kamilmysliwiec commented 2 years ago

This should be fixed in 8.0.11