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 bug in media upload, add optional detectoin of mime-/file-type #8

Merged
merged 1 commit into from
Dec 26, 2022

Conversation

dscottboggs
Copy link
Owner

A bug was found where the client didn't include any file-type information with media uploads. This change fixes that by making two changes:

  1. The path to the file is included in the form now. This is enough to indicate to the Mastodon server what kind of file it is, and results in a 200 Ok response.
  2. With the optional, default "magic" feature, the file will be inspected to determine what kind of file it is, and the mime-type is included in the form. This may help with debugging and providing more robust error messages in the case where a file's extension is misinterpreted or different from the actual filetype, and could help the server better understand what to do with this file. To turn off this feature, disable the "magic" default feature.

This will resolve #6 . Thanks to @ckruse for reporting this bug.

@dscottboggs dscottboggs merged commit a1e12dc into main Dec 26, 2022
@ckruse
Copy link

ckruse commented Dec 26, 2022

Thank you very much for your work! ❤️

Comment on lines +38 to +40
/// A handle to access libmagic for mime-types.
#[cfg(feature = "magic")]
magic: magic::Cookie,
Copy link
Contributor

Choose a reason for hiding this comment

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

This was a semver-incompatible change as reported in #12 because the feature is a default feature.

Please roll back the change or make the feature optional!

Copy link
Owner Author

Choose a reason for hiding this comment

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

Apparently, I'm unaware of the implications of features on semver, what sort of release is required for a new default feature which is not exposed as an API to the user? I was under the impression SemVer was as simple as

  • Major: breaking changes
  • Minor: new features (in the sense of an additional function or set of functions which are exposed)
  • Patch: Bug-fixes and changes which don't affect the API

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, you're right about the semver rules. But this change introduced an API change and was released as a patchlevel release!

@dscottboggs dscottboggs deleted the fix/media-upload branch May 7, 2024 17:40
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.

400 bad request with media and media_with_thumbnail methods
3 participants