-
Notifications
You must be signed in to change notification settings - Fork 152
fix(mobile): Android image metadata stripping fixes #2188
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
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,55 @@ | ||
| # Android Bitmap media fixtures | ||
|
|
||
| These 3 x 2 fixtures were produced by Android 16 (API 36) `Bitmap.compress`, not by a generic image encoder. | ||
|
|
||
| ## Regeneration | ||
|
|
||
| 1. Compile and run the following program on an API 36 emulator: | ||
|
|
||
| ```java | ||
| import android.graphics.Bitmap; | ||
| import android.graphics.Color; | ||
| import android.graphics.ColorSpace; | ||
| import java.io.FileOutputStream; | ||
|
|
||
| public final class Main { | ||
| private static void write(Bitmap bitmap, Bitmap.CompressFormat format, String stem) | ||
| throws Exception { | ||
| String extension = format == Bitmap.CompressFormat.PNG ? "png" : "jpg"; | ||
| try (FileOutputStream output = | ||
| new FileOutputStream("/data/local/tmp/" + stem + "." + extension)) { | ||
| if (!bitmap.compress(format, 100, output)) { | ||
| throw new IllegalStateException("Bitmap.compress failed for " + stem); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| public static void main(String[] args) throws Exception { | ||
| Bitmap srgb = Bitmap.createBitmap(3, 2, Bitmap.Config.ARGB_8888); | ||
| srgb.setPixels(new int[] { | ||
| Color.argb(255, 255, 0, 0), Color.argb(255, 0, 255, 0), | ||
| Color.argb(255, 0, 0, 255), Color.argb(128, 255, 255, 0), | ||
| Color.argb(64, 0, 255, 255), Color.argb(0, 255, 0, 255), | ||
| }, 0, 3, 0, 0, 3, 2); | ||
| write(srgb, Bitmap.CompressFormat.PNG, "bitmap-srgb"); | ||
| write(srgb, Bitmap.CompressFormat.JPEG, "bitmap-srgb"); | ||
|
|
||
| Bitmap displayP3 = Bitmap.createBitmap( | ||
| 3, 2, Bitmap.Config.RGBA_F16, true, | ||
| ColorSpace.get(ColorSpace.Named.DISPLAY_P3)); | ||
| displayP3.eraseColor(Color.pack( | ||
| 1.0f, 0.0f, 0.0f, 1.0f, | ||
| ColorSpace.get(ColorSpace.Named.DISPLAY_P3))); | ||
| write(displayP3, Bitmap.CompressFormat.PNG, "bitmap-display-p3"); | ||
| write(displayP3, Bitmap.CompressFormat.JPEG, "bitmap-display-p3"); | ||
| } | ||
| } | ||
| ``` | ||
|
|
||
| 2. Pull the four files from `/data/local/tmp/` into this directory. Copy all four into `mobile/android/app/src/test/resources/fixtures/android/`, and copy `bitmap-display-p3.png` and `bitmap-srgb.png` into `mobile/android/app/src/androidTest/resources/fixtures/android/`. | ||
| 3. Run `cmp` on every copied fixture to confirm it is byte-identical. | ||
| 4. Run the app's `AndroidImageProcessor.decodeSrgbBitmap` and `encodeAndScrub` path for both source color spaces and both formats on the API 36 emulator. Save the four outputs under `sanitized/` with the `-sanitized` suffix. | ||
| 5. Run `cargo test -p buzz-media android_` to verify the relay accepts every sanitized fixture while rejecting the three unsanitized fixtures that contain forbidden metadata. | ||
| 6. Run `cd mobile/android && ./gradlew app:testDebugUnitTest app:connectedDebugAndroidTest` with an API 36 emulator to verify structural scrubbing and the Display-P3 to sRGB conversion for sanitized PNG and JPEG output. | ||
|
|
||
| Regenerate both the encoded inputs and sanitized outputs whenever Android `Bitmap.compress` behavior or `AndroidMediaSanitizer` changes. Do not update only the sanitized files, because the tests cover the exact encoder-to-sanitizer boundary. |
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added
BIN
+289 Bytes
crates/buzz-media/tests/fixtures/android/sanitized/bitmap-display-p3-sanitized.jpg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added
BIN
+101 Bytes
crates/buzz-media/tests/fixtures/android/sanitized/bitmap-display-p3-sanitized.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added
BIN
+376 Bytes
crates/buzz-media/tests/fixtures/android/sanitized/bitmap-srgb-sanitized.jpg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added
BIN
+112 Bytes
crates/buzz-media/tests/fixtures/android/sanitized/bitmap-srgb-sanitized.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
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
188 changes: 188 additions & 0 deletions
188
mobile/android/app/src/androidTest/kotlin/xyz/block/buzz/mobile/AndroidImageProcessorTest.kt
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,188 @@ | ||
| package xyz.block.buzz.mobile | ||
|
|
||
| import android.graphics.Bitmap | ||
| import android.graphics.BitmapFactory | ||
| import android.graphics.Color | ||
| import android.os.Build | ||
| import androidx.test.ext.junit.runners.AndroidJUnit4 | ||
| import org.junit.Test | ||
| import org.junit.runner.RunWith | ||
| import kotlin.math.abs | ||
| import kotlin.test.assertEquals | ||
| import kotlin.test.assertNotNull | ||
| import kotlin.test.assertTrue | ||
|
|
||
| @RunWith(AndroidJUnit4::class) | ||
| class AndroidImageProcessorTest { | ||
| companion object { | ||
| private val ALLOWED_PNG_CHUNKS = setOf( | ||
| "IHDR", "PLTE", "IDAT", "IEND", "cHRM", "gAMA", "sBIT", "sRGB", "bKGD", "hIST", "tRNS", "sPLT", | ||
| "acTL", "fcTL", "fdAT", | ||
| ) | ||
| } | ||
|
|
||
| @Test | ||
| fun supportedApiCanProcessImagesWithoutNewerPlatformApis() { | ||
| if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) return | ||
|
|
||
| val sourceBytes = fixtureBytes("bitmap-srgb.png") | ||
| val processed = assertNotNull(AndroidImageProcessor.decodeSrgbBitmap(sourceBytes)) | ||
| assertEquals(Bitmap.Config.ARGB_8888, processed.config) | ||
|
|
||
| val scrubbed = assertNotNull( | ||
| AndroidImageProcessor.encodeAndScrub(processed, Bitmap.CompressFormat.PNG), | ||
| ) | ||
| val chunkTypes = pngChunkTypes(scrubbed) | ||
| assertEquals("IHDR", chunkTypes.first()) | ||
| assertEquals("IEND", chunkTypes.last()) | ||
| assertTrue(chunkTypes.all { it in ALLOWED_PNG_CHUNKS }) | ||
| assertPngPixelsPreserved(sourceBytes, scrubbed) | ||
| } | ||
|
|
||
| @Test | ||
| fun transparentPngPixelsSurviveSrgbProcessing() { | ||
| val sourceBytes = fixtureBytes("bitmap-srgb.png") | ||
| val source = assertNotNull(BitmapFactory.decodeByteArray(sourceBytes, 0, sourceBytes.size)) | ||
| val processed = assertNotNull(AndroidImageProcessor.decodeSrgbBitmap(sourceBytes)) | ||
| assertTrue(processed.hasAlpha()) | ||
|
|
||
| val scrubbed = assertNotNull( | ||
| AndroidImageProcessor.encodeAndScrub(processed, Bitmap.CompressFormat.PNG), | ||
| ) | ||
| assertPngPixelsPreserved(sourceBytes, scrubbed) | ||
| assertEquals(128, Color.alpha(source.getPixel(0, 1))) | ||
| assertEquals(64, Color.alpha(source.getPixel(1, 1))) | ||
| assertEquals(0, Color.alpha(source.getPixel(2, 1))) | ||
| } | ||
|
|
||
| @Test | ||
| fun displayP3InputIsConvertedToSrgbBeforeEncodedMetadataIsScrubbed() { | ||
| if (Build.VERSION.SDK_INT < Build.VERSION_CODES.O) return | ||
|
|
||
| val sourceBytes = fixtureBytes("bitmap-display-p3.png") | ||
| val source = assertNotNull(BitmapFactory.decodeByteArray(sourceBytes, 0, sourceBytes.size)) | ||
| assertEquals(android.graphics.ColorSpace.get(android.graphics.ColorSpace.Named.DISPLAY_P3), source.colorSpace) | ||
| val expectedSrgbPixel = source.getColor(0, 0).convert( | ||
| android.graphics.ColorSpace.get(android.graphics.ColorSpace.Named.SRGB), | ||
| ) | ||
|
|
||
| val srgbBitmap = assertNotNull(AndroidImageProcessor.decodeSrgbBitmap(sourceBytes)) | ||
| assertEquals(android.graphics.ColorSpace.get(android.graphics.ColorSpace.Named.SRGB), srgbBitmap.colorSpace) | ||
| assertEquals(Bitmap.Config.ARGB_8888, srgbBitmap.config) | ||
| assertColorClose(expectedSrgbPixel, srgbBitmap.getColor(0, 0), tolerance = 1.0f / 255.0f) | ||
|
|
||
| for ((format, tolerance) in listOf( | ||
| Bitmap.CompressFormat.PNG to (1.0f / 255.0f), | ||
| Bitmap.CompressFormat.JPEG to (2.0f / 255.0f), | ||
| )) { | ||
| val scrubbed = assertNotNull(AndroidImageProcessor.encodeAndScrub(srgbBitmap, format)) | ||
| when (format) { | ||
| Bitmap.CompressFormat.PNG -> { | ||
| assertEquals(listOf("IHDR", "sRGB", "sBIT", "IDAT", "IEND"), pngChunkTypes(scrubbed)) | ||
| } | ||
| Bitmap.CompressFormat.JPEG -> { | ||
| assertEquals(listOf(0xE0), jpegMetadataMarkers(scrubbed)) | ||
| } | ||
| else -> error("Unexpected format: $format") | ||
| } | ||
|
|
||
| val decodedOutput = assertNotNull(BitmapFactory.decodeByteArray(scrubbed, 0, scrubbed.size)) | ||
| assertEquals( | ||
| android.graphics.ColorSpace.get(android.graphics.ColorSpace.Named.SRGB), | ||
| decodedOutput.colorSpace, | ||
| ) | ||
| assertColorClose(expectedSrgbPixel, decodedOutput.getColor(0, 0), tolerance) | ||
| } | ||
| } | ||
|
|
||
| private fun assertPngPixelsPreserved(expectedBytes: ByteArray, actualBytes: ByteArray) { | ||
| val expected = assertNotNull(BitmapFactory.decodeByteArray(expectedBytes, 0, expectedBytes.size)) | ||
| val actual = assertNotNull(BitmapFactory.decodeByteArray(actualBytes, 0, actualBytes.size)) | ||
| assertEquals(expected.width, actual.width) | ||
| assertEquals(expected.height, actual.height) | ||
| for (y in 0 until expected.height) { | ||
| for (x in 0 until expected.width) { | ||
| assertEquals( | ||
| expected.getPixel(x, y), | ||
| actual.getPixel(x, y), | ||
| "pixel ($x, $y)", | ||
| ) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| private fun fixtureBytes(name: String): ByteArray { | ||
| return requireNotNull(javaClass.getResourceAsStream("/fixtures/android/$name")) { | ||
| "Missing fixture: $name" | ||
| }.use { it.readBytes() } | ||
| } | ||
|
|
||
| private fun assertColorClose(expected: Color, actual: Color, tolerance: Float) { | ||
| val expectedComponents = expected.components | ||
| val actualComponents = actual.convert(expected.colorSpace).components | ||
| for (index in expectedComponents.indices) { | ||
| assertTrue( | ||
| abs(expectedComponents[index] - actualComponents[index]) <= tolerance, | ||
| "component $index: expected ${expectedComponents[index]}, got ${actualComponents[index]}", | ||
| ) | ||
| } | ||
| } | ||
|
|
||
| private fun pngChunkTypes(bytes: ByteArray): List<String> { | ||
| val result = mutableListOf<String>() | ||
| var offset = 8 | ||
| while (offset < bytes.size) { | ||
| require(bytes.size - offset >= 12) | ||
| val payloadLength = readUnsignedInt(bytes, offset) | ||
| val type = bytes.decodeToString(offset + 4, offset + 8) | ||
| result += type | ||
| offset += payloadLength + 12 | ||
| if (type == "IEND") { | ||
| assertEquals(bytes.size, offset) | ||
| return result | ||
| } | ||
| } | ||
| error("PNG is missing IEND") | ||
| } | ||
|
|
||
| private fun jpegMetadataMarkers(bytes: ByteArray): List<Int> { | ||
| val result = mutableListOf<Int>() | ||
| var offset = 2 | ||
| var inScan = false | ||
| while (offset < bytes.size) { | ||
| if (inScan && bytes[offset] != 0xFF.toByte()) { | ||
| offset += 1 | ||
| continue | ||
| } | ||
| require(bytes[offset] == 0xFF.toByte()) | ||
| while (offset < bytes.size && bytes[offset] == 0xFF.toByte()) { | ||
| offset += 1 | ||
| } | ||
| require(offset < bytes.size) | ||
| val marker = bytes[offset].toInt() and 0xFF | ||
| offset += 1 | ||
| if (inScan && marker == 0x00) continue | ||
| if (marker in 0xD0..0xD7 || marker == 0x01) continue | ||
| if (marker == 0xD9) { | ||
| assertEquals(bytes.size, offset) | ||
| return result | ||
| } | ||
| val segmentLength = readUnsignedShort(bytes, offset) | ||
| if (marker in 0xE0..0xEF || marker == 0xFE) result += marker | ||
| offset += segmentLength | ||
| inScan = marker == 0xDA | ||
| } | ||
| error("JPEG is missing EOI") | ||
| } | ||
|
|
||
| private fun readUnsignedShort(bytes: ByteArray, offset: Int): Int { | ||
| return ((bytes[offset].toInt() and 0xFF) shl 8) or (bytes[offset + 1].toInt() and 0xFF) | ||
| } | ||
|
|
||
| private fun readUnsignedInt(bytes: ByteArray, offset: Int): Int { | ||
| return ((bytes[offset].toInt() and 0xFF) shl 24) or | ||
| ((bytes[offset + 1].toInt() and 0xFF) shl 16) or | ||
| ((bytes[offset + 2].toInt() and 0xFF) shl 8) or | ||
| (bytes[offset + 3].toInt() and 0xFF) | ||
| } | ||
| } |
Binary file added
BIN
+407 Bytes
...le/android/app/src/androidTest/resources/fixtures/android/bitmap-display-p3.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added
BIN
+112 Bytes
mobile/android/app/src/androidTest/resources/fixtures/android/bitmap-srgb.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Oops, something went wrong.
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🤖 Non-blocking: the new JVM sanitizer tests (
app:testDebugUnitTest) are not executed by the CI mobile job, which runsflutter testandflutter build apkonly (assembleDebugdoes not run unit tests). The relay-side contract tests incrates/buzz-mediado run in CI and pin the actual byte contract, so this is coverage redundancy rather than a gap — but wiringtestDebugUnitTestinto the mobile CI job in a follow-up would keep the structural scrubber tests from ever going stale unnoticed.