Repository navigation
fix(blob): strip JPEG metadata at the upload choke point - #1549
Merged
Merged
Conversation
BlobStorageController.upload sent whatever bytes it was given. Uploads were clean only because every current caller goes through ImageUploadPreparer, whose Bitmap.compress re-encode writes no EXIF. A caller that uploads a picked file directly would send its GPS coordinates along with it. Port iOS's JPEGMetadata walker and run it in upload() before initiateExternalUpload, so the reserved size is the size of the bytes actually sent. It keeps APP0 (JFIF), APP2 (ICC) and APP14 (Adobe) and drops every other APPn, EXIF and XMP included, plus COM segments. Input that isn't a JPEG, or whose segment lengths overrun the data, passes through untouched. The iOS JPEGMetadataTests cases are ported one for one with the same fixtures, so the two walkers are checked against the same inputs.
github-actions
Bot
added
area: network
…tension The byte helpers in services/flipcash are top-level internal extensions (ByteArray.toHash(), ByteArray.toPublicKey()), not objects. Replace the public JPEGMetadata object with an internal ByteArray.withoutJpegMetadata() in internal/extensions, next to them. The walker's behaviour is unchanged.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
BlobStorageController.uploadsent whatever bytes it was given. Today's uploads are clean only because the profile photo, create-group and edit-group screens all go throughImageUploadPreparer, and itsBitmap.compressre-encode writes no EXIF. An upload path that reads a picked file directly would send the photo's GPS coordinates with it.iOS strips at its choke point:
BlobUploader.storerunsJPEGMetadata.strippedbefore reserving. This PR ports that walker to Kotlin and calls it inupload()the same way.ByteArray.withoutJpegMetadata(): an internal extension inservices/flipcash'sinternal/extensions, alongsideByteArray.toHash()and the other byte helpers, with no Android types. It keeps APP0 (JFIF), APP2 (ICC) and APP14 (Adobe) and drops every other APPn, EXIF and XMP included, plus COM segments. It handles fill bytes and the standalone TEM/RSTn markers, and stops at SOS. Input that isn't a JPEG, or whose segment lengths overrun the data, passes through untouched.BlobStorageController.upload: strips beforeinitiateExternalUpload. The reservation declares a byte count, so the reserved size has to match the size of the bytes actually sent.JPEGMetadataTestscases inJpegMetadataTest, ported with the same fixtures, plus aBlobStorageControllerTestcase asserting that a JPEG carrying EXIF is uploaded stripped and that the reserved size matches the stripped length.ImageUploadPrepareris unchanged; its re-encode still does the resizing.