Skip to content

don't report error on unknown mime type from S3 #2040

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

Merged
merged 1 commit into from
Feb 15, 2023

Conversation

syphar
Copy link
Member

@syphar syphar commented Feb 11, 2023

I saw this sentry error,

from (for example) serving http://docs.rs/crate/mach_object/0.1.3/source/test/helloworld.universal, which raises the error "could not parse mime from storage object".

IMO we don't need this error, defaulting to application/octet-stream is totally fine.

@github-actions github-actions bot added the S-waiting-on-review Status: This pull request has been implemented and needs to be reviewed label Feb 11, 2023
@syphar syphar force-pushed the sentry-web-file-mime branch from 3442bcc to 3582923 Compare February 11, 2023 16:26
@Nemo157
Copy link
Member

Nemo157 commented Feb 14, 2023

The question I have is how did an invalid mimetype get into S3, and I probably found the answer with a little more looking, it's an old crate built in 2017 back when https://docs.rs/magic was used for mimetype detection, I'm not surprised it may have put some invalid data into the database back then.

@Nemo157
Copy link
Member

Nemo157 commented Feb 14, 2023

(This does make me question our current default of text/plain when mime detection fails while adding a file though)

docs.rs/src/storage/mod.rs

Lines 549 to 551 in c927eac

let mime = mime_guess::from_path(file_path.as_ref())
.first_raw()
.unwrap_or("text/plain");

@syphar
Copy link
Member Author

syphar commented Feb 15, 2023

I moved both comments / questions into an issue: #2050

@syphar syphar merged commit f5359d4 into rust-lang:master Feb 15, 2023
@github-actions github-actions bot added S-waiting-on-deploy This PR is ready to be merged, but is waiting for an admin to have time to deploy it and removed S-waiting-on-review Status: This pull request has been implemented and needs to be reviewed labels Feb 15, 2023
@syphar syphar removed the S-waiting-on-deploy This PR is ready to be merged, but is waiting for an admin to have time to deploy it label Feb 18, 2023
@syphar syphar deleted the sentry-web-file-mime branch February 20, 2023 18:37
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