feat: pixel based query rects for raster requests #854
+34,931
−13,186
Conversation
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
…-based-queries-rebase
…-based-queries-rebase
…-based-queries-rebase
…-based-queries-rebase
- [ ] I added an entry to [`CHANGELOG.md`](CHANGELOG.md) if knowledge of this change could be valuable to users. --- Here is a brief summary of what I did: <TEXT>
- [ ] I added an entry to [`CHANGELOG.md`](CHANGELOG.md) if knowledge of this change could be valuable to users. --- Here is a brief summary of what I did: <TEXT>
…-based-queries-rebase
…-based-queries-rebase
…longer works now that resolutions are no longer part of the query rectangle
- [ ] I added an entry to [`CHANGELOG.md`](CHANGELOG.md) if knowledge of this change could be valuable to users. --- Here is a brief summary of what I did: <TEXT>
…ks now that resolutions are no longer part of the query rectangle (#1112) - [ ] I added an entry to [`CHANGELOG.md`](CHANGELOG.md) if knowledge of this change could be valuable to users. --- Here is a brief summary of what I did: <TEXT>
…e/geoengine into new-pixel-based-queries-rebase
- [ ] I added an entry to [`CHANGELOG.md`](CHANGELOG.md) if knowledge of this change could be valuable to users. --- Here is a brief summary of what I did: <TEXT>
…-based-queries-rebase
jdroenner
changed the title
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| call_on_generic_raster_processor!( | ||
| processor, | ||
| p => | ||
| raster_stream_to_png_bytes(p, query_rect, query_ctx, request.width, request.height, request.time.map(Into::into), raster_colorizer.map(Into::into), conn_closed).await | ||
| ).map_err(error::Error::from) | ||
| raster_stream_to_png_bytes(p, query_rect, query_ctx, request.width, request.height, request.time.map(Into::into), Some(raster_colorizer), conn_closed).await // TODO: pass raster colorizer here |
Check failure
Code scanning / CodeQL
Uncontrolled allocation size High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 3 days ago
General approach: Enforce reasonable upper bounds on user-controlled parameters that influence allocation (image width, height, and potentially the number of time steps) before calling raster_stream_to_png_bytes. If inputs exceed those bounds, reject the request with an appropriate error instead of proceeding. This limits maximum allocation size and avoids arithmetic overflows when computing buffer sizes.
Best concrete fix in this file:
- Introduce constants for maximum allowed WMS image width and height in
wms.rs, e.g.,MAX_WMS_WIDTHandMAX_WMS_HEIGHT. These should be large enough for practical use but finite; without broader project context, values like 4096 are a conservative, common choice. - In
wms_get_map’s innercompute_resultfunction, before constructingquery_rectand before callingraster_stream_to_png_bytes, validaterequest.widthandrequest.heightagainst these limits. If either exceeds the limit or is zero, return an error (reusing the project’s existingError/ErrorResponsetypes where possible). - Keep behavior unchanged for valid sizes; only oversize/invalid inputs will now cause an early error. The call to
raster_stream_to_png_bytesremains as-is.
We only need to modify services/src/api/handlers/wms.rs:
- Add two
constdefinitions near the top-level (after imports or near other constants). - Add a small validation block inside
compute_resultbefore the existinglet query_rect = ...line.
No changes are needed in services/src/api/ogc/util.rs.
Suggested changeset
1
services/src/api/handlers/wms.rs
| @@ -35,6 +35,10 @@ | ||
| use utoipa::openapi::{Ref, Required}; | ||
| use uuid::Uuid; | ||
|
|
||
| /// Maximum allowed WMS image dimensions to prevent uncontrolled allocations. | ||
| const MAX_WMS_WIDTH: u32 = 4096; | ||
| const MAX_WMS_HEIGHT: u32 = 4096; | ||
|
|
||
| pub(crate) fn init_wms_routes<C>(cfg: &mut web::ServiceConfig) | ||
| where | ||
| C: ApplicationContext, | ||
| @@ -436,6 +440,20 @@ | ||
|
|
||
| debug!("WMS re-scale-project: {:?}", query_tiling_pixel_grid); | ||
|
|
||
| // Guard against excessively large or zero-sized images to prevent uncontrolled allocations. | ||
| if request.width == 0 | ||
| || request.height == 0 | ||
| || request.width > MAX_WMS_WIDTH | ||
| || request.height > MAX_WMS_HEIGHT | ||
| { | ||
| return Err(Error::InvalidParams { | ||
| details: format!( | ||
| "Requested image size {}x{} is invalid or exceeds maximum {}x{}", | ||
| request.width, request.height, MAX_WMS_WIDTH, MAX_WMS_HEIGHT | ||
| ), | ||
| }); | ||
| } | ||
|
|
||
| let query_rect = RasterQueryRectangle::new( | ||
| query_tiling_pixel_grid.grid_bounds(), | ||
| query_time, |
Copilot is powered by AI and may make mistakes. Always verify output.
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.
CHANGELOG.mdif knowledge of this change could be valuable to users.Here is a brief summary of what I did:
This PR changes the QueryRectangle struct to combine a temporal and a spatial query.
All QueryRectangles are now created using a static constructor method.
VectorQueryRectangle use the same components as before just different composition.
RasterQueryRectangle now use a GridBounds in Pixels.
Projection might still have a 1Px offset issue:
TODO: Tests
TODO: Re-Grid-Operator