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

Remove exif info from image uploads #352

Merged
merged 7 commits into from
Sep 23, 2023
Merged

Conversation

zeerooth
Copy link
Collaborator

Resolves #263

Supports jpeg, png and webp (just like img-parts)

Some things to still consider:

  • Unfortunately, to parse the image and manipulate it we have to load it whole into the memory, but I don't think there's any way around that, especially when looking at currently available libraries.
  • Should we lock this behaviour behind a feature/config option? I personally think it's best not to make it opt-out.
  • We don't know what the file size is going to be in the multipart upload, so I use BytesMut without any capacity, which means it has to reallocate when receiving new stream chunks. This is unfortunate but again, I don't think there's a way around that aspect too.

@zeerooth zeerooth requested a review from aumetra September 21, 2023 17:09
Copy link
Member

@aumetra aumetra left a comment

Choose a reason for hiding this comment

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

just some small nits

@aumetra
Copy link
Member

aumetra commented Sep 21, 2023

this looks fine, I would keep it as something that is just forced on the users. I don't like the idea that an instance could accidentally let users post their geolocation.

And yeah, it's not possible to do without buffering with current crates unfortunately. I wanna check for better alternatives but until then I would just add this

Copy link
Member

@aumetra aumetra left a comment

Choose a reason for hiding this comment

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

Looks good now, thanks!

@aumetra aumetra enabled auto-merge September 23, 2023 13:37
@aumetra aumetra added this pull request to the merge queue Sep 23, 2023
Merged via the queue into kitsune-soc:main with commit 0a0da1d Sep 23, 2023
4 checks passed
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.

Add compatibility with an exif remover tool of some kind.
3 participants