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: better handle extensions for unrecognized mimetypes #126

Merged
merged 2 commits into from
Jun 6, 2024

Conversation

mistydemeo
Copy link
Contributor

Previously we'd error out if we couldn't decide on a filename for the file. Now, if we don't assign an extension based on the mimetype, we'll simply return the original filename unmodified.

cf these custom versions of RemoteAsset::copy - we'll want to make sure this does what we want before merging. https://github.com/axodotdev/cargo-dist/blob/66b5ffaaee88a375c69f1c655ac26b0261cb20f3/cargo-dist/src/sign/ssldotcom.rs#L203

Copy link
Member

@ashleygwilliams ashleygwilliams left a comment

Choose a reason for hiding this comment

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

so the main issue with this is that if there is no extension it won't work locally, a URL can deliver a file but not actually have an extension. that might be too paranoid and we should just take this but that's the main reason this is like this

@mistydemeo
Copy link
Contributor Author

The usecase here was ::copy, where the lack of a file extension wasn't an issue; we were assigning it a filename anyway. I think it's fine for us to download something without a known file extension so long as we can trust the caller to know what to do with it.

Previously we'd error out if we couldn't decide on a filename for the
file. Now, if we don't assign an extension based on the mimetype, we'll
simply return the original filename unmodified.
@mistydemeo mistydemeo force-pushed the remote_asset_copy_zip branch from 4cc37be to 174418b Compare May 22, 2024 23:49
@mistydemeo mistydemeo merged commit fc00926 into main Jun 6, 2024
14 checks passed
@mistydemeo mistydemeo deleted the remote_asset_copy_zip branch June 6, 2024 18:33
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.

None yet

3 participants