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

Implement Enhanced Error Handling for Disallowed MIME Types #58

Merged
merged 4 commits into from
Sep 17, 2024

Conversation

TrevisGordan
Copy link
Contributor

@TrevisGordan TrevisGordan commented Apr 26, 2024

Overview

This Pull Request introduces several changes aimed at improving the clarity and accuracy of error messages related to MIME type restrictions. It addresses the issue described in #57. Where the error message and codes are misleading, suggesting security risks where there were none. The changes ensure that users can receive precise and non-misleading feedback when uploading files with disallowed MIME types.

Changes Made

  • Gitignore Updates: Added /build, /.vscode folders, and .pickle files to .gitignore to prevent unnecessary files from being tracked by Git. This keeps our repository clean and focused on relevant changes.

  • New Error Type: Introduced a new error class FileMimeTypeForbiddenError to specifically handle cases where the MIME type of the uploaded file is not allowed. This helps in clearly distinguishing between security-related file rejections and policy-based restrictions.

  • Error Handling in Scanner: Implemented the FileMimeTypeForbiddenError in the file scanning process. Now, if a file is uploaded with a MIME type that is not permitted, this specific error is raised, providing clear and accurate feedback to the user.

  • Test Modifications: Updated the tests related to MIME type errors to reflect the new error handling logic. This ensures our test suite remains robust and aligned with the current functionality.

  • Documentation Update: Added the new error code MCS_MEDIA_FORBIDDEN or MCS_MIMETYPE_FORBIDDEN to the API documentation. This update helps developers and users understand the potential errors they might encounter and the reasons behind them.

Wordings and namings are suggestions and up for discussion and potential revision. ;)

@anoadragon453 anoadragon453 requested a review from a team July 24, 2024 09:43
@devonh devonh linked an issue Jul 24, 2024 that may be closed by this pull request
Copy link
Member

@devonh devonh left a comment

Choose a reason for hiding this comment

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

Hey - Thanks for this! Your changes look good to me.
Not sure why CI isn't being run though.

@dklimpel
Copy link

@devonh

Not sure why CI isn't being run though.

Is this his first PR and the CI needs to be started by a team member.

@TrevisGordan
Copy link
Contributor Author

Hey @devonh,

It looks like there isn’t a CI set up for this repo. When I compared previous PR merges, they were manually approved and merged by the members. 👉👈

Let me know if you need any further adjustments!

@devonh devonh merged commit 5f99639 into element-hq:main Sep 17, 2024
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.

Misleading Error Message for Disallowed MIME Types
3 participants