ParseFilePipeBuilder Cannot read properties of undefined (reading 'size') when upload file

aribambang commented 2 years ago

Is there an existing issue for this?

[X] I have searched the existing issues

Current behavior

i'm uploading image file using form-data, when i don't attach file then in ParseFilePipeBuilder function i get error ERROR [ExceptionsHandler] Cannot read properties of undefined (reading 'size')

[Nest] 56810  - 07/25/2022, 4:34:31 AM   ERROR [ExceptionsHandler] Cannot read properties of undefined (reading 'size')
TypeError: Cannot read properties of undefined (reading 'size')
    at MaxFileSizeValidator.isValid (/Users/aribambang/project/node_modules/@nestjs/common/pipes/file/max-file-size.validator.js:20:21)
    at ParseFilePipe.validateOrThrow (/Users/aribambang/project/node_modules/@nestjs/common/pipes/file/parse-file.pipe.js:39:41)
    at ParseFilePipe.validate (/Users/aribambang/project/node_modules/@nestjs/common/pipes/file/parse-file.pipe.js:34:24)
    at ParseFilePipe.transform (/Users/aribambang/project/node_modules/@nestjs/common/pipes/file/parse-file.pipe.js:28:24)
    at /Users/aribambang/project/node_modules/@nestjs/core/pipes/pipes-consumer.js:16:33
    at processTicksAndRejections (node:internal/process/task_queues:95:5)
    at resolveParamValue (/Users/aribambang/project/node_modules/@nestjs/core/router/router-execution-context.js:147:23)
    at async Promise.all (index 0)
    at pipesFn (/Users/aribambang/project/node_modules/@nestjs/core/router/router-execution-context.js:150:13)
    at /Users/aribambang/project/node_modules/@nestjs/core/router/router-execution-context.js:37:30

@UseInterceptors(FileInterceptor('logo'))
  @Post()
  async create(
    @Body() createDto: CreateDto,
    @UploadedFile(
      new ParseFilePipeBuilder()
        .addMaxSizeValidator({
          maxSize: 1048576,
        })
        .addFileTypeValidator({
          fileType: /\.(jpe?g|png)$/i,
        })
        .build({
          errorHttpStatusCode: HttpStatus.UNPROCESSABLE_ENTITY,
        }),
    )
    logo?: Express.Multer.File,
  ): Promise<void> {
    console.log(createDto);
    console.log(logo);
  }

Minimum reproduction code

https://stackblitz.com/edit/nestjs-typescript-starter-amuib7

Steps to reproduce

No response

Expected behavior

We must to create a validation function on ParseFilePipeBuilder when the file doesn't exist during upload.

Package

[ ] I don't know. Or some 3rd-party package
[X] code>@nestjs/common</code
[ ] code>@nestjs/core</code
[ ] code>@nestjs/microservices</code
[ ] code>@nestjs/platform-express</code
[ ] code>@nestjs/platform-fastify</code
[ ] code>@nestjs/platform-socket.io</code
[ ] code>@nestjs/platform-ws</code
[ ] code>@nestjs/testing</code
[ ] code>@nestjs/websockets</code
[ ] Other (see below)

Other package

No response

NestJS version

9.0.5

Packages versions

    "@nestjs-modules/mailer": "^1.8.1",
    "@nestjs/common": "^9.0.5",
    "@nestjs/config": "^2.2.0",
    "@nestjs/core": "^9.0.5",
    "@nestjs/jwt": "^9.0.0",
    "@nestjs/passport": "^9.0.0",
    "@nestjs/platform-express": "^9.0.5",
    "@nestjs/sequelize": "^9.0.0",

Node.js version

No response

In which operating systems have you tested?

[X] macOS
[ ] Windows
[ ] Linux

Other

No response

jmcdo29 commented 2 years ago

You know, linking to our own code isn't a minimum reproduction.

I'm surprised this happens rather than multer throwing an exception as interceptors are triggered before pipes.

Please provide a minimum reproduction repository (Git repository/StackBlitz/CodeSandbox project).

aribambang commented 2 years ago

You know, linking to our own code isn't a minimum reproduction.

I'm surprised this happens rather than multer throwing an exception as interceptors are triggered before pipes.

Please provide a minimum reproduction repository (Git repository/StackBlitz/CodeSandbox project).

i'm sorry, I have updated the minimum reproduction.

jmcdo29 commented 2 years ago

Okay, I can confirm that multer only cares about the field existing, not necessarily that there is a file to parse there. We should probably add a FileExistsValidator that we prepend to the array if there are validators to be ran

kamilmysliwiec commented 2 years ago

cc @thiagomini

thiagomini commented 2 years ago

I'll have a look on that and work in a fix asap, thanks for reporting!

thiagomini commented 2 years ago

@kamilmysliwiec Created a PR with the fix for this issue, would appreciate your review on that

woutrbe commented 2 years ago

I'm dealing with the same issue as this, where I have an optional file upload field. While PR seems like a good solution, is there any way to use the ParseFilePipeBuilder manually in the controller? So that I can catch the errors myself?

@Post('/')
@UseInterceptors(FileInterceptor('logo'))
public async save(
   @Body() body: SettingsDto,
   @UploadedFile() file: any,
   @Res() res: Response,
) {
   try {
      // How would I use ParseFilePipeBuilder here? 
     new ParseFilePipeBuilder()
         .addFileTypeValidator({
        fileType: 'ico'
      })
         .addMaxSizeValidator({
        maxSize: 50000
         })
     .build()
   } catch(err) { 
        return res.render('/views/settings.pug', { errror: err.message })
   }
}

kamilmysliwiec commented 2 years ago

Let's track this here https://github.com/nestjs/nest/pull/10025

nestjs / nest