fix(android): render require() marker images on development builds

README.md

@@ -514,7 +514,21 @@ Supported image sources:
 | ------------- | ---------------------- | ----------------------------------------- |
 | Bundled asset | `require('./pin.png')` | Resolved on JS side before crossing Nitro |
 | Local URI     | `{ uri: 'file:///…' }` | Platform file paths                       |
-| Remote URL    | `{ uri: 'https://…' }` | Async fetch with in-memory cache          |
+| Remote URL    | `{ uri: 'https://…' }` | Async fetch with in-memory cache; Android restricts the host, see below |
+
+### Remote image hosts on Android
+
+A remote URL you pass yourself — `image={{ uri: someUrl }}` — is checked before Android fetches
+it. The URL must use `http`/`https`, must not carry `user:password@`, and its host must not
+resolve to a private address: loopback, link-local, site-local, multicast, IPv6 unique-local, a
+`.local`/`.localhost` name, or a cloud metadata endpoint. The address the connection is actually
+made to is checked again before the request is sent, every redirect target is checked the same
+way, and a redirect from `https` to `http` is not followed. This guards the common case of a
+marker icon URL arriving as data from an API. A rejected URL falls back to the default pin and
+logs `Rejected remote marker image URI` under the `NitroMaps` tag.
+
+`require()`d assets are exempt, including the packager URL Metro resolves them to in a
+development build. iOS does not apply this policy today.
 
 Additional props:
 

package/android/src/main/java/com/margelo/nitro/nitromaps/MarkerIconFactory.kt

@@ -11,13 +11,6 @@ import android.util.LruCache
 import com.google.android.gms.maps.model.BitmapDescriptor
 import com.google.android.gms.maps.model.BitmapDescriptorFactory
 import com.google.android.gms.maps.model.Marker
-import java.net.Inet6Address
-import java.net.InetAddress
-import java.net.URI
-import java.net.URISyntaxException
-import java.net.URL
-import java.net.UnknownHostException
-import java.util.Locale
 import java.util.WeakHashMap
 import java.util.concurrent.ExecutorService
 import java.util.concurrent.Executors
@@ -167,7 +160,9 @@ internal class MarkerIconFactory(
   private fun defaultMarkerDisplaySizePx(): Pair<Float, Float> = (DEFAULT_MARKER_WIDTH_DP * density) to (DEFAULT_MARKER_HEIGHT_DP * density)
 
   private fun cacheKey(image: MarkerImage): String {
-    return "${image.uri}|${image.width ?: ""}|${image.height ?: ""}|${image.scale ?: ""}"
+    // `origin` is part of the key so a bundled image cannot warm the cache for a
+    // user-supplied one with the same URI, which would hand it a policy-free entry.
+    return "${image.uri}|${image.width ?: ""}|${image.height ?: ""}|${image.scale ?: ""}|${image.origin ?: ""}"
   }
 
   private fun loadLocalIcon(
@@ -188,8 +183,8 @@ internal class MarkerIconFactory(
       return
     }
 
-    if (isRemoteMarkerUri(image.uri)) {
-      remoteMarkerUriRejectReason(image.uri, resolveHostAddress = false)?.let { reason ->
+    if (RemoteMarkerUriPolicy.isRemoteUri(image.uri)) {
+      RemoteMarkerUriPolicy.rejectReason(image, resolveHostAddress = false)?.let { reason ->
         logRejectedRemoteMarkerUri(image.uri, reason)
         deliverOnMainThread { onLoaded(null) }
         return
@@ -198,7 +193,7 @@ internal class MarkerIconFactory(
 
     loadExecutor.execute {
       val descriptor =
-        if (isRemoteMarkerUri(image.uri)) {
+        if (RemoteMarkerUriPolicy.isRemoteUri(image.uri)) {
           loadRemoteIcon(image, key)
         } else {
           loadLocalIcon(image, key)
@@ -248,19 +243,10 @@ internal class MarkerIconFactory(
     image: MarkerImage,
     key: String,
   ): BitmapDescriptor? {
-    remoteMarkerUriRejectReason(image.uri, resolveHostAddress = true)?.let { reason ->
-      logRejectedRemoteMarkerUri(image.uri, reason)
-      return null
-    }
-
     return try {
-      val connection = URL(image.uri).openConnection()
-      connection.connectTimeout = 10_000
-      connection.readTimeout = 10_000
-      connection.getInputStream().use { stream ->
-        val bitmap = decodeByteArray(stream.readBytes(), image) ?: return null
-        cacheBitmap(key, resizeBitmap(bitmap, image))
-      }
+      val bytes = remoteImageFetcher.fetch(image, ::logRejectedRemoteMarkerUri) ?: return null
+      val bitmap = decodeByteArray(bytes, image) ?: return null
+      cacheBitmap(key, resizeBitmap(bitmap, image))
     } catch (error: Exception) {
       Log.w(NITRO_MAPS_LOG_TAG, "Failed to load marker image: ${image.uri}", error)
       null
@@ -279,7 +265,7 @@ internal class MarkerIconFactory(
       return decodeFile(uri, image)
     }
 
-    if (isRemoteMarkerUri(uri)) {
+    if (RemoteMarkerUriPolicy.isRemoteUri(uri)) {
       return null
     }
 
@@ -408,101 +394,6 @@ internal class MarkerIconFactory(
     return descriptor
   }
 
-  private fun isRemoteMarkerUri(uriString: String): Boolean {
-    val scheme = parseRemoteMarkerUri(uriString)?.scheme?.lowercase(Locale.US) ?: return false
-    return scheme == "http" || scheme == "https"
-  }
-
-  private fun parseRemoteMarkerUri(uriString: String): URI? =
-    try {
-      URI(uriString)
-    } catch (_: URISyntaxException) {
-      null
-    }
-
-  private fun remoteMarkerUriRejectReason(
-    uriString: String,
-    resolveHostAddress: Boolean,
-  ): String? {
-    val uri = parseRemoteMarkerUri(uriString) ?: return "invalid URI"
-
-    when (uri.scheme?.lowercase(Locale.US)) {
-      "http", "https" -> Unit
-      null -> return "missing scheme"
-      else -> return "unsupported scheme"
-    }
-
-    if (uri.userInfo != null) {
-      return "user info not allowed"
-    }
-
-    val host = uri.host?.lowercase(Locale.US)?.takeIf { it.isNotEmpty() } ?: return "missing host"
-    if (!isAllowlistedRemoteHost(host, resolveHostAddress)) {
-      return "host not allowlisted"
-    }
-
-    return null
-  }
-
-  private fun isAllowlistedRemoteHost(
-    host: String,
-    resolveHostAddress: Boolean,
-  ): Boolean {
-    if (host == "localhost" || host.endsWith(".localhost") || host.endsWith(".local")) {
-      return false
-    }
-    if (host in BLOCKED_REMOTE_HOSTS) {
-      return false
-    }
-
-    if (!resolveHostAddress && !isNumericHostLiteral(host)) {
-      return true
-    }
-
-    val address =
-      try {
-        InetAddress.getByName(host)
-      } catch (_: UnknownHostException) {
-        return !resolveHostAddress
-      }
-
-    return isAllowlistedRemoteAddress(address)
-  }
-
-  private fun isNumericHostLiteral(host: String): Boolean {
-    if (host.startsWith("[") && host.endsWith("]")) {
-      return true
-    }
-
-    val parts = host.split('.')
-    return parts.size == 4 &&
-      parts.all { part ->
-        val value = part.toIntOrNull() ?: return@all false
-        value in 0..255
-      }
-  }
-
-  private fun isAllowlistedRemoteAddress(address: InetAddress): Boolean {
-    if (
-      address.isLoopbackAddress ||
-      address.isAnyLocalAddress ||
-      address.isLinkLocalAddress ||
-      address.isSiteLocalAddress ||
-      address.isMulticastAddress
-    ) {
-      return false
-    }
-
-    if (address is Inet6Address) {
-      val firstOctet = address.address[0].toInt() and 0xff
-      if ((firstOctet and 0xfe) == 0xfc) {
-        return false
-      }
-    }
-
-    return true
-  }
-
   private fun logRejectedRemoteMarkerUri(
     uri: String,
     reason: String,
@@ -545,10 +436,8 @@ internal class MarkerIconFactory(
 
     private val loadExecutor: ExecutorService = Executors.newSingleThreadExecutor()
 
-    private val BLOCKED_REMOTE_HOSTS =
-      setOf(
-        "metadata.google.internal",
-        "metadata.goog",
-      )
+    // Shared by every map, and first built on the load executor: an HTTP client loads the system
+    // trust store when it is created.
+    private val remoteImageFetcher by lazy { RemoteMarkerImageFetcher() }
   }
 }

package/android/src/main/java/com/margelo/nitro/nitromaps/RemoteMarkerImageFetcher.kt

@@ -0,0 +1,139 @@
+package com.margelo.nitro.nitromaps
+
+import okhttp3.Interceptor
+import okhttp3.OkHttpClient
+import okhttp3.Request
+import okhttp3.Response
+import java.io.IOException
+import java.net.MalformedURLException
+import java.net.Proxy
+import java.net.URL
+import java.util.concurrent.TimeUnit
+
+/**
+ * Downloads a remote marker image and follows its redirects itself, so that every hop of a
+ * user-supplied URL passes [RemoteMarkerUriPolicy]. An automatic redirect would reach a private
+ * host without asking the policy.
+ *
+ * Each hop is checked twice. First the URL, before the request: that gives a reason to log, and
+ * in the common case no connection is made at all. Then the address the connection was actually
+ * made to, before anything is sent over it ([ConnectedAddressCheck]). The client resolves the host
+ * again to connect, and a DNS answer that changed in between (rebinding) would otherwise send the
+ * request to a private address the first check never saw.
+ *
+ * @param baseClient the client the fetching clients are derived from; a seam for tests.
+ * @param hostRejectReason the policy check on one hop's URL; a seam for tests.
+ */
+internal class RemoteMarkerImageFetcher(
+  baseClient: OkHttpClient = OkHttpClient(),
+  private val hostRejectReason: (uri: String) -> String? = { uri ->
+    RemoteMarkerUriPolicy.rejectReason(uri, resolveHostAddress = true)
+  },
+) {
+  /** Fetches a `require()`d image, whose packager URL is private in a development build. */
+  private val bundledClient =
+    baseClient
+      .newBuilder()
+      .followRedirects(false)
+      .followSslRedirects(false)
+      .connectTimeout(TIMEOUT_MS, TimeUnit.MILLISECONDS)
+      .readTimeout(TIMEOUT_MS, TimeUnit.MILLISECONDS)
+      .build()
+
+  /** Fetches a user-supplied image, and only over a connection the policy allows. */
+  private val userClient = bundledClient.newBuilder().addNetworkInterceptor(ConnectedAddressCheck).build()
+
+  /**
+   * Returns the body at [image]'s URL, or `null` after telling [onRejected] which URI may not be
+   * fetched and why. A failed request throws.
+   */
+  fun fetch(
+    image: MarkerImage,
+    onRejected: (uri: String, reason: String) -> Unit,
+  ): ByteArray? {
+    // Like the policy itself, a bundled image skips the host checks, on every hop.
+    val checksHost = image.origin != MarkerImageOrigin.BUNDLED
+    val client = if (checksHost) userClient else bundledClient
+    var url = URL(image.uri)
+
+    repeat(MAX_REDIRECTS + 1) {
+      if (checksHost) {
+        // The URL about to be requested, as it will be requested, rather than the string it came from.
+        hostRejectReason(url.toString())?.let { reason ->
+          onRejected(url.toString(), reason)
+          return null
+        }
+      }
+
+      val response =
+        try {
+          client.newCall(Request.Builder().url(url).build()).execute()
+        } catch (rejected: RejectedConnectionException) {
+          onRejected(url.toString(), rejected.reason)
+          return null
+        }
+
+      response.use {
+        if (!response.isRedirect) {
+          if (!response.isSuccessful) {
+            throw IOException("HTTP ${response.code} for $url")
+          }
+          return response.body?.bytes()
+        }
+
+        val target = response.header("Location")?.let { resolveLocation(url, it) }
+        if (target == null) {
+          onRejected(url.toString(), "redirect without a usable location")
+          return null
+        }
+        RemoteMarkerUriPolicy.redirectRejectReason(url, target)?.let { reason ->
+          onRejected(target.toString(), reason)
+          return null
+        }
+        url = target
+      }
+    }
+
+    onRejected(image.uri, "more than $MAX_REDIRECTS redirects")
+    return null
+  }
+
+  private fun resolveLocation(
+    from: URL,
+    location: String,
+  ): URL? =
+    try {
+      // Unlike `URI.resolve`, keeps the slash when `from` has an empty path.
+      URL(from, location)
+    } catch (_: MalformedURLException) {
+      null
+    }
+
+  /**
+   * Closes a direct connection to an address [RemoteMarkerUriPolicy] rejects before the request is
+   * written to it. Behind a proxy, the proxy resolves the host, so only the check on the URL applies.
+   */
+  private object ConnectedAddressCheck : Interceptor {
+    override fun intercept(chain: Interceptor.Chain): Response {
+      val connection = chain.connection()
+      if (connection != null && connection.route().proxy.type() == Proxy.Type.DIRECT) {
+        val address = connection.route().socketAddress.address
+        if (address == null || !RemoteMarkerUriPolicy.isAllowlistedAddress(address)) {
+          connection.socket().close()
+          throw RejectedConnectionException("connected address not allowlisted")
+        }
+      }
+      return chain.proceed(chain.request())
+    }
+  }
+
+  /** A connection the policy refused before anything was sent over it. */
+  private class RejectedConnectionException(
+    val reason: String,
+  ) : IOException(reason)
+
+  companion object {
+    const val MAX_REDIRECTS = 5
+    private const val TIMEOUT_MS = 10_000L
+  }
+}