fix(martin): reject invalid scale and inverted bbox in static rendering#2830
Merged
CommanderStorm merged 16 commits intoMay 26, 2026
Merged
Conversation
Harden request validation on the static map endpoint: - Scale factor: reject non-finite and non-positive values. `f32::NAN`, `f32::NEG_INFINITY`, and negative scales all saturate to `0u8` on the `as u8` cast, silently passing the MAX_SCALE check and then feeding NaN/Inf into `log2(pixel_ratio)` in the renderer. - Bounding box: reject inverted boxes (max < min) with a 400 rather than rendering a degenerate camera. Also rename the `static_camera_expected/` fixture directory to `static_camera/` for consistency with the other static-render fixtures. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Hardens request validation for the static map rendering endpoint by rejecting non-finite/non-positive scale factors and inverted bounding boxes, preventing degenerate inputs from reaching the renderer. Also renames a test fixture directory for consistency.
Changes:
- Reject scale factors that are NaN, infinite, zero, or negative with
400 Scale factor must be a positive finite number, before the saturatingas u8cast that would otherwise silently coerce them to 0. - Reject inverted bounding boxes (
max < minon either axis) with400 Bounding box is inverted: ...before they reachbbox_to_center_zoom. - Rename
tests/fixtures/static_camera_expected/→static_camera/(puregit mv) and update the single referencing constant.
Reviewed changes
Copilot reviewed 3 out of 10 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| martin/src/srv/styles_static.rs | Adds CameraRequest::validate (inverted-bbox check), adds finite/positive scale check before the cast in SizeRequest::validate, wires camera validation into handle_static_request, and adds rstest cases for both new error paths. |
| martin/tests/styles_rendering_test.rs | Updates CAMERA_EXPECTED_DIR constant to CAMERA_DIR to match the renamed fixture directory. |
| tests/fixtures/static_camera/{bbox_pm30,bearing_90,center_berlin_z4,center_z3,pitch_45}.png | Fixture files moved from static_camera_expected/ (byte-identical). |
CommanderStorm
commented
May 24, 2026
CommanderStorm
commented
May 24, 2026
CommanderStorm
commented
May 24, 2026
Co-authored-by: Frank Elsinga <frank@elsinga.de>
Performance Comparison
|
TEMPORARY (revert after diagnosis): include the underlying StyleError in the 500 response body so the coverage-job render-test failures (bbox/bearing) reveal the real cause in CI logs. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The render tests fetched vector tiles live from demotiles:443. The demo style's source was a TileJSON whose `tiles` field is an absolute demotiles URL, so tile fetches bypassed the mitmproxy cassette (which only captured tiles.json + glyphs). Under the coverage job's heavier concurrent load (whole workspace under llvm-cov, multi-worker pool) the burst of direct connections to demotiles got reset (SSL_ERROR_SYSCALL), failing the bbox/bearing renders -- while the lighter live unit-test job happened to pass. Give the test style an inline `tiles` source so the existing URL-rewrite routes tile fetches through the proxy, and re-record the cassette so it now contains the vector tiles. Renders no longer touch the network (verified with `--server-replay --set server_replay_extra=kill` on both render-test binaries). Update the served-style integration snapshots to match the inline-tiles source. Also reverts the temporary diagnostic that surfaced the StyleError. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This pool-concurrency unit test rendered the demo style directly (no proxy rewrite), so it fetched vector tiles live from demotiles. Under the coverage job's load that burst of connections got reset (SSL_ERROR_SYSCALL). Render a local inline-GeoJSON style instead: it needs no network, exercises the multi-worker pool exactly the same, and works both bare and under the render-cache proxy. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
nyurik
approved these changes
May 25, 2026
Switching the maplibre source from a TileJSON `url` to explicit `tiles` enlarged the served style; the body fixtures were re-blessed but the `.headers` etag still pinned the old 1041-byte length. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The `Unit test the server` job ran `cargo test --package martin` (rendering is a default feature) with no proxy, so styles_rendering_test fetched vector tiles from the live demotiles.maplibre.org upstream. That made the job flake intermittently (10/21 renders 500ing, ~6min runtime) whenever those fetches stalled on the runner. Wrap the martin tests in `just with-render-cache` on Linux so tiles replay from the committed cassette, matching the coverage job. Verified hermetic: under server_replay_extra=kill the suite passes 21/21 with zero cassette misses. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
CommanderStorm
commented
May 26, 2026
CommanderStorm
commented
May 26, 2026
CommanderStorm
commented
May 26, 2026
Merged
CommanderStorm
added a commit
that referenced
this pull request
Jun 16, 2026
## 🤖 New release
* `martin-tile-utils`: 0.7.3 -> 0.7.4 (✓ API compatible changes)
* `mbtiles`: 0.17.6 -> 0.18.0 (⚠ API breaking changes)
* `martin-core`: 0.7.0 -> 0.8.0 (⚠ API breaking changes)
* `martin`: 1.10.1 -> 1.11.0
### ⚠ `mbtiles` breaking changes
```text
--- failure enum_tuple_variant_changed_kind: An enum tuple variant changed kind ---
Description:
A public enum's exhaustive tuple variant has changed to a different kind of enum variant, breaking possible instantiations and patterns.
ref: https://doc.rust-lang.org/reference/items/enumerations.html
impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.47.0/src/lints/enum_tuple_variant_changed_kind.ron
Failed in:
variant MbtError::InconsistentMetadata in /tmp/.tmpCkABTN/martin/mbtiles/src/errors.rs:39
variant MbtError::IncorrectTileHash in /tmp/.tmpCkABTN/martin/mbtiles/src/errors.rs:50
variant MbtError::MissingTileReference in /tmp/.tmpCkABTN/martin/mbtiles/src/errors.rs:59
variant MbtError::InvalidTileIndex in /tmp/.tmpCkABTN/martin/mbtiles/src/errors.rs:68
variant MbtError::AggHashMismatch in /tmp/.tmpCkABTN/martin/mbtiles/src/errors.rs:78
variant MbtError::MismatchedTargetType in /tmp/.tmpCkABTN/martin/mbtiles/src/errors.rs:115
variant MbtError::AggHashMismatchWithDiff in /tmp/.tmpCkABTN/martin/mbtiles/src/errors.rs:152
variant MbtError::AggHashMismatchAfterApply in /tmp/.tmpCkABTN/martin/mbtiles/src/errors.rs:162
variant MbtError::BinDiffIncorrectTileHash in /tmp/.tmpCkABTN/martin/mbtiles/src/errors.rs:177
```
### ⚠ `martin-core` breaking changes
```text
--- failure auto_trait_impl_removed: auto trait no longer implemented ---
Description:
A public type has stopped implementing one or more auto traits. This can break downstream code that depends on the traits being implemented.
ref: https://doc.rust-lang.org/reference/special-types-and-traits.html#auto-traits
impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.47.0/src/lints/auto_trait_impl_removed.ron
Failed in:
type StyleSources is no longer UnwindSafe, in /tmp/.tmpCkABTN/martin/martin-core/src/resources/styles/mod.rs:93
--- failure enum_tuple_variant_changed_kind: An enum tuple variant changed kind ---
Description:
A public enum's exhaustive tuple variant has changed to a different kind of enum variant, breaking possible instantiations and patterns.
ref: https://doc.rust-lang.org/reference/items/enumerations.html
impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.47.0/src/lints/enum_tuple_variant_changed_kind.ron
Failed in:
variant FontError::InvalidFontRangeStartEnd in /tmp/.tmpCkABTN/martin/martin-core/src/resources/fonts/error.rs:19
variant PmtilesError::UnknownTileType in /tmp/.tmpCkABTN/martin/martin-core/src/tiles/pmtiles/error.rs:23
variant PostgresError::CannotUseClientKey in /tmp/.tmpCkABTN/martin/martin-core/src/tiles/postgres/errors.rs:40
variant PostgresError::PostgisTooOld in /tmp/.tmpCkABTN/martin/martin-core/src/tiles/postgres/errors.rs:84
variant PostgresError::PostgresqlTooOld in /tmp/.tmpCkABTN/martin/martin-core/src/tiles/postgres/errors.rs:93
variant PostgresError::PrepareQueryError in /tmp/.tmpCkABTN/martin/martin-core/src/tiles/postgres/errors.rs:102
--- failure enum_variant_missing: pub enum variant removed or renamed ---
Description:
A publicly-visible enum has at least one variant that is no longer available under its prior name. It may have been renamed or removed entirely.
ref: https://doc.rust-lang.org/cargo/reference/semver.html#item-remove
impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.47.0/src/lints/enum_variant_missing.ron
Failed in:
variant StyleError::SingleThreadedRenderPoolError, previously in file /tmp/.tmpFbd1fK/martin-core/src/resources/styles/error.rs:8
variant PostgresError::InvalidTableExtent, previously in file /tmp/.tmpFbd1fK/martin-core/src/tiles/postgres/errors.rs:84
--- failure inherent_method_missing: pub method removed or renamed ---
Description:
A publicly-visible method or associated fn is no longer available under its prior name. It may have been renamed or removed entirely.
ref: https://doc.rust-lang.org/cargo/reference/semver.html#item-remove
impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.47.0/src/lints/inherent_method_missing.ron
Failed in:
StyleSources::set_rendering_enabled, previously in file /tmp/.tmpFbd1fK/martin-core/src/resources/styles/mod.rs:193
```
<details><summary><i><b>Changelog</b></i></summary><p>
## `mbtiles`
<blockquote>
##
[0.18.0](mbtiles-v0.17.6...mbtiles-v0.18.0)
- 2026-06-16
### Added
- static rendering core
([#2804](#2804))
### Other
- migrate `MBTilesReloader` to be based on `ReloadDriver` and
`Discovery` (pg-reload-6)
([#2853](#2853))
- make sure that error types with multiple identical types have names
([#2832](#2832))
- lockfile maintenance
([#2822](#2822))
- *(deps)* Update `sqlx` to 0.9.0 and adapt MBTiles dynamic SQL to
`SqlSafeStr` ([#2821](#2821))
- update to maplibre_native@8.1
([#2859](#2859))
- add #![forbid(unsafe_code)] to martin-tile-utils
([#2847](#2847))
</blockquote>
## `martin-core`
<blockquote>
##
[0.8.0](martin-core-v0.7.0...martin-core-v0.8.0)
- 2026-06-16
### Added
- Auto-refresh pmtiles source on source data change
([#2858](#2858))
- duckdb-source implementation at martin-core
([#2831](#2831))
- static rendering core
([#2804](#2804))
### Fixed
- *(unstable-cog)* Allow for 0.1% (up to 3m of error at z0) in COGs from
a matching zoom level in WebMercatorQuad
([#2878](#2878))
- *(martin)* reject invalid scale and inverted bbox in static rendering
([#2830](#2830))
- ignore ignored directory for sprite resolution
([#2815](#2815))
### Other
- split out the test code to `test_support`
([#2885](#2885))
- update to maplibre_native@8.1
([#2859](#2859))
- *(pg)* cleanup how extent is handled (PGReloader 0)
([#2837](#2837))
- make sure that error types with multiple identical types have names
([#2832](#2832))
- make the MLN renderer multi threaded
([#2826](#2826))
- cache render fixtures
([#2828](#2828))
</blockquote>
## `martin`
<blockquote>
##
[1.11.0](martin-v1.10.1...martin-v1.11.0)
- 2026-06-16
### Added
- *(pg)* enable Postgres reloading
([#2841](#2841))
- teach ObjectStoreDiscovery to track source data versions
([#2864](#2864))
- Auto-refresh pmtiles source on source data change
([#2858](#2858))
- Add COGReloader and PMTilesReloader (Tile Reload Phase 2)
([#2750](#2750))
- static rendering core
([#2804](#2804))
- duckdb-source implementation at martin-core
([#2831](#2831))
### Fixed
- *(martin)* reject invalid scale and inverted bbox in static rendering
([#2830](#2830))
- Use ST_EstimatedExtent for quick bounds calc
([#1220](#1220))
- ignore ignored directory for sprite resolution
([#2815](#2815))
- *(unstable-cog)* Allow for 0.1% (up to 3m of error at z0) in COGs from
a matching zoom level in WebMercatorQuad
([#2878](#2878))
### Other
- *(deps)* autoupdate pre-commit
([#2896](#2896))
- *(deps)* Bump the all-npm-version-updates group across 2 directories
with 19 updates ([#2889](#2889))
- add tests for how EXACTLY our config system does parsing (config-4)
([#2872](#2872))
- split out the test code to `test_support`
([#2885](#2885))
- move pg fingerprinting to u128
([#2886](#2886))
- split Config types into config.rs, making main/mod.rs a pure mod
(config-3) ([#2876](#2876))
- move parsing to its own module (config-2)
([#2875](#2875))
- extract impl Config to lifecycle module (confgi-1)
([#2874](#2874))
- pg reloader test to reduce the pg-reloader diff
([#2867](#2867))
- rename PmTilesReloader to PmtilesReloader
([#2861](#2861))
- *(deps)* Bump the all-npm-version-updates group across 2 directories
with 18 updates ([#2856](#2856))
- migrate Pmtiles to `ReloadDriver` and `Discovery` (pg-reload-7)
([#2840](#2840))
- migrate `MBTilesReloader` to be based on `ReloadDriver` and
`Discovery` (pg-reload-6)
([#2853](#2853))
- adopt `ReloadDriver`/`Discovery` for Mbtiles (pg-reload-5)
([#2852](#2852))
- move reload-`Trigger`-ing to a separate trait
([#2845](#2845))
- *(deps)* autoupdate pre-commit
([#2849](#2849))
- extract `Sink` trait for the tile-source manager
([#2844](#2844))
- *(pg)* Refactor discovery to enable fingerprinting (PGReloader 2)
([#2835](#2835))
- *(pg)* split auto-discovery into discover + instantiate phases
(PGReloader 1) ([#2836](#2836))
- *(pg)* cleanup how extent is handled (PGReloader 0)
([#2837](#2837))
- make sure that error types with multiple identical types have names
([#2832](#2832))
- fix import ordering drift
([#2838](#2838))
- lockfile maintenance
([#2822](#2822))
- make the MLN renderer multi threaded
([#2826](#2826))
- add a pre-commit to simplify UTF characters
([#2827](#2827))
- update to maplibre_native@8.1
([#2859](#2859))
- cache render fixtures
([#2828](#2828))
- add #![forbid(unsafe_code)] to martin-tile-utils
([#2847](#2847))
- *(deps)* Update `sqlx` to 0.9.0 and adapt MBTiles dynamic SQL to
`SqlSafeStr` ([#2821](#2821))
</blockquote>
</p></details>
---
This PR was generated with
[release-plz](https://github.com/release-plz/release-plz/).
---------
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
breaks out one of the items of #2794