fix(martin): reject invalid scale and inverted bbox in static rendering

.github/workflows/ci.yml

@@ -73,14 +73,23 @@ jobs:
       - name: Install rendering dependencies
         if: runner.os == 'Linux'
         run: just install-dependencies
+      - name: Install uv
+        if: runner.os == 'Linux'
+        uses: astral-sh/setup-uv@08807647e7069bb48b6ef5acd8ec9567f424441b # v8.1.0
       - name: Run cargo test
         run: |
           set -x
           cargo test --package martin-tile-utils
           cargo test --package mbtiles --no-default-features
           cargo test --package mbtiles
-          cargo test --package martin
           cargo test --package martin-core
+          # style rendering requires upstream resources
+          # we use a cassete recording to not have random flakes
+          if [ "$RUNNER_OS" = Linux ]; then
+            just with-render-cache 'cargo test --package martin'
+          else
+            cargo test --package martin
+          fi
         env:
           AWS_SKIP_CREDENTIALS: 1
           AWS_REGION: eu-central-1

justfile

@@ -294,8 +294,7 @@ install-mitmproxy:
     fi
 
 # Internal: run `cmd` with mitmproxy reverse-proxying the two rendering-test
-# upstreams. Plain HTTP only -- ports must match the `PROXIED_HOSTS` constants in
-# martin-core/tests/rendering_test.rs and martin/tests/styles_rendering_test.rs.
+# upstreams. Plain HTTP only - ports must match test_render_cache::PROXIED_HOSTS.
 [linux]
 _run-render-proxy mode *cmd: install-mitmproxy
     #!/usr/bin/env bash

martin-core/src/resources/styles/render_pool.rs

@@ -250,20 +250,41 @@ struct RendererState {
 
 #[cfg(test)]
 mod tests {
-    use std::path::Path;
     use std::sync::Arc;
 
     use super::*;
 
-    fn fixture_path(name: &str) -> PathBuf {
-        Path::new("../tests/fixtures/styles").join(name)
-    }
-
     #[tokio::test]
     async fn concurrent_renders_all_succeed() {
+        let style_json = r##"{
+            "version": 8,
+            "sources": {
+                "poly": {
+                    "type": "geojson",
+                    "data": {
+                        "type": "Feature",
+                        "properties": {},
+                        "geometry": {
+                            "type": "Polygon",
+                            "coordinates": [[[-10, -10], [10, -10], [10, 10], [-10, 10], [-10, -10]]]
+                        }
+                    }
+                }
+            },
+            "layers": [
+                {"id": "bg", "type": "background", "paint": {"background-color": "#ffffff"}},
+                {"id": "poly", "type": "fill", "source": "poly", "paint": {"fill-color": "#ff0000"}}
+            ]
+        }"##;
+        let style_file = tempfile::Builder::new()
+            .suffix(".json")
+            .tempfile()
+            .expect("create style tempfile");
+        std::fs::write(style_file.path(), style_json).expect("write style");
+
         let workers = NonZeroUsize::new(4);
         let pool = Arc::new(RendererPool::new(workers).expect("spawn render pool"));
-        let style = fixture_path("maplibre_demo.json");
+        let style = style_file.path().to_path_buf();
 
         let mut handles = Vec::new();
         for i in 0..16 {

martin/src/srv/styles_static.rs

@@ -112,6 +112,24 @@ impl<'de> Deserialize<'de> for CameraRequest {
     }
 }
 
+impl CameraRequest {
+    fn validate(self) -> Result<Self, HttpResponse> {
+        if let Self::BoundingBox {
+            min_lon,
+            min_lat,
+            max_lon,
+            max_lat,
+        } = self
+            && (max_lon < min_lon || max_lat < min_lat)
+        {
+            return Err(HttpResponse::BadRequest()
+                .content_type(ContentType::plaintext())
+                .body("Bounding box is inverted: max must be greater than or equal to min"));
+        }
+        Ok(self)
+    }
+}
+
 /// Parsed `{size}` path segment: `WIDTHxHEIGHT[@SCALEx]`. Bounds are
 /// checked in [`Self::validate`] after deserialization so the response
 /// can name which bound was hit.
@@ -129,12 +147,6 @@ const MAX_SCALE: u8 = 4;
 
 impl SizeRequest {
     fn validate(self) -> Result<Self, HttpResponse> {
-        #[expect(
-            clippy::cast_possible_truncation,
-            clippy::cast_sign_loss,
-            reason = "scale is bounded above by MAX_SCALE and is non-negative"
-        )]
-        let scale_u8 = self.scale.round() as u8;
         if self.width == 0 || self.height == 0 {
             return Err(HttpResponse::BadRequest()
                 .content_type(ContentType::plaintext())
@@ -147,6 +159,17 @@ impl SizeRequest {
                     "Image dimensions exceed maximum allowed ({MAX_WIDTH}x{MAX_HEIGHT})"
                 )));
         }
+        if !self.scale.is_finite() || self.scale <= 0.0 {
+            return Err(HttpResponse::BadRequest()
+                .content_type(ContentType::plaintext())
+                .body("Scale factor must be a positive finite number"));
+        }
+        #[expect(
+            clippy::cast_possible_truncation,
+            clippy::cast_sign_loss,
+            reason = "scale was checked to be finite and positive above"
+        )]
+        let scale_u8 = self.scale.round() as u8;
         if scale_u8 > MAX_SCALE {
             return Err(HttpResponse::BadRequest()
                 .content_type(ContentType::plaintext())
@@ -269,7 +292,12 @@ async fn handle_static_request(path: &StaticImagePath, styles: &StyleSources) ->
         Err(resp) => return resp,
     };
 
-    let camera = resolve_camera(path.camera, size);
+    let camera_req = match path.camera.validate() {
+        Ok(c) => c,
+        Err(resp) => return resp,
+    };
+
+    let camera = resolve_camera(camera_req, size);
 
     trace!(
         "Rendering static image for style {style_id} at ({lon},{lat}) z{zoom} {w}x{h}@{scale}",
@@ -519,6 +547,11 @@ mod tests {
     #[case::oversize_width("9999x100.png", "Image dimensions exceed maximum")]
     #[case::oversize_height("100x9999.png", "Image dimensions exceed maximum")]
     #[case::oversize_scale("100x100@9x.png", "Scale factor exceeds maximum")]
+    #[case::zero_scale("100x100@0x.png", "Scale factor must be a positive finite number")]
+    #[case::negative_scale("100x100@-2x.png", "Scale factor must be a positive finite number")]
+    #[case::nan_scale("100x100@nanx.png", "Scale factor must be a positive finite number")]
+    #[case::pos_inf_scale("100x100@infx.png", "Scale factor must be a positive finite number")]
+    #[case::neg_inf_scale("100x100@-infx.png", "Scale factor must be a positive finite number")]
     #[actix_rt::test]
     async fn dimension_violations_return_400_with_specific_message(
         #[case] size: &str,
@@ -533,4 +566,22 @@ mod tests {
             "size={size:?}: expected body to start with {expected_prefix:?}, got {body:?}"
         );
     }
+
+    #[rstest]
+    #[case::inverted_lon("10,0,-10,5")]
+    #[case::inverted_lat("0,5,1,-5")]
+    #[actix_rt::test]
+    async fn inverted_bbox_returns_400(#[case] params: &str) {
+        let (styles, _f) = one_style();
+        let resp = call!(
+            get(&format!("/style/s/static/{params}/200x200.png")),
+            styles
+        );
+        assert_eq!(resp.status(), StatusCode::BAD_REQUEST, "params={params:?}");
+        let body = body_text(resp).await;
+        assert!(
+            body.starts_with("Bounding box"),
+            "params={params:?}: expected body to start with \"Bounding box\", got {body:?}"
+        );
+    }
 }

martin/tests/styles_rendering_test.rs

@@ -269,7 +269,7 @@ async fn render_concurrent_requests() {
     );
 }
 
-const CAMERA_EXPECTED_DIR: &str = "../tests/fixtures/static_camera_expected";
+const CAMERA_DIR: &str = "../tests/fixtures/static_camera";
 
 async fn png_response(uri: &str) -> Vec<u8> {
     let app = create_app! { CONFIG_STYLES.as_str() };
@@ -358,7 +358,7 @@ fn assert_visually_similar(name: &str, a: &[u8], b: &[u8]) {
 }
 
 fn camera_ref(stem: &str) -> PathBuf {
-    PathBuf::from(CAMERA_EXPECTED_DIR).join(format!("{stem}.png"))
+    PathBuf::from(CAMERA_DIR).join(format!("{stem}.png"))
 }
 
 #[tokio::test]

tests/expected/configured/style_maplibre_demo.1.json

@@ -535,8 +535,12 @@
   },
   "sources": {
     "maplibre": {
-      "type": "vector",
-      "url": "https://demotiles.maplibre.org/tiles/tiles.json"
+      "maxzoom": 6,
+      "minzoom": 0,
+      "tiles": [
+        "https://demotiles.maplibre.org/tiles/{z}/{x}/{y}.pbf"
+      ],
+      "type": "vector"
     }
   },
   "version": 8,

tests/expected/configured/style_maplibre_demo.1.json.headers

@@ -1,5 +1,5 @@
 content-encoding: br
 content-type: application/json
-etag: W/"1041-WziwVsCt8kPEqnH09WGynw=="
+etag: W/"1062-03lipI2ul-91qQlCIStBcA=="
 transfer-encoding: chunked
 vary: accept-encoding, Origin, Access-Control-Request-Method, Access-Control-Request-Headers

tests/expected/configured/style_maplibre_demo.json

@@ -535,8 +535,12 @@
   },
   "sources": {
     "maplibre": {
-      "type": "vector",
-      "url": "https://demotiles.maplibre.org/tiles/tiles.json"
+      "maxzoom": 6,
+      "minzoom": 0,
+      "tiles": [
+        "https://demotiles.maplibre.org/tiles/{z}/{x}/{y}.pbf"
+      ],
+      "type": "vector"
     }
   },
   "version": 8,

tests/expected/configured/style_maplibre_demo.json.headers

@@ -1,5 +1,5 @@
 content-encoding: br
 content-type: application/json
-etag: W/"1041-WziwVsCt8kPEqnH09WGynw=="
+etag: W/"1062-03lipI2ul-91qQlCIStBcA=="
 transfer-encoding: chunked
 vary: accept-encoding, Origin, Access-Control-Request-Method, Access-Control-Request-Headers

tests/fixtures/styles/maplibre_demo.json

@@ -396,8 +396,10 @@
   },
   "sources": {
     "maplibre": {
-      "type": "vector",
-      "url": "https://demotiles.maplibre.org/tiles/tiles.json"
+      "maxzoom": 6,
+      "minzoom": 0,
+      "tiles": ["https://demotiles.maplibre.org/tiles/{z}/{x}/{y}.pbf"],
+      "type": "vector"
     }
   },
   "version": 8,