Check LayerDrawable bounds ourselves

DataDog · Sep 13, 2023 · 6e89a6d · 6e89a6d
1 parent 564979a
commit 6e89a6d
Show file tree

Hide file tree

Showing 3 changed files with 48 additions and 19 deletions.
diff --git a/...y/src/main/kotlin/com/datadog/android/sessionreplay/internal/recorder/LayerDrawableExt.kt b/...y/src/main/kotlin/com/datadog/android/sessionreplay/internal/recorder/LayerDrawableExt.kt
@@ -12,16 +12,14 @@ import com.datadog.android.api.InternalLogger
 
 @Suppress("TooGenericExceptionCaught")
 internal fun LayerDrawable.safeGetDrawable(index: Int, logger: InternalLogger = InternalLogger.UNBOUND): Drawable? {
-    return try {
-        this.getDrawable(index)
-    } catch (e: IndexOutOfBoundsException) {
-        // this should never happen
+    return if (index < 0 || index >= this.numberOfLayers) {
         logger.log(
             level = InternalLogger.Level.ERROR,
             target = InternalLogger.Target.MAINTAINER,
-            { "Failed to get drawable from layer" },
-            e
+            { "Failed to get drawable from layer - invalid index passed: $index" }
         )
         null
+    } else {
+        this.getDrawable(index)
     }
 }
diff --git a/...c/test/kotlin/com/datadog/android/sessionreplay/internal/recorder/LayerDrawableExtTest.kt b/...c/test/kotlin/com/datadog/android/sessionreplay/internal/recorder/LayerDrawableExtTest.kt
@@ -46,6 +46,8 @@ internal class LayerDrawableExtTest {
     @Test
     fun `M return drawable for index W safeGetDrawable()`() {
         // Given
+        whenever(mockLayerDrawable.numberOfLayers)
+            .thenReturn(1)
         whenever(mockLayerDrawable.getDrawable(0))
             .thenReturn(mockBitmapDrawable)
 
@@ -55,20 +57,50 @@ internal class LayerDrawableExtTest {
         )
 
         // Then
-        assertThat(drawable)
-            .isEqualTo(mockBitmapDrawable)
+        assertThat(drawable).isEqualTo(mockBitmapDrawable)
     }
 
     @Test
-    fun `M log error W safeGetDrawable() { out of bounds }`() {
+    fun `M return null W safeGetDrawable() { index below 0 }`() {
         // Given
+        whenever(mockLayerDrawable.numberOfLayers)
+            .thenReturn(0)
+        whenever(mockLayerDrawable.getDrawable(any()))
+            .thenThrow(IndexOutOfBoundsException())
+
+        // When
+        val drawable = mockLayerDrawable.safeGetDrawable(
+            index = -1,
+            mockInternalLogger
+        )
+
+        // Then
+        val captor = argumentCaptor<() -> String>()
+        verify(mockInternalLogger).log(
+            level = any(),
+            target = any(),
+            captor.capture(),
+            anyOrNull(),
+            anyOrNull(),
+            anyOrNull()
+        )
+        assertThat(captor.firstValue.invoke())
+            .startsWith("Failed to get drawable from layer - invalid index passed")
+        assertThat(drawable).isNull()
+    }
+
+    @Test
+    fun `M return null W safeGetDrawable() { index above number of layers }`() {
+        // Given
+        whenever(mockLayerDrawable.numberOfLayers)
+            .thenReturn(0)
         whenever(mockLayerDrawable.getDrawable(any()))
             .thenThrow(IndexOutOfBoundsException())
 
         // When
         val drawable = mockLayerDrawable.safeGetDrawable(
             index = 1,
-            logger = mockInternalLogger
+            mockInternalLogger
         )
 
         // Then
@@ -82,8 +114,7 @@ internal class LayerDrawableExtTest {
             anyOrNull()
         )
         assertThat(captor.firstValue.invoke())
-            .isEqualTo("Failed to get drawable from layer")
-        assertThat(drawable)
-            .isNull()
+            .startsWith("Failed to get drawable from layer - invalid index passed")
+        assertThat(drawable).isNull()
     }
 }
diff --git a/...t/kotlin/com/datadog/android/sessionreplay/internal/recorder/base64/Base64LRUCacheTest.kt b/...t/kotlin/com/datadog/android/sessionreplay/internal/recorder/base64/Base64LRUCacheTest.kt
@@ -27,7 +27,6 @@ import org.mockito.Mock
 import org.mockito.Mockito.mock
 import org.mockito.junit.jupiter.MockitoExtension
 import org.mockito.junit.jupiter.MockitoSettings
-import org.mockito.kotlin.any
 import org.mockito.kotlin.argumentCaptor
 import org.mockito.kotlin.whenever
 import org.mockito.quality.Strictness
@@ -117,6 +116,7 @@ internal class Base64LRUCacheTest {
         val mockRippleDrawable: RippleDrawable = mock()
         val mockBgLayer: Drawable = mock()
         val mockFgLayer: Drawable = mock()
+        whenever(mockRippleDrawable.numberOfLayers).thenReturn(2)
         whenever(mockRippleDrawable.safeGetDrawable(0))
             .thenReturn(mockBgLayer)
         whenever(mockRippleDrawable.safeGetDrawable(1))
@@ -138,14 +138,14 @@ internal class Base64LRUCacheTest {
     }
 
     @Test
-    fun `M not generate key prefix W put() { layerDrawable with only one layer }`(forge: Forge) {
+    fun `M not generate key prefix W put() { layerDrawable with only one layer }`(
+        @Mock mockRippleDrawable: RippleDrawable,
+        @Mock mockBgLayer: Drawable,
+        @StringForgery fakeBase64: String
+    ) {
         // Given
-        val mockRippleDrawable: RippleDrawable = mock()
-        val mockBgLayer: Drawable = mock()
-        whenever(mockRippleDrawable.safeGetDrawable(any())).thenReturn(mockBgLayer)
         whenever(mockRippleDrawable.numberOfLayers).thenReturn(1)
         whenever(mockRippleDrawable.safeGetDrawable(0)).thenReturn(mockBgLayer)
-        val fakeBase64 = forge.aString()
         testedCache.put(mockRippleDrawable, fakeBase64)
 
         val expectedPrefix = System.identityHashCode(mockBgLayer).toString() + "-"