Skip to content

chore(common): Backport FileTypeValidator fallback support to v10.4.17#15003

Open
mag123c wants to merge 1 commit into
nestjs:10.4.17from
mag123c:backport/10.4.17-filetype-fallback
Open

chore(common): Backport FileTypeValidator fallback support to v10.4.17#15003
mag123c wants to merge 1 commit into
nestjs:10.4.17from
mag123c:backport/10.4.17-filetype-fallback

Conversation

@mag123c
Copy link
Copy Markdown
Contributor

@mag123c mag123c commented Apr 22, 2025

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Other... Please describe:

What is the current behavior?

File validation fails when the buffer is too small or unidentifiable by magic number,
even if the mimetype is correct.

Related issue: #14977

What is the new behavior?

  • Adds a fallbackToMimetype option to FileTypeValidator
  • If magic number detection fails, and fallback is enabled, the validator uses the mimetype
  • Also improves buildErrorMessage() for clearer failure messages

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

This PR backports the fix originally merged in #14995 into the 10.4.17 line.

This was originally reported in #14977. All tests have been verified on the v10 codebase.

@mehdirayan
Copy link
Copy Markdown

Hi
i have an old code that no longer works after the upgrade.
i intercept the file like this:

@UseInterceptors(
FileInterceptor('photo', { dest: join(__dirname, '..', '/uploads') }),
)
@post()
async uploadeFile(
@uploadedfile(
new ParseFilePipe({
validators: [
new FileTypeValidator({ fileType: 'image/jpeg' }),
new MaxFileSizeValidator({ maxSize: 1024 * 100 }),
],
}),
)
file: Express.Multer.File,
)

The file is saved in the upload folder, so the buffer is missing in the file that is transmitted to validation.
The validation throw a type mismatch exception, with is confusing.

@mag123c
Copy link
Copy Markdown
Contributor Author

mag123c commented May 22, 2025

@mehdirayan The issue for verifying the magic-number, the root cause of the issue, has not yet been resolved.

Check the issues below #14876, #14977, #15055

Copy link
Copy Markdown

@themavik themavik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fallbackToMimetype means when fileTypeFromBuffer returns null you accept the client mimetype regex, which is weaker than magic-byte checks—call that out for security-minded users. The short-buffer vs mismatch specs help.

@Qodo-Free-For-OSS
Copy link
Copy Markdown

Hi, When magic-number detection throws, isValid() returns false without attempting mimetype fallback even if fallbackToMimetype is enabled. This breaks the intended “fallback when detection fails” behavior for import/parsing/runtime errors.

Severity: action required | Category: correctness

How to fix: Apply fallback in catch block

Agent prompt to fix - you can give this to your LLM of choice:

Issue description

FileTypeValidator.isValid() currently returns false on any exception thrown by the file-type import or fileTypeFromBuffer(), even when fallbackToMimetype is enabled. This means the new fallback feature does not work when detection fails via exception.

Issue Context

Fallback logic is implemented only for the fileType === null/undefined case, not for the exception case.

Fix Focus Areas

  • packages/common/pipes/file/file-type.validator.ts[73-96]

Suggested change

In the catch block, if fallbackToMimetype is enabled, perform the same file.mimetype.match(this.validationOptions.fileType) check and return that result; otherwise return false.

Also consider adding a unit test that simulates a thrown error from detection (e.g., by stubbing the import/detection) to ensure fallback works in this path.


Qodo code review - free for open-source.

@Qodo-Free-For-OSS
Copy link
Copy Markdown

Hi, buildErrorMessage() appends “magic number detection failed, used mimetype fallback” whenever fallbackToMimetype is enabled, even if validation failed due to a detected magic-number mismatch (where fallback was not used). This produces factually incorrect error messages.

Severity: remediation recommended | Category: observability

How to fix: Only mention fallback when used

Agent prompt to fix - you can give this to your LLM of choice:

Issue description

FileTypeValidator.buildErrorMessage() adds a suffix claiming magic-number detection failed and a mimetype fallback was used whenever fallbackToMimetype is enabled. This is misleading because fallback is only used when file-type detection returns no type (and possibly should be used on exceptions too), not when detection succeeds but the detected mime doesn’t match.

Issue Context

ParseFilePipe calls buildErrorMessage(file) after isValid() returns false and does not provide information about which branch failed.

Fix Focus Areas

  • packages/common/pipes/file/file-type.validator.ts[34-55]
  • packages/common/pipes/file/parse-file.pipe.ts[68-82]

Suggested change

Adjust error message logic so the fallback suffix is emitted only when fallback was actually attempted/used. Options:

  • Track validation mode in FileTypeValidator (e.g., store a lastFailureReason on the instance during isValid()), and consult it in buildErrorMessage().
  • Or keep buildErrorMessage() generic and remove the fallback-specific suffix to avoid incorrect claims.

Add/adjust tests to cover both: (a) detected type mismatch and (b) undetectable type with fallback enabled.


Found by Qodo code review

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants