Skip to content

Commit 18aa452

Browse files
Cole Huntleyfacebook-github-bot
authored andcommitted
Fix Image.getSize failing for data: URIs on Android
Summary: Fixes #57787. Supersedes #57788. ## Context On Android, `Image.getSize()` and `Image.getSizeWithHeaders()` reject for every `data:` URI. Both resolve dimensions through Fresco's encoded-image pipeline. That pipeline has producer sequences only for network, local-file and local-content URIs; every other scheme falls through to a throw: Unsupported uri scheme for encoded image fetch! Uri is: data:image/jpg;base64,... Fresco's decoded-image pipeline does handle the scheme (`SOURCE_TYPE_DATA -> dataFetchSequence`), so the capability exists — the encoded entry point simply does not expose it. A previous change added a fast path for `res://` URIs for exactly this reason; `data:` was never given one. Any app that gates rendering on `getSize` therefore cannot display an inline base64 image on Android at all, while iOS is unaffected. ## This Diff - Adds a `data:` fast path to `getSize` and `getSizeWithHeaders`, mirroring the existing resource-drawable fast path, routed through `fetchDecodedImage` rather than `fetchEncodedImage`. - Pins the request to auto-rotate so it produces the same Fresco bitmap cache key that `ReactImageView` builds for the same URI. Rotation options are part of that key, so a mismatch here would decode into a key nothing reads and force a second decode at render time. - Sets `DownsampleMode.NEVER` so the reported dimensions stay intrinsic rather than post-downsample, preserving the behaviour the encoded path was originally adopted for. That option is absent from the bitmap cache key, so it does not disturb the parity above. - Reads the visible dimensions straight off the decoded image, which has already had its EXIF rotation applied, rather than repeating the axis swap the encoded subscriber performs by hand. `BaseCloseableStaticBitmap` applies that same predicate internally, so repeating it here would double-apply it. - Adds unit coverage for the `data:` scheme, which had none: decoded-pipeline routing, cache-key parity with `ReactImageView`, intrinsic dimensions, the headers variant, and both failure paths. - Adds two `data:` rows to the RNTester `Image.getSize` platform test — one plain, one tagged EXIF orientation 6 — so a real decode, rather than a mocked pipeline, proves the reported dimensions are the visible ones. Both are inline base64, so they need no network. Because the decode now warms the bitmap memory cache under the key the `<Image>` subsequently reads, a caller that sets its image source from the `getSize` success callback paints from cache instead of decoding at render time. ## Alternatives Considered **Parse the dimensions out of the base64 header.** Satisfies the `getSize` contract and is cheaper, but warms nothing. Callers that relied on the decode side effect for smooth playback would keep re-decoding every frame at render time — it fixes the rejection without fixing the regression that accompanied it. **Add a `data:` arm to Fresco's encoded producer sequence.** Pushes a behavioural change into shared image infrastructure used by every app, to serve a caller that wants a decoded result anyway. The narrower fix belongs on this side. **Have callers stop gating rendering on `getSize`.** Makes the image appear, but permanently discards the decode-then-render ordering, leaving the surface visibly flickering. Changelog: [Android][Fixed] - Fix `Image.getSize()` and `Image.getSizeWithHeaders()` rejecting `data:` URIs Reviewed By: Abbondanzo, javache, cortinico Differential Revision: D118657320
1 parent 08c7781 commit 18aa452

3 files changed

Lines changed: 275 additions & 2 deletions

File tree

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/modules/image/ImageLoaderModule.kt

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,13 +13,16 @@ import android.util.SparseArray
1313
import com.facebook.common.executors.CallerThreadExecutor
1414
import com.facebook.common.memory.PooledByteBuffer
1515
import com.facebook.common.references.CloseableReference
16+
import com.facebook.common.util.UriUtil
1617
import com.facebook.datasource.BaseDataSubscriber
1718
import com.facebook.datasource.DataSource
1819
import com.facebook.datasource.DataSubscriber
1920
import com.facebook.drawee.backends.pipeline.Fresco
2021
import com.facebook.fbreact.specs.NativeImageLoaderAndroidSpec
2122
import com.facebook.imagepipeline.common.RotationOptions
23+
import com.facebook.imagepipeline.core.DownsampleMode
2224
import com.facebook.imagepipeline.core.ImagePipeline
25+
import com.facebook.imagepipeline.image.CloseableImage
2326
import com.facebook.imagepipeline.image.EncodedImage
2427
import com.facebook.imagepipeline.request.ImageRequest
2528
import com.facebook.imagepipeline.request.ImageRequestBuilder
@@ -93,6 +96,11 @@ internal class ImageLoaderModule : NativeImageLoaderAndroidSpec, LifecycleEventL
9396
resolveResourceSize(uriString, promise)
9497
return
9598
}
99+
// Fast path: data: URIs are served by the decoded pipeline; see resolveDataUriSize.
100+
if (UriUtil.isDataUri(source.uri)) {
101+
resolveDataUriSize(source.uri, promise)
102+
return
103+
}
96104
val request: ImageRequest =
97105
ImageRequestBuilder.newBuilderWithSource(source.uri)
98106
.setRotationOptions(RotationOptions.disableRotation())
@@ -122,6 +130,11 @@ internal class ImageLoaderModule : NativeImageLoaderAndroidSpec, LifecycleEventL
122130
resolveResourceSize(uriString, promise)
123131
return
124132
}
133+
// Fast path: a data: URI carries its own bytes, so headers are not applicable.
134+
if (UriUtil.isDataUri(source.uri)) {
135+
resolveDataUriSize(source.uri, promise)
136+
return
137+
}
125138
val imageRequestBuilder: ImageRequestBuilder =
126139
ImageRequestBuilder.newBuilderWithSource(source.uri)
127140
.setRotationOptions(RotationOptions.disableRotation())
@@ -180,6 +193,67 @@ internal class ImageLoaderModule : NativeImageLoaderAndroidSpec, LifecycleEventL
180193
}
181194
}
182195

196+
/**
197+
* Resolve the intrinsic size of a `data:` URI: Fresco's encoded-image pipeline has no producer
198+
* sequence for the scheme and throws, while its decoded pipeline has one.
199+
*
200+
* Both request options are load-bearing. `autoRotate` matches
201+
* [com.facebook.react.views.image.ReactImageView], so the decode warms the bitmap cache under the
202+
* key that view later reads — `disableRotation` would warm a dead one. `DownsampleMode.NEVER`
203+
* keeps the reported size intrinsic, at the cost of caching a larger bitmap than that view alone
204+
* would decode above `maxBitmapDimension`.
205+
*/
206+
private fun resolveDataUriSize(uri: Uri, promise: Promise) {
207+
val request =
208+
ImageRequestBuilder.newBuilderWithSource(uri)
209+
.setRotationOptions(RotationOptions.autoRotate())
210+
.setDownsampleOverride(DownsampleMode.NEVER)
211+
.build()
212+
val dataSource = imagePipeline.fetchDecodedImage(request, callerContext)
213+
dataSource.subscribe(createDecodedSizeSubscriber(promise), CallerThreadExecutor.getInstance())
214+
}
215+
216+
private fun createDecodedSizeSubscriber(
217+
promise: Promise,
218+
): DataSubscriber<CloseableReference<CloseableImage>> =
219+
object : BaseDataSubscriber<CloseableReference<CloseableImage>>() {
220+
override fun onNewResultImpl(dataSource: DataSource<CloseableReference<CloseableImage>>) {
221+
if (!dataSource.isFinished) {
222+
return
223+
}
224+
val ref = dataSource.result
225+
if (ref == null) {
226+
promise.reject(ERROR_GET_SIZE_FAILURE, "Failed to get the size of the image")
227+
return
228+
}
229+
try {
230+
// Already the visible dimensions: the decode applied the EXIF rotation, which is why
231+
// this does not repeat the axis swap the encoded subscriber has to do by hand.
232+
val image = ref.get()
233+
val width = image.width
234+
val height = image.height
235+
if (width < 0 || height < 0) {
236+
promise.reject(ERROR_GET_SIZE_FAILURE, "Failed to get the size of the image")
237+
return
238+
}
239+
promise.resolve(
240+
buildReadableMap {
241+
put("width", width)
242+
put("height", height)
243+
},
244+
)
245+
} catch (e: Exception) {
246+
promise.reject(ERROR_GET_SIZE_FAILURE, e)
247+
} finally {
248+
CloseableReference.closeSafely(ref)
249+
}
250+
}
251+
252+
override fun onFailureImpl(dataSource: DataSource<CloseableReference<CloseableImage>>) {
253+
promise.reject(ERROR_GET_SIZE_FAILURE, dataSource.failureCause)
254+
}
255+
}
256+
183257
/**
184258
* Resolve the intrinsic size of a drawable resource by name. Works for all drawable types
185259
* including VectorDrawable, which cannot be decoded by Fresco's encoded-image pipeline.

packages/react-native/ReactAndroid/src/test/java/com/facebook/react/modules/image/ImageLoaderModuleTest.kt

Lines changed: 170 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,23 @@ package com.facebook.react.modules.image
99

1010
import android.content.res.Resources
1111
import android.graphics.drawable.Drawable
12+
import androidx.core.net.toUri
13+
import com.facebook.common.references.CloseableReference
14+
import com.facebook.datasource.DataSource
15+
import com.facebook.datasource.DataSubscriber
16+
import com.facebook.imagepipeline.cache.DefaultCacheKeyFactory
17+
import com.facebook.imagepipeline.common.RotationOptions
18+
import com.facebook.imagepipeline.core.DownsampleMode
19+
import com.facebook.imagepipeline.core.ImagePipeline
20+
import com.facebook.imagepipeline.image.CloseableImage
21+
import com.facebook.imagepipeline.request.ImageRequest
22+
import com.facebook.imagepipeline.request.ImageRequestBuilder
1223
import com.facebook.react.bridge.Promise
24+
import com.facebook.react.bridge.ReactApplicationContext
1325
import com.facebook.react.bridge.ReactTestHelper
1426
import com.facebook.react.bridge.ReadableMap
1527
import com.facebook.react.bridge.WritableMap
28+
import com.facebook.react.views.image.ReactCallerContextFactory
1629
import com.facebook.react.views.imagehelper.ResourceDrawableIdHelper
1730
import com.facebook.testutils.shadows.ShadowArguments
1831
import com.facebook.testutils.shadows.ShadowSoLoader
@@ -24,22 +37,30 @@ import org.junit.runner.RunWith
2437
import org.mockito.MockedStatic
2538
import org.mockito.Mockito.mockStatic
2639
import org.mockito.kotlin.any
40+
import org.mockito.kotlin.anyOrNull
41+
import org.mockito.kotlin.argumentCaptor
2742
import org.mockito.kotlin.eq
2843
import org.mockito.kotlin.mock
44+
import org.mockito.kotlin.never
45+
import org.mockito.kotlin.verify
2946
import org.mockito.kotlin.whenever
3047
import org.robolectric.RobolectricTestRunner
3148
import org.robolectric.annotation.Config
3249

50+
/** The payload is never decoded — every test here mocks the image pipeline. */
51+
private const val DATA_URI = "data:image/jpg;base64,/9j/4AAQSkZJRgABAQAAAQABAAD/2wBDAA=="
52+
3353
@Config(shadows = [ShadowArguments::class, ShadowSoLoader::class])
3454
@RunWith(RobolectricTestRunner::class)
3555
class ImageLoaderModuleTest {
3656

3757
private lateinit var imageLoaderModule: ImageLoaderModule
3858
private lateinit var mockedHelper: MockedStatic<ResourceDrawableIdHelper>
59+
private lateinit var reactContext: ReactApplicationContext
3960

4061
@Before
4162
fun setUp() {
42-
val reactContext = ReactTestHelper.createCatalystContextForTest()
63+
reactContext = ReactTestHelper.createCatalystContextForTest()
4364
imageLoaderModule = ImageLoaderModule(reactContext)
4465

4566
mockedHelper = mockStatic(ResourceDrawableIdHelper::class.java)
@@ -197,6 +218,154 @@ class ImageLoaderModuleTest {
197218
assertThat(promise.errorCode).isEqualTo("E_INVALID_URI")
198219
}
199220

221+
@Test
222+
fun testGetSizeWithDataUriUsesDecodedPipeline() {
223+
val pipeline = mock<ImagePipeline>()
224+
val dataSource = finishedDataSource(decodedRef(1408, 1408))
225+
whenever(pipeline.fetchDecodedImage(anyOrNull(), anyOrNull())).thenReturn(dataSource)
226+
227+
val promise = SimplePromise()
228+
moduleWithPipeline(pipeline).getSize(DATA_URI, promise)
229+
captureSubscriber(dataSource).onNewResult(dataSource)
230+
231+
// The encoded pipeline has no producer sequence for data: URIs and throws for them, so routing
232+
// there at all is the regression this guards.
233+
verify(pipeline, never()).fetchEncodedImage(any(), anyOrNull())
234+
assertThat(promise.rejected).isEqualTo(0)
235+
assertThat(promise.resolved).isEqualTo(1)
236+
val result = promise.value as ReadableMap
237+
assertThat(result.getInt("width")).isEqualTo(1408)
238+
assertThat(result.getInt("height")).isEqualTo(1408)
239+
}
240+
241+
@Test
242+
fun testGetSizeWithHeadersWithDataUriUsesDecodedPipeline() {
243+
val pipeline = mock<ImagePipeline>()
244+
val dataSource = finishedDataSource(decodedRef(640, 480))
245+
whenever(pipeline.fetchDecodedImage(anyOrNull(), anyOrNull())).thenReturn(dataSource)
246+
247+
val promise = SimplePromise()
248+
moduleWithPipeline(pipeline).getSizeWithHeaders(DATA_URI, null, promise)
249+
captureSubscriber(dataSource).onNewResult(dataSource)
250+
251+
verify(pipeline, never()).fetchEncodedImage(any(), anyOrNull())
252+
assertThat(promise.rejected).isEqualTo(0)
253+
val result = promise.value as ReadableMap
254+
assertThat(result.getInt("width")).isEqualTo(640)
255+
assertThat(result.getInt("height")).isEqualTo(480)
256+
}
257+
258+
/**
259+
* The decode is only worth doing if it lands under the key `ReactImageView` later reads. Rotation
260+
* options are part of the bitmap cache key, so `disableRotation` here would warm a key nothing
261+
* looks up and every rendered frame would decode a second time.
262+
*/
263+
@Test
264+
fun testGetSizeWithDataUriSharesBitmapCacheKeyWithReactImageView() {
265+
val pipeline = mock<ImagePipeline>()
266+
val dataSource = finishedDataSource(decodedRef(1408, 1408))
267+
whenever(pipeline.fetchDecodedImage(anyOrNull(), anyOrNull())).thenReturn(dataSource)
268+
269+
val promise = SimplePromise()
270+
moduleWithPipeline(pipeline).getSize(DATA_URI, promise)
271+
272+
val requestCaptor = argumentCaptor<ImageRequest>()
273+
verify(pipeline).fetchDecodedImage(requestCaptor.capture(), anyOrNull())
274+
275+
// Mirrors ReactImageView.maybeUpdateViewFromRequest for a data: URI: shouldResize() is false
276+
// for a non-file, non-content URI under the default resize method, so resize options are null.
277+
// ReactImageView spells the rotation as the deprecated setAutoRotateEnabled(true), which
278+
// delegates to exactly this, so the request it builds — and its cache key — is unchanged.
279+
val reactImageViewRequest =
280+
ImageRequestBuilder.newBuilderWithSource(DATA_URI.toUri())
281+
.setRotationOptions(RotationOptions.autoRotate())
282+
.setProgressiveRenderingEnabled(false)
283+
.build()
284+
285+
val cacheKeyFactory = DefaultCacheKeyFactory.getInstance()
286+
assertThat(cacheKeyFactory.getBitmapCacheKey(requestCaptor.firstValue, null))
287+
.isEqualTo(cacheKeyFactory.getBitmapCacheKey(reactImageViewRequest, null))
288+
}
289+
290+
/**
291+
* Guards the intent of the change that moved getSize off the decoded pipeline in the first place:
292+
* the reported dimensions must be intrinsic, not post-downsample.
293+
*/
294+
@Test
295+
fun testGetSizeWithDataUriRequestsAnUndownsampledDecode() {
296+
val pipeline = mock<ImagePipeline>()
297+
val dataSource = finishedDataSource(decodedRef(1408, 1408))
298+
whenever(pipeline.fetchDecodedImage(anyOrNull(), anyOrNull())).thenReturn(dataSource)
299+
300+
moduleWithPipeline(pipeline).getSize(DATA_URI, SimplePromise())
301+
302+
val requestCaptor = argumentCaptor<ImageRequest>()
303+
verify(pipeline).fetchDecodedImage(requestCaptor.capture(), anyOrNull())
304+
val request = requestCaptor.firstValue
305+
assertThat(request.downsampleOverride).isEqualTo(DownsampleMode.NEVER)
306+
assertThat(request.resizeOptions).isNull()
307+
assertThat(request.rotationOptions).isEqualTo(RotationOptions.autoRotate())
308+
}
309+
310+
@Test
311+
fun testGetSizeWithDataUriRejectsWhenDecodeFails() {
312+
val pipeline = mock<ImagePipeline>()
313+
val dataSource = mock<DataSource<CloseableReference<CloseableImage>>>()
314+
whenever(dataSource.failureCause).thenReturn(RuntimeException("decode failed"))
315+
whenever(pipeline.fetchDecodedImage(anyOrNull(), anyOrNull())).thenReturn(dataSource)
316+
317+
val promise = SimplePromise()
318+
moduleWithPipeline(pipeline).getSize(DATA_URI, promise)
319+
captureSubscriber(dataSource).onFailure(dataSource)
320+
321+
assertThat(promise.resolved).isEqualTo(0)
322+
assertThat(promise.rejected).isEqualTo(1)
323+
assertThat(promise.errorCode).isEqualTo("E_GET_SIZE_FAILURE")
324+
assertThat(promise.errorMessage).contains("decode failed")
325+
}
326+
327+
@Test
328+
fun testGetSizeWithDataUriRejectsWhenDecodeYieldsNoResult() {
329+
val pipeline = mock<ImagePipeline>()
330+
val dataSource = finishedDataSource(null)
331+
whenever(pipeline.fetchDecodedImage(anyOrNull(), anyOrNull())).thenReturn(dataSource)
332+
333+
val promise = SimplePromise()
334+
moduleWithPipeline(pipeline).getSize(DATA_URI, promise)
335+
captureSubscriber(dataSource).onNewResult(dataSource)
336+
337+
assertThat(promise.resolved).isEqualTo(0)
338+
assertThat(promise.rejected).isEqualTo(1)
339+
assertThat(promise.errorCode).isEqualTo("E_GET_SIZE_FAILURE")
340+
}
341+
342+
private fun moduleWithPipeline(pipeline: ImagePipeline): ImageLoaderModule =
343+
ImageLoaderModule(reactContext, pipeline, mock<ReactCallerContextFactory>())
344+
345+
private fun decodedRef(width: Int, height: Int): CloseableReference<CloseableImage> {
346+
val image = mock<CloseableImage>()
347+
whenever(image.width).thenReturn(width)
348+
whenever(image.height).thenReturn(height)
349+
return CloseableReference.of(image)
350+
}
351+
352+
private fun finishedDataSource(
353+
result: CloseableReference<CloseableImage>?,
354+
): DataSource<CloseableReference<CloseableImage>> {
355+
val dataSource = mock<DataSource<CloseableReference<CloseableImage>>>()
356+
whenever(dataSource.isFinished).thenReturn(true)
357+
whenever(dataSource.result).thenReturn(result)
358+
return dataSource
359+
}
360+
361+
private fun captureSubscriber(
362+
dataSource: DataSource<CloseableReference<CloseableImage>>,
363+
): DataSubscriber<CloseableReference<CloseableImage>> {
364+
val captor = argumentCaptor<DataSubscriber<CloseableReference<CloseableImage>>>()
365+
verify(dataSource).subscribe(captor.capture(), any())
366+
return captor.firstValue
367+
}
368+
200369
internal class SimplePromise : Promise {
201370
companion object {
202371
private const val ERROR_DEFAULT_CODE = "EUNSPECIFIED"

packages/rn-tester/js/examples/Image/ImageExample.js

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -809,6 +809,22 @@ const smallImage = {
809809
uri: IMAGE1,
810810
};
811811

812+
// 16x8 JPEG with no EXIF orientation, inline so the check needs no network.
813+
const GET_SIZE_JPEG_DATA_URI =
814+
'data:image/jpeg;base64,/9j/4AAQSkZJRgABAQAAAQABAAD/2wBDABQODxIPDRQSEBIXFRQYHjIhHhwcHj0sLiQySUBMS0dARkVQWnNiU' +
815+
'FVtVkVGZIhlbXd7gYKBTmCNl4x9lnN+gXz/2wBDARUXFx4aHjshITt8U0ZTfHx8fHx8fHx8fHx8fHx8fHx8fHx8fHx8fHx8fHx8fHx8fHx8f' +
816+
'Hx8fHx8fHx8fHx8fHz/wAARCAAIABADASIAAhEBAxEB/8QAFQABAQAAAAAAAAAAAAAAAAAAAAT/xAAWEAEBAQAAAAAAAAAAAAAAAAAAFWL/x' +
817+
'AAVAQEBAAAAAAAAAAAAAAAAAAAFBv/EABcRAAMBAAAAAAAAAAAAAAAAAAADFFH/2gAMAwEAAhEDEQA/AKYuSLkExQzRSpun/9k=';
818+
819+
// The same 16x8 raster tagged EXIF orientation 6 (rotate 90 CW to display), so a
820+
// decoder that honours the tag reports 8x16 and one that ignores it reports 16x8.
821+
const GET_SIZE_EXIF_ROTATED_JPEG_DATA_URI =
822+
'data:image/jpeg;base64,/9j/4QAiRXhpZgAASUkqAAgAAAABABIBAwABAAAABgAAAAAAAAD/4AAQSkZJRgABAQAAAQABAAD/2wBDABQOD' +
823+
'xIPDRQSEBIXFRQYHjIhHhwcHj0sLiQySUBMS0dARkVQWnNiUFVtVkVGZIhlbXd7gYKBTmCNl4x9lnN+gXz/2wBDARUXFx4aHjshITt8U0ZTf' +
824+
'Hx8fHx8fHx8fHx8fHx8fHx8fHx8fHx8fHx8fHx8fHx8fHx8fHx8fHx8fHx8fHx8fHz/wAARCAAIABADASIAAhEBAxEB/8QAFQABAQAAAAAAA' +
825+
'AAAAAAAAAAAAAT/xAAWEAEBAQAAAAAAAAAAAAAAAAAAFWL/xAAVAQEBAAAAAAAAAAAAAAAAAAAFBv/EABcRAAMBAAAAAAAAAAAAAAAAAAADF' +
826+
'FH/2gAMAwEAAhEDEQA/AJYuSLkDVDNJqpun/9k=';
827+
812828
const GET_SIZE_TEST_IMAGES: ReadonlyArray<{
813829
expectedHeight: number,
814830
expectedWidth: number,
@@ -834,6 +850,20 @@ const GET_SIZE_TEST_IMAGES: ReadonlyArray<{
834850
uri: 'https://www.facebook.com/assets/react_native_oss_tests/exif-6@1x.jpg',
835851
name: 'EXIF rotated JPEG',
836852
},
853+
{
854+
expectedHeight: 8,
855+
expectedWidth: 16,
856+
uri: GET_SIZE_JPEG_DATA_URI,
857+
name: 'data: URI JPEG',
858+
},
859+
{
860+
// Transposed against the row above: the tag must be applied, so these are
861+
// the visible dimensions rather than the 16x8 stored raster.
862+
expectedHeight: 16,
863+
expectedWidth: 8,
864+
uri: GET_SIZE_EXIF_ROTATED_JPEG_DATA_URI,
865+
name: 'EXIF rotated data: URI JPEG',
866+
},
837867
];
838868

839869
function getImageSize(uri: string): Promise<{height: number, width: number}> {
@@ -897,7 +927,7 @@ function ImageGetSizePlatformTest(props: PlatformTestComponentBaseProps) {
897927

898928
return (
899929
<RNTesterText>
900-
Calling Image.getSize for {GET_SIZE_TEST_IMAGES.length} remote images.
930+
Calling Image.getSize for {GET_SIZE_TEST_IMAGES.length} images.
901931
</RNTesterText>
902932
);
903933
}

0 commit comments

Comments
 (0)