Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Catch preflight errors on file upload #212

Conversation

mertkirbuga
Copy link
Contributor

FT-6c8dae2f-08cf-4ed6-a119-d15c8d538fd0

  • I have added automatic tests where applicable
  • The PR title is suitable as a release note
  • The PR contains a description of what has been changed
  • The description contains manual test instructions

Changes

Includes changes related with catching errors on file upload on preflight phase.

Please also follow up https://github.com/ftrackhq/ftrack-server/pull/750

Test

  • Try to upload one of the unallowed files and observe that snackbar on the top is displayed.

@mertkirbuga mertkirbuga requested a review from a team as a code owner February 28, 2024 15:35
if (this.onError) {
this.onError(error as Error);
}
return;
Copy link
Contributor

Choose a reason for hiding this comment

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

We should probably retain the previous behavior where start throw:ed if the preflight failed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Suggested change
return;
if (this.onError) {
this.onError(error as Error);
}
throw error;
image

When I throw an error like above instead of return, I receive this error message instead of notifyError. Is there any way to prevent this?

@mertkirbuga mertkirbuga merged commit c535587 into main Mar 8, 2024
5 checks passed
@mertkirbuga mertkirbuga deleted the backlog/attackers-can-upload-any-malicious-file-to-notes-attachments branch March 8, 2024 10:26
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.

2 participants