Skip to content

fix(engine): keep a cloud-optimised GeoTIFF's look in a saved project - #1533

Draft
elpidioassinaturas wants to merge 4 commits into
opengeos:mainfrom
elpidioassinaturas:fix/saved-address-look
Draft

elpidioassinaturas wants to merge 4 commits into
opengeos:mainfrom
elpidioassinaturas:fix/saved-address-look

Conversation

@elpidioassinaturas

@elpidioassinaturas elpidioassinaturas commented Nov 7, 2026 •

Copy link
Copy Markdown
Contributor

Summary

A cloud-optimised GeoTIFF tile layer saved by its plain web address loses its look. saved_address cuts everything after # from a plain address, and that part holds #range=…, #bands=…, #dequantize=aef and, since #1339, #cube=1. Saving then says "1 layer address(es) were saved without their keys, tokens or passwords", which is untrue here.

  • A data cube has this in url and in every time_urls entry. stac.addCube writes its fixed "range" there, so a reopened cube stretches each file on its own and the same value no longer draws the same grey from date to date.
  • A cube of one file has no time_urls, so #cube=1 is all that keeps it a tile layer. Reading the code, on main it is cut on saving and the layer opens as a raster.
  • AlphaEarth tiles (embeddings.add) lose their bands and stretch.
  • A tile layer from map.addTiles opens as a raster since feat(raster): cloud-optimised GeoTIFFs are raster layers read in place #1339, without the look it was saved with.

layer.addUrl of a plain COG is no longer affected: since #1339 it makes a raster layer, whose stretch is saved apart from its address.

The doc comment on saved_address says a COG's look is kept, and split_look exists for that, but only the wrapper and template branches used it.

Reproduced with this branch's tests and project.rs from main at 183ccdd3, saving map.addTiles of a public COG with #range=0,1000:

rustgis-cli: 1 layer address(es) were saved without their keys, tokens or passwords: after RustGIS closes, add those layers again to give them
"url":"https://sentinel-cogs.s3.us-west-2.amazonaws.com/sentinel-s2-l2a-cogs/36/Q/WD/2020/7/S2A_36QWD_20200701_0_L2A/TCI.tif"

With this PR the same commands give "stripped": 0, no such notice, and the address ends in TCI.tif#range=0,1000. Opened again, it is a raster layer stretched from 0 to 1000. With ?token=s3cret&v=1 before the look, the token goes, it is counted, and the address ends in TCI.tif?v=1#range=0,1000.

Changes

  • crates/engine/src/project.rs, saved_address: the plain branch takes the look off with split_look first, cleans the address as before, and puts the look back. An address with nothing else to remove now comes back unchanged, so lost_secret no longer counts it and project.saveAs says nothing about keys.
  • crates/engine/src/project.rs, split_look: names what a saved look may hold: bands and range (which Look::parse has checked are numbers), dequantize as aef or none, cube as 0 or 1. Anything else isn't kept. Look::parse accepts any word for dequantize and cube, and a look is saved as written, so #dequantize=<anything> would otherwise reach the file. This also closes that gap in the wrapper and template branches, which already kept looks as written. Reading is unchanged.
  • docs/user-guide/workspace.md: says a look is saved and any other # part isn't.

Choices in this proposal

  • Any other fragment still goes. OAuth and SAS-style links carry tokens after #, as sources::redact says, so only a look stays: band numbers, a range of two numbers, aef or none. #access_token=…, #range=0,1&token=… and #range=0,1?token=… are not looks. In #sig=…#range=0,1 only the last part stays.
  • The look is kept as written, not rewritten by Look::suffix. Rewriting would turn #range=0.0,1000.0 into #range=0,1000, the address would differ from the original, and it would be counted as a removed key.
  • What a saved look may hold is a closed list in split_look, not a change to Look::parse, so what a file saves changes and how an address is read doesn't. A key added to Look later isn't saved as written until it's named here: until then its look goes on saving, with the notice. RustGIS only ever writes aef and cube=1, and map.setTiles takes aef or none.
  • FORMAT stays at 1. A saved look is mostly nothing new to a reader: v0.4.0 and v0.5.0 read #range=…, #bands=… and #dequantize=aef through cog::split and draw them, and they already save looks on template and S3 connection addresses. v0.5.0 doesn't know cube, so it reads an address holding it as having no look at all, which is what it would get from that address saved on main. Saving again in those releases cuts the look and counts it in the "without their keys" notice, as they do today. Raising the version would make them refuse every project saved from now on, with or without a look, which seems too much for that. It's the maintainers' call.

Known limits

  • Projects already saved have lost their look. It can't be recovered from the file: add the layer or the cube again.
  • fix(security): keep credentials within runtime boundaries #1521 moves this code. Its public_web_address still removes every fragment from a plain address, so whichever merges second has to carry this rule over.
  • A hand-typed #range=0,1&dequantize=other still draws with its range in the open project, but its whole fragment goes when saved, range included, and is counted in the notice. RustGIS never writes such an address.
  • A project from this version opened and saved again in v0.4.0 or v0.5.0 loses its looks, with that notice, whose words don't fit a look.
  • A look-shaped fragment on an address that isn't a COG now stays too (a WMS address ending in #range=0,1, say). It holds only numbers and these words.
  • A fragment that isn't a look is still counted in the "without their keys" notice even when it holds no key (#section), since it can't be told from a token. Unchanged.
  • sources::redact, used for the recent and saved sources lists, still drops every fragment, a look included. Its comment says nothing that adds a source needs it. Not changed here.

Testing

  • tests::layer_address_redaction_keeps_a_cogs_look (engine): looks kept on plain, s3://, anonymous, login-bound and template addresses; keys removed around a look; fragments that resemble a look removed, on template and S3 connection addresses too.
  • tests::saved_cubes_keep_their_look (engine): both ways through project.saveAs. A clean cube gives stripped 0, no notice, and opens with the same url and time_urls. A token on one date gives stripped 1 and the notice, the file holds no token but keeps the look, and the open project keeps everything. A cube of one file opens with its #cube=1, as a tile layer.
  • Both fail with project.rs from main. With the closed list turned off, the first fails on #dequantize=s3cret.
  • On Windows at 0bc424f2: cargo fmt --check and cargo clippy -p rustgis-engine --all-targets -D warnings pass, and so do the rustgis-tiles, rustgis-map and rustgis-data lib tests. Linux CI is the reference for the rest.
  • The rustgis-engine lib suite has no clean run on Windows here, with or without this change. Over 6 runs with it and 4 with project.rs from main, the failures differ from run to run: 1 to 5 tests with it, 0 to 4 without (not counting this PR's two tests, which are expected to fail without it). They include pmtiles_/mbtiles_archives_frame_the_map_on_their_bounds, an_oversized_ogc_items_binding_is_refused_before_native_fetch and several tests feat(raster): cloud-optimised GeoTIFFs are raster layers read in place #1339 added, with errors such as "dropped: the view moved on" and "timeout: global". Tests run in parallel share the process's tile cache, which a session with logins clears when dropped.
  • One number I can't fully explain: a_cog_read_in_place_from_a_connection_records_it_and_follows_its_login failed in 4 of the 6 runs with this change (http status: 401) and in none of the 4 without it. It passes alone, 3 of 3, and doesn't save a project. saved_address only returns a string and is reached only when saving, so I don't see a path from this change to that test, but the runs are too few to rule it out. Part of these runs shared the machine with another build.

No visible change in the interface, so no screenshots.

🤖 Generated with Claude Code

A plain web address lost everything after `#` when saved, so a COG lost its
`#range=…` and `#bands=…`, a data cube lost its fixed range on every date, and
saving counted each address as a removed key. Only a look stays: any other
fragment still goes, as OAuth and SAS-style links carry tokens there.

`dequantize` in a look now takes only `aef` or `none`, since a look is saved as
written and must not hold free text.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Nov 7, 2026

Copy link
Copy Markdown
Contributor

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

  • Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

Comment thread crates/tiles/src/cog.rs Resolved
Comment thread crates/engine/src/project.rs Resolved
@github-actions

github-actions Bot commented Nov 7, 2026

Copy link
Copy Markdown
Contributor

Code review

I traced saved_address, split_look, cog::split and Look::parse by hand against each new test case, and checked the wrapper and template branches and the cache and embeddings callers. I could not compile or run the tests here (no shell), so the test results are unverified. Prior review threads: none, so no earlier concerns were left alone.

Bugs

  • crates/tiles/src/cog.rs:111 — Tightening dequantize also changes the reader. #range=0,1&dequantize=other used to apply its range and now is silently ignored, with no notice. AGENTS.md says a reader that drops data must say so. Medium confidence. Posted inline.

Security

  • No problems found. Kept look fragments can hold only band numbers, a finite range pair, or aef/none. Every test case I traced removes the keys and keeps the look as the PR says.
  • Low-confidence note: range= accepts any finite floats, so an all-digit secret placed under that key would be kept. The PR's "holds no free text" is true except for this.

Performance

  • None.

Quality

  • Nit: docs/user-guide/workspace.md lists #bands=… and #range=… but not &dequantize=aef, which is the AlphaEarth case the PR is about.
  • Nit: saved_cubes_keep_their_look calls remove_dir_all only at the end, so a failed assert leaves the temp directory behind.
  • Nit: in the wrapper branch, the look is appended after bind validates the length, so an address near MAX_ADDRESS could save as a wrapper that then fails to parse. Low confidence; the layer would fail visibly, not silently.

AGENTS.md

  • crates/engine/src/project.rs:100 — Saved files now carry looks that older RustGIS versions dropped. docs/maintaining.md says a format version goes up when an older reader would lose data, and FORMAT is still 1. Maintainer decision. Medium-low confidence. Posted inline.
  • Otherwise compliant: no new unwrap/panic, the stripped-count notice still applies to the cases that change, both clean and lossy cases are tested, and no dependencies changed.

Change size

Compared with the base branch (merge base 3c119b7 to PR head 159523b), the whole PR.

Measure Value
Files changed 4 (0 added, 4 modified, 0 removed, 0 renamed)
Size before 1.83 MB
Size after 1.84 MB
Total size change +5.5 KB
Path Before After Change
crates/engine/src/tests.rs 1.67 MB 1.68 MB +4.6 KB
crates/tiles/src/cog.rs 60.6 KB 61.1 KB +522 B
crates/engine/src/project.rs 60.9 KB 61.0 KB +171 B
docs/user-guide/workspace.md 36.4 KB 36.5 KB +144 B

@github-actions

github-actions Bot commented Nov 7, 2026 •

Copy link
Copy Markdown
Contributor

📖 Docs preview

Item Value
Preview https://preview.rustgis.app/pr-1533/
Web app https://preview.rustgis.app/pr-1533/app/
Commit 0bc424f

GitHub Pages takes a minute or two to publish a new commit.

elpidiofilho and others added 3 commits November 7, 2026 00:05
…reading

Tightening `Look::parse` also changed how an address is read: a hand-typed
`#range=0,1&dequantize=other` lost its range, unsaid. Reading stays as it was;
`split_look` now refuses to keep a look whose `dequantize` is another word.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
After opengeos#1339 a look also has `cube`, which takes any word as `dequantize` does.
`split_look` now names what may stay, so a key added later isn't saved as
written until it is named here. A one-file cube keeps its `#cube=1`, the only
thing that keeps it a tile layer when its project opens.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@elpidioassinaturas

Copy link
Copy Markdown
Contributor Author

Left from the review summary above, for the release quality pass: the temp directory in saved_cubes_keep_their_look isn't removed when an assert fails, as in the tests around it. The note on the wrapper branch appending the look after bind checks the length is on code this PR doesn't change. The docs now list #dequantize=aef.

Comment thread crates/engine/src/project.rs Resolved
@github-actions

github-actions Bot commented Nov 7, 2026

Copy link
Copy Markdown
Contributor

Code review

Verdict: no bugs, crash paths, or token leaks found. One low-to-medium concern about the notice wording, posted inline.

Bugs

  • None found. I traced the new split_look against cog::split and Look::parse. A fragment is kept only when Look::parse accepts the whole look and every key is a number or one of the allowed words. Otherwise the fragment is cut. Cases I checked: #range=0,1000?token=…, #dequantize=aef?token=…, #sig=…#range=…, the template, S3-wrapper and login-bound branches, and restore_addresses. Confidence: high.

Security

  • None found. Keys, tokens and signatures in a look are still removed. Any look value outside the closed list (bands, range, dequantize=aef|none, cube=0|1) is dropped with its whole fragment. Confidence: high.

Performance

  • None found. Confidence: high.

Quality

  • crates/engine/src/project.rs:114: when a look is rejected, the notice reports it as a lost key, token or password, even when the lost part is a range or band list. This is disclosed in the PR body and the new test pins the current wording. Confidence: low-medium. Posted inline.

AGENTS.md

  • The "Never silent" rule is only partly met by the wording above. Every other AGENTS.md point I checked is satisfied: no unwrap in non-test code, the tests cover both clean and lossy saves, and the docs change matches the behaviour.

Earlier review threads

  • Two threads already exist and both are resolved and answered by the author. I left both alone: the FORMAT version question, and the Look::parse reader change, which the author has reverted.

Change size

Compared with main (merge base 183ccdd to PR head 0bc424f).

Measure Value
Files changed 3 (added 0, modified 3, removed 0, renamed 0)
Size before 1.84 MB
Size after 1.85 MB
Total size change +7.0 KB
File Before After Change
crates/engine/src/tests.rs 1.74 MB 1.75 MB +6.2 KB
crates/engine/src/project.rs 64.7 KB 65.4 KB +0.7 KB
docs/user-guide/workspace.md 36.3 KB 36.5 KB +163 B

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants