From f1ea29b3f31544d101c6f74bc5e716355286fe6a Mon Sep 17 00:00:00 2001 From: Daniel Hofbauer Date: Mon, 2 Nov 2026 16:47:12 +0100 Subject: [PATCH 01/13] refactor(tiles): record request failures one way RequestDiagnostics::record passes a request's result through and records its failure. The six wrapper/inner pairs on the desktop and in the browser become one function each, the request an inline closure or async block, so each request is created, sent and recorded in one place. A failed local read still makes no network record; a test now says so. --- apps/rustgis-web/src/web.rs | 148 +++++------ crates/tiles/src/cache.rs | 498 ++++++++++++++++++------------------ 2 files changed, 315 insertions(+), 331 deletions(-) diff --git a/apps/rustgis-web/src/web.rs b/apps/rustgis-web/src/web.rs index fe18cec36..25096fba0 100644 --- a/apps/rustgis-web/src/web.rs +++ b/apps/rustgis-web/src/web.rs @@ -578,13 +578,7 @@ async fn fetch_bytes(url: &str, capture: Capture, omit: &'static [u16]) -> Resul use rustgis_ui_egui::rustgis_engine::render::rustgis_tiles::{cache::RequestDiagnostics, failure::Failure}; let mut diagnostics = RequestDiagnostics::with_capture(url, "GET", capture); diagnostics.omit_statuses(omit); - fetch_bytes_inner(url, &mut diagnostics).await.inspect_err(|error| diagnostics.failed(&Failure::new(url, error))) -} - -async fn fetch_bytes_inner( - url: &str, - diagnostics: &mut rustgis_ui_egui::rustgis_engine::render::rustgis_tiles::cache::RequestDiagnostics<'_>, -) -> Result, String> { + let answer = async { let err = |e: wasm_bindgen::JsValue| format!("{e:?}"); let window = web_sys::window().ok_or("no window")?; use rustgis_ui_egui::rustgis_engine::render::rustgis_tiles::cache; @@ -722,7 +716,10 @@ async fn fetch_bytes_inner( if whole_tail.is_some_and(|n| bytes.len() as u64 > n) { return Err(format!("{url}: {}", cache::NO_RANGES)); } - Ok(bytes) + Ok::<_, String>(bytes) + } + .await; + diagnostics.record(answer.map_err(|e| Failure::new(url, e))).map_err(|failure| failure.message) } /// POSTs a JSON document and reads the answer (at most the tile cache's `MAX_BYTES`), without @@ -730,40 +727,36 @@ async fn fetch_bytes_inner( async fn post_json(url: &str, body: &str, capture: Capture) -> Result, String> { use rustgis_ui_egui::rustgis_engine::render::rustgis_tiles::{cache::RequestDiagnostics, failure::Failure}; let mut diagnostics = RequestDiagnostics::with_capture(url, "POST", capture); - post_json_inner(url, body, &mut diagnostics).await.inspect_err(|error| diagnostics.failed(&Failure::new(url, error))) -} - -async fn post_json_inner( - url: &str, - body: &str, - diagnostics: &mut rustgis_ui_egui::rustgis_engine::render::rustgis_tiles::cache::RequestDiagnostics<'_>, -) -> Result, String> { - use rustgis_ui_egui::rustgis_engine::render::rustgis_tiles::cache; - let err = |e: wasm_bindgen::JsValue| format!("{e:?}"); - let window = web_sys::window().ok_or("no window")?; - let init = web_sys::RequestInit::new(); - init.set_method("POST"); - init.set_redirect(web_sys::RequestRedirect::Error); - let headers = web_sys::Headers::new().map_err(err)?; - headers.set("Content-Type", "application/json").map_err(err)?; - headers.set("Accept", "application/json").map_err(err)?; - init.set_headers(&headers); - init.set_body(&wasm_bindgen::JsValue::from_str(body)); - let resp: web_sys::Response = wasm_bindgen_futures::JsFuture::from(window.fetch_with_str_and_init(url, &init)) - .await - .map_err(|_| "the browser couldn't reach it (offline, or the server doesn't accept requests from web pages)".to_string())? - .dyn_into() - .map_err(err)?; - diagnostics.answered(resp.status()); - if !resp.ok() { - return Err(format!("HTTP {}", resp.status())); - } - let buf = wasm_bindgen_futures::JsFuture::from(resp.array_buffer().map_err(err)?).await.map_err(err)?; - let bytes = js_sys::Uint8Array::new(&buf); - if bytes.length() as usize > cache::MAX_BYTES { - return Err(format!("{} bytes is too large", bytes.length())); + let answer = async { + use rustgis_ui_egui::rustgis_engine::render::rustgis_tiles::cache; + let err = |e: wasm_bindgen::JsValue| format!("{e:?}"); + let window = web_sys::window().ok_or("no window")?; + let init = web_sys::RequestInit::new(); + init.set_method("POST"); + init.set_redirect(web_sys::RequestRedirect::Error); + let headers = web_sys::Headers::new().map_err(err)?; + headers.set("Content-Type", "application/json").map_err(err)?; + headers.set("Accept", "application/json").map_err(err)?; + init.set_headers(&headers); + init.set_body(&wasm_bindgen::JsValue::from_str(body)); + let resp: web_sys::Response = wasm_bindgen_futures::JsFuture::from(window.fetch_with_str_and_init(url, &init)) + .await + .map_err(|_| "the browser couldn't reach it (offline, or the server doesn't accept requests from web pages)".to_string())? + .dyn_into() + .map_err(err)?; + diagnostics.answered(resp.status()); + if !resp.ok() { + return Err(format!("HTTP {}", resp.status())); + } + let buf = wasm_bindgen_futures::JsFuture::from(resp.array_buffer().map_err(err)?).await.map_err(err)?; + let bytes = js_sys::Uint8Array::new(&buf); + if bytes.length() as usize > cache::MAX_BYTES { + return Err(format!("{} bytes is too large", bytes.length())); + } + Ok::<_, String>(bytes.to_vec()) } - Ok(bytes.to_vec()) + .await; + diagnostics.record(answer.map_err(|e| Failure::new(url, e))).map_err(|failure| failure.message) } /// Asks for an address's headers (`HEAD`): a TIFF header's announced length and the type. The @@ -772,46 +765,43 @@ async fn post_json_inner( async fn head(url: &str, capture: Capture) -> Result { use rustgis_ui_egui::rustgis_engine::render::rustgis_tiles::{cache::RequestDiagnostics, failure::Failure}; let mut diagnostics = RequestDiagnostics::with_capture(url, "HEAD", capture); - head_inner(url, &mut diagnostics).await.inspect_err(|error| diagnostics.failed(&Failure::new(url, error))) -} - -async fn head_inner( - url: &str, - diagnostics: &mut rustgis_ui_egui::rustgis_engine::render::rustgis_tiles::cache::RequestDiagnostics<'_>, -) -> Result { - use rustgis_ui_egui::rustgis_engine::render::rustgis_tiles::{cache, s3}; - let err = |e: wasm_bindgen::JsValue| format!("{e:?}"); - let window = web_sys::window().ok_or("no window")?; - let prepared = s3::prepare(url)?; - // A request signed for S3 is signed for a GET. - if prepared.signed { - return Ok(cache::Head::default()); - } - let init = web_sys::RequestInit::new(); - init.set_method("HEAD"); - if !prepared.headers.is_empty() { - let headers = web_sys::Headers::new().map_err(err)?; - for (k, v) in &prepared.headers { - headers.set(k, v).map_err(err)?; + let answer = async { + use rustgis_ui_egui::rustgis_engine::render::rustgis_tiles::{cache, s3}; + let err = |e: wasm_bindgen::JsValue| format!("{e:?}"); + let window = web_sys::window().ok_or("no window")?; + let prepared = s3::prepare(url)?; + // A request signed for S3 is signed for a GET. + if prepared.signed { + return Ok(cache::Head::default()); } - init.set_headers(&headers); - } - let resp: web_sys::Response = wasm_bindgen_futures::JsFuture::from(window.fetch_with_str_and_init(&prepared.url, &init)) - .await - .map_err(|_| "the browser couldn't reach it (offline, or the server doesn't accept requests from web pages)".to_string())? - .dyn_into() - .map_err(err)?; - diagnostics.answered(resp.status()); - if !resp.ok() { - return Err(format!("HTTP {}", resp.status())); + let init = web_sys::RequestInit::new(); + init.set_method("HEAD"); + if !prepared.headers.is_empty() { + let headers = web_sys::Headers::new().map_err(err)?; + for (k, v) in &prepared.headers { + headers.set(k, v).map_err(err)?; + } + init.set_headers(&headers); + } + let resp: web_sys::Response = wasm_bindgen_futures::JsFuture::from(window.fetch_with_str_and_init(&prepared.url, &init)) + .await + .map_err(|_| "the browser couldn't reach it (offline, or the server doesn't accept requests from web pages)".to_string())? + .dyn_into() + .map_err(err)?; + diagnostics.answered(resp.status()); + if !resp.ok() { + return Err(format!("HTTP {}", resp.status())); + } + let header = |k: &str| resp.headers().get(k).ok().flatten(); + Ok::<_, String>(cache::Head { + tiff_header_length: header("x-tiff-header-length").as_deref().and_then(cache::tiff_header_length), + content_type: header("content-type"), + // Seen only where the server exposes it to pages. + accepts_ranges: header("accept-ranges").as_deref().is_some_and(cache::accepts_byte_ranges), + }) } - let header = |k: &str| resp.headers().get(k).ok().flatten(); - Ok(cache::Head { - tiff_header_length: header("x-tiff-header-length").as_deref().and_then(cache::tiff_header_length), - content_type: header("content-type"), - // Seen only where the server exposes it to pages. - accepts_ranges: header("accept-ranges").as_deref().is_some_and(cache::accepts_byte_ranges), - }) + .await; + diagnostics.record(answer.map_err(|e| Failure::new(url, e))).map_err(|failure| failure.message) } /// Makes an HTTP request (the AI assistant's calls to its model), returning the response of any diff --git a/crates/tiles/src/cache.rs b/crates/tiles/src/cache.rs index cd884b1d5..8d0bd7ea8 100644 --- a/crates/tiles/src/cache.rs +++ b/crates/tiles/src/cache.rs @@ -654,6 +654,11 @@ impl<'a> RequestDiagnostics<'a> { self.timed_out = true; } + /// A request's result, its failure recorded ([`Self::failed`]) on the way through. + pub fn record(&self, result: Result) -> Result { + result.inspect_err(|failure| self.failed(failure)) + } + pub fn failed(&self, failure: &Failure) { use crate::failure::Reason; use rustgis_data::diagnostics::{Network, NetworkKind}; @@ -1294,56 +1299,52 @@ mod native { /// them. pub(super) fn head_with(agent: &ureq::Agent, url: &str) -> Result { let mut diagnostics = super::RequestDiagnostics::new(url, "HEAD"); - head_answer(agent, url, &mut diagnostics).map_err(|failure| { - diagnostics.failed(&failure); - failure.message - }) - } - - fn head_answer(agent: &ureq::Agent, url: &str, diagnostics: &mut super::RequestDiagnostics<'_>) -> Result { - let prepared = crate::s3::prepare(url)?; - if prepared.signed { - return Ok(super::Head::default()); - } - let mut at = prepared.url.clone(); - for redirects in 0..=5 { - let mut req = agent.head(&at); - if redirects == 0 { - for (k, v) in &prepared.headers { - req = req.header(k.as_str(), v.as_str()); - } + let answer = (|| -> Result { + let prepared = crate::s3::prepare(url)?; + if prepared.signed { + return Ok(super::Head::default()); } - diagnostics.sending(); - let resp = req.config().http_status_as_error(false).build().call().map_err(unanswered)?; - diagnostics.answered(resp.status().as_u16()); - let header = |k: &str| resp.headers().get(k).and_then(|v| v.to_str().ok()); - if resp.status().is_redirection() { - let next = crate::template::join(&at, header("location").ok_or("a redirect with no Location")?); - crate::address::check(&next, Some(&at))?; - if at.starts_with("https://") && !next.starts_with("https://") { - return Err(format!("{next}: a redirect from https to http").into()); + let mut at = prepared.url.clone(); + for redirects in 0..=5 { + let mut req = agent.head(&at); + if redirects == 0 { + for (k, v) in &prepared.headers { + req = req.header(k.as_str(), v.as_str()); + } } - at = next; - continue; - } - if resp.status().as_u16() >= 400 { - // The selected headers say who refused and what it wants: kept, as a GET's - // refusal keeps them, so the history's details aren't only a status. - let headers: Vec<(String, String)> = resp - .headers() - .iter() - .filter(|(k, _)| crate::failure::KEPT_HEADERS.contains(&k.as_str())) - .map(|(k, v)| (k.as_str().to_string(), String::from_utf8_lossy(v.as_bytes()).into_owned())) - .collect(); - return Err(Failure::answered("", resp.status().as_u16(), &headers, &[])); + diagnostics.sending(); + let resp = req.config().http_status_as_error(false).build().call().map_err(unanswered)?; + diagnostics.answered(resp.status().as_u16()); + let header = |k: &str| resp.headers().get(k).and_then(|v| v.to_str().ok()); + if resp.status().is_redirection() { + let next = crate::template::join(&at, header("location").ok_or("a redirect with no Location")?); + crate::address::check(&next, Some(&at))?; + if at.starts_with("https://") && !next.starts_with("https://") { + return Err(format!("{next}: a redirect from https to http").into()); + } + at = next; + continue; + } + if resp.status().as_u16() >= 400 { + // The selected headers say who refused and what it wants: kept, as a GET's + // refusal keeps them, so the history's details aren't only a status. + let headers: Vec<(String, String)> = resp + .headers() + .iter() + .filter(|(k, _)| crate::failure::KEPT_HEADERS.contains(&k.as_str())) + .map(|(k, v)| (k.as_str().to_string(), String::from_utf8_lossy(v.as_bytes()).into_owned())) + .collect(); + return Err(Failure::answered("", resp.status().as_u16(), &headers, &[])); + } + return Ok(super::Head { + tiff_header_length: header("x-tiff-header-length").and_then(super::tiff_header_length), + content_type: header("content-type").map(str::to_string), + accepts_ranges: header("accept-ranges").is_some_and(super::accepts_byte_ranges), + }); } - return Ok(super::Head { - tiff_header_length: header("x-tiff-header-length").and_then(super::tiff_header_length), - content_type: header("content-type").map(str::to_string), - accepts_ranges: header("accept-ranges").is_some_and(super::accepts_byte_ranges), - }); - } - Err("too many redirects".into()) + Err("too many redirects".into()) + })(); + diagnostics.record(answer).map_err(|failure| failure.message) } /// POSTs a JSON document on a thread of its own. @@ -1359,46 +1360,37 @@ mod native { /// POSTs a JSON document and reads the answer (at most [`MAX_BYTES`]); an error status is /// `http status: `, and a redirect isn't followed. pub(super) fn post_with(agent: &ureq::Agent, url: &str, body: &str) -> Result, String> { - let mut diagnostics = super::RequestDiagnostics::new(url, "POST"); - post_answer(agent, url, body, &mut diagnostics).map_err(|failure| { - diagnostics.failed(&failure); - failure.message - }) - } - - fn post_answer( - agent: &ureq::Agent, - url: &str, - body: &str, - diagnostics: &mut super::RequestDiagnostics<'_>, - ) -> Result, Failure> { use std::io::Read; - let mut resp = agent - .post(url) - .header("Content-Type", "application/json") - .header("Accept", "application/json") - .config() - .http_status_as_error(false) - .build() - .send(body) - .map_err(unanswered)?; - let status = resp.status().as_u16(); - diagnostics.answered(status); - if !(200..300).contains(&status) { - return Err(Failure::without_answer("", format!("http status: {status}"), Reason::Status(status))); - } - let mut buf = Vec::new(); - resp.body_mut() - .with_config() - .limit(MAX_BYTES as u64 + 2) - .reader() - .take(MAX_BYTES as u64 + 1) - .read_to_end(&mut buf) - .map_err(|e| e.to_string())?; - if buf.len() > MAX_BYTES { - return Err(format!("larger than {MAX_BYTES} bytes").into()); - } - Ok(buf) + let mut diagnostics = super::RequestDiagnostics::new(url, "POST"); + let answer = (|| -> Result, Failure> { + let mut resp = agent + .post(url) + .header("Content-Type", "application/json") + .header("Accept", "application/json") + .config() + .http_status_as_error(false) + .build() + .send(body) + .map_err(unanswered)?; + let status = resp.status().as_u16(); + diagnostics.answered(status); + if !(200..300).contains(&status) { + return Err(Failure::without_answer("", format!("http status: {status}"), Reason::Status(status))); + } + let mut buf = Vec::new(); + resp.body_mut() + .with_config() + .limit(MAX_BYTES as u64 + 2) + .reader() + .take(MAX_BYTES as u64 + 1) + .read_to_end(&mut buf) + .map_err(|e| e.to_string())?; + if buf.len() > MAX_BYTES { + return Err(format!("larger than {MAX_BYTES} bytes").into()); + } + Ok(buf) + })(); + diagnostics.record(answer).map_err(|failure| failure.message) } /// Downloads on a thread of its own, so a large file doesn't hold up the tiles. @@ -1499,196 +1491,188 @@ mod native { /// as tiles a server doesn't have are routine and only their status is read. A status in /// `omit` is an expected answer: its failure makes no network record. fn fetch_answer(agent: &ureq::Agent, url: &str, detail: bool, omit: &'static [u16]) -> Result, Failure> { + use std::io::Read; // Local file failures are not network traffic. - if crate::mbtiles::is_tile(url) || url.starts_with(LOCAL) { - return fetch_answer_inner(agent, url, detail, &mut super::RequestDiagnostics::new(url, "GET")); - } + let local = crate::mbtiles::is_tile(url) || url.starts_with(LOCAL); let mut diagnostics = super::RequestDiagnostics::new(url, "GET"); diagnostics.omit_statuses(omit); - fetch_answer_inner(agent, url, detail, &mut diagnostics).inspect_err(|failure| diagnostics.failed(failure)) - } - - fn fetch_answer_inner( - agent: &ureq::Agent, - url: &str, - detail: bool, - diagnostics: &mut super::RequestDiagnostics<'_>, - ) -> Result, Failure> { - use std::io::Read; - if crate::mbtiles::is_tile(url) { - return crate::mbtiles::read_local(url).map_err(Failure::from); - } - if let Some(local) = url.strip_prefix(LOCAL) { - let (path, range) = super::split_range(local); - let (a, b) = range.ok_or("a local read without a range")?; - return crate::pmtiles::read_file(path, a, b - a + 1).map_err(Failure::from); - } - let (url, limit) = super::split_download_limit(url); - let max = limit.unwrap_or(MAX_BYTES); - let (url, mut tail) = super::split_tail(url); - let (url, mut range) = super::split_range(url); - // The last bytes asked for, while the file's length is asked for instead (see below). - let mut measuring: Option = None; - // An `s3://` address becomes its web address, and a request to an S3 connection's - // endpoint is signed. - let prepared = crate::s3::prepare(url)?; - let mut at = prepared.url.clone(); - let mut redirects = 0; - let mut resp = loop { - let mut req = agent.get(&at); - if let Some((a, b)) = range { - req = req.header("Range", &format!("bytes={a}-{b}")); - } else if let Some(n) = tail { - req = req.header("Range", &format!("bytes=-{n}")); + let answer = (|| -> Result, Failure> { + if crate::mbtiles::is_tile(url) { + return crate::mbtiles::read_local(url).map_err(Failure::from); } - // A range of the file's own bytes: a server compressing its answer would count the - // range in the compressed ones. - if range.is_some() || tail.is_some() { - req = req.header("Accept-Encoding", "identity"); + if let Some(local) = url.strip_prefix(LOCAL) { + let (path, range) = super::split_range(local); + let (a, b) = range.ok_or("a local read without a range")?; + return crate::pmtiles::read_file(path, a, b - a + 1).map_err(Failure::from); } - if redirects == 0 { - for (k, v) in &prepared.headers { - req = req.header(k.as_str(), v.as_str()); + let (url, limit) = super::split_download_limit(url); + let max = limit.unwrap_or(MAX_BYTES); + let (url, mut tail) = super::split_tail(url); + let (url, mut range) = super::split_range(url); + // The last bytes asked for, while the file's length is asked for instead (see below). + let mut measuring: Option = None; + // An `s3://` address becomes its web address, and a request to an S3 connection's + // endpoint is signed. + let prepared = crate::s3::prepare(url)?; + let mut at = prepared.url.clone(); + let mut redirects = 0; + let mut resp = loop { + let mut req = agent.get(&at); + if let Some((a, b)) = range { + req = req.header("Range", &format!("bytes={a}-{b}")); + } else if let Some(n) = tail { + req = req.header("Range", &format!("bytes=-{n}")); } - } - // An Earthdata Login token goes to NASA's Earthdata data hosts only, asked for at - // each hop: a redirect that leaves them (to a presigned address) goes without it. - if !prepared.signed - && let Some(auth) = crate::earthdata::authorization(&at) - { - req = req.header("Authorization", &auth); - } - // An ArcGIS sign-in's token goes to the portal and the servers it covers, asked for at - // each hop like Earthdata's. - if !prepared.signed - && let Some(auth) = crate::arcgis_auth::authorization_fresh(&at) - { - req = req.header("X-Esri-Authorization", &auth); - } - // An error status is answered, not failed: its headers and body say who refused. - diagnostics.sending(); - let mut resp = req.config().http_status_as_error(false).build().call().map_err(unanswered)?; - let status = resp.status().as_u16(); - diagnostics.answered(status); - // A data host refusing the request (or the token): say what to do. - if status == 401 && crate::earthdata::is_data_host(&at) { - return Err(Failure::without_answer("", crate::earthdata::refused_message(url), Reason::Status(status))); - } - // A store that serves ranges but not the last bytes by their count (GitHub's release - // files, on Azure storage, answer `bytes=-n` with 501): the file's length from a - // one-byte range's `Content-Range`, then the last bytes by their place, at the same - // address. A server that gives no length goes on as one without ranges; the one-byte - // range redirected is asked for again where it leads, and refused fails as any refusal. - if let Some(n) = tail.filter(|_| status == 501 && range.is_none()) { - (measuring, tail, range) = (Some(n), None, Some((0, 0))); - continue; - } - if let Some(n) = measuring.filter(|_| status < 300) { - measuring = None; - let header = resp.headers().get("content-range").and_then(|v| v.to_str().ok()); - match header.filter(|_| status == 206).and_then(content_range_total) { - Some(len) => { - range = Some((len.saturating_sub(n), len - 1)); - // The probe's byte read, so its connection goes back to the pool for the - // next request; a body that won't read is left, the request goes on. - let _ = resp.body_mut().with_config().limit(64).read_to_vec(); - continue; + // A range of the file's own bytes: a server compressing its answer would count the + // range in the compressed ones. + if range.is_some() || tail.is_some() { + req = req.header("Accept-Encoding", "identity"); + } + if redirects == 0 { + for (k, v) in &prepared.headers { + req = req.header(k.as_str(), v.as_str()); } - None => return Err(format!("{url}: {}", super::NO_RANGES).into()), } - } - if status >= 400 { - // A signed request's refusal is told by its status alone: an answer's headers and - // body are the server's text, and a server may echo the request's Authorization - // header (the access key's id) back in it. - return Err(if detail && !prepared.signed { - refused(&mut resp) - } else { - Failure::without_answer("", format!("http status: {status}"), Reason::Status(status)) - }); - } - if !resp.status().is_redirection() { - break resp; - } - // A signed request goes only where its credentials are for. - if prepared.signed { - return Err(format!( + // An Earthdata Login token goes to NASA's Earthdata data hosts only, asked for at + // each hop: a redirect that leaves them (to a presigned address) goes without it. + if !prepared.signed + && let Some(auth) = crate::earthdata::authorization(&at) + { + req = req.header("Authorization", &auth); + } + // An ArcGIS sign-in's token goes to the portal and the servers it covers, asked for at + // each hop like Earthdata's. + if !prepared.signed + && let Some(auth) = crate::arcgis_auth::authorization_fresh(&at) + { + req = req.header("X-Esri-Authorization", &auth); + } + // An error status is answered, not failed: its headers and body say who refused. + diagnostics.sending(); + let mut resp = req.config().http_status_as_error(false).build().call().map_err(unanswered)?; + let status = resp.status().as_u16(); + diagnostics.answered(status); + // A data host refusing the request (or the token): say what to do. + if status == 401 && crate::earthdata::is_data_host(&at) { + return Err(Failure::without_answer("", crate::earthdata::refused_message(url), Reason::Status(status))); + } + // A store that serves ranges but not the last bytes by their count (GitHub's release + // files, on Azure storage, answer `bytes=-n` with 501): the file's length from a + // one-byte range's `Content-Range`, then the last bytes by their place, at the same + // address. A server that gives no length goes on as one without ranges; the one-byte + // range redirected is asked for again where it leads, and refused fails as any refusal. + if let Some(n) = tail.filter(|_| status == 501 && range.is_none()) { + (measuring, tail, range) = (Some(n), None, Some((0, 0))); + continue; + } + if let Some(n) = measuring.filter(|_| status < 300) { + measuring = None; + let header = resp.headers().get("content-range").and_then(|v| v.to_str().ok()); + match header.filter(|_| status == 206).and_then(content_range_total) { + Some(len) => { + range = Some((len.saturating_sub(n), len - 1)); + // The probe's byte read, so its connection goes back to the pool for the + // next request; a body that won't read is left, the request goes on. + let _ = resp.body_mut().with_config().limit(64).read_to_vec(); + continue; + } + None => return Err(format!("{url}: {}", super::NO_RANGES).into()), + } + } + if status >= 400 { + // A signed request's refusal is told by its status alone: an answer's headers and + // body are the server's text, and a server may echo the request's Authorization + // header (the access key's id) back in it. + return Err(if detail && !prepared.signed { + refused(&mut resp) + } else { + Failure::without_answer("", format!("http status: {status}"), Reason::Status(status)) + }); + } + if !resp.status().is_redirection() { + break resp; + } + // A signed request goes only where its credentials are for. + if prepared.signed { + return Err(format!( "{url}: the S3 endpoint answered with a redirect (HTTP {}), which a signed request doesn't follow: check the connection's region and endpoint", resp.status().as_u16() ).into()); + } + let location = resp.headers().get("location").and_then(|v| v.to_str().ok()).ok_or("a redirect with no Location")?; + let next = crate::template::join(&at, location); + // An Earthdata data host sends a request it didn't accept to Earthdata Login. + if crate::earthdata::is_data_host(&at) && crate::earthdata::is_login(&next) { + return Err(Failure::without_answer("", crate::earthdata::refused_message(url), Reason::Status(401))); + } + // Where a redirect leads is the server's choice, not the user's. + crate::address::check(&next, Some(&at))?; + // Never from HTTPS down to plain HTTP. + if at.starts_with("https://") && !next.starts_with("https://") { + return Err(format!("{next}: a redirect from https to http").into()); + } + redirects += 1; + if redirects > 5 { + return Err("too many redirects".into()); + } + at = next; + }; + if resp.status().as_u16() == 204 { + return Ok(Vec::new()); } - let location = resp.headers().get("location").and_then(|v| v.to_str().ok()).ok_or("a redirect with no Location")?; - let next = crate::template::join(&at, location); - // An Earthdata data host sends a request it didn't accept to Earthdata Login. - if crate::earthdata::is_data_host(&at) && crate::earthdata::is_login(&next) { - return Err(Failure::without_answer("", crate::earthdata::refused_message(url), Reason::Status(401))); + // A range asked for and the whole file answered (200): fine for a range from the start + // that holds the whole file (a small archive's header read), cut to the range; otherwise + // the server doesn't do ranges. + let whole = range.is_some() && resp.status().as_u16() != 206; + if let Some((a, _)) = range.filter(|_| whole) + && a > 0 + { + return Err(format!("{url}: {}", super::NO_RANGES).into()); } - // Where a redirect leads is the server's choice, not the user's. - crate::address::check(&next, Some(&at))?; - // Never from HTTPS down to plain HTTP. - if at.starts_with("https://") && !next.starts_with("https://") { - return Err(format!("{next}: a redirect from https to http").into()); + let mut buf = Vec::new(); + if !resp.status().is_success() { + return Err(format!("HTTP {}", resp.status().as_u16()).into()); } - redirects += 1; - if redirects > 5 { - return Err("too many redirects".into()); + // The last bytes asked for and the whole file answered: fine when it is no longer. + if let Some(n) = tail.filter(|_| resp.status().as_u16() != 206) { + // Read one byte past `n` (no more, and no error at the limit): a longer file is + // reported as a server without ranges, so the file is downloaded whole instead. + let limit = n.min(max as u64).saturating_add(1); + resp.body_mut() + .with_config() + .limit(limit.saturating_add(1)) + .reader() + .take(limit) + .read_to_end(&mut buf) + .map_err(|e| e.to_string())?; + if buf.len() as u64 > n { + return Err(format!("{url}: {}", super::NO_RANGES).into()); + } + if buf.len() > max { + return Err(format!("larger than {max} bytes").into()); + } + return Ok(buf); } - at = next; - }; - if resp.status().as_u16() == 204 { - return Ok(Vec::new()); - } - // A range asked for and the whole file answered (200): fine for a range from the start - // that holds the whole file (a small archive's header read), cut to the range; otherwise - // the server doesn't do ranges. - let whole = range.is_some() && resp.status().as_u16() != 206; - if let Some((a, _)) = range.filter(|_| whole) - && a > 0 - { - return Err(format!("{url}: {}", super::NO_RANGES).into()); - } - let mut buf = Vec::new(); - if !resp.status().is_success() { - return Err(format!("HTTP {}", resp.status().as_u16()).into()); - } - // The last bytes asked for and the whole file answered: fine when it is no longer. - if let Some(n) = tail.filter(|_| resp.status().as_u16() != 206) { - // Read one byte past `n` (no more, and no error at the limit): a longer file is - // reported as a server without ranges, so the file is downloaded whole instead. - let limit = n.min(max as u64).saturating_add(1); + // ureq limits the wire bytes; `take` also caps the decoded bytes when gzip expands them. resp.body_mut() .with_config() - .limit(limit.saturating_add(1)) + .limit(max as u64 + 2) .reader() - .take(limit) + .take(max as u64 + 1) .read_to_end(&mut buf) .map_err(|e| e.to_string())?; - if buf.len() as u64 > n { - return Err(format!("{url}: {}", super::NO_RANGES).into()); - } if buf.len() > max { return Err(format!("larger than {max} bytes").into()); } - return Ok(buf); - } - // ureq limits the wire bytes; `take` also caps the decoded bytes when gzip expands them. - resp.body_mut() - .with_config() - .limit(max as u64 + 2) - .reader() - .take(max as u64 + 1) - .read_to_end(&mut buf) - .map_err(|e| e.to_string())?; - if buf.len() > max { - return Err(format!("larger than {max} bytes").into()); - } - if let Some((a, b)) = range.filter(|_| whole) { - if buf.len() as u64 > b - a + 1 { - return Err(format!("{url}: {}", super::NO_RANGES).into()); + if let Some((a, b)) = range.filter(|_| whole) { + if buf.len() as u64 > b - a + 1 { + return Err(format!("{url}: {}", super::NO_RANGES).into()); + } + buf.truncate(usize::try_from(b - a + 1).unwrap_or(usize::MAX)); } - buf.truncate(usize::try_from(b - a + 1).unwrap_or(usize::MAX)); - } - Ok(buf) + Ok(buf) + })(); + if local { answer } else { diagnostics.record(answer) } } } @@ -1985,6 +1969,16 @@ mod tests { assert!(network.duration_ms.is_some_and(|duration| duration >= 1)); } + /// A local read (a PMTiles archive's part on disk) that fails is not network traffic: the + /// history keeps no record of it. + #[test] + fn network_diagnostics_skip_failed_local_reads() { + let history = rustgis_data::diagnostics::History::default(); + let _scope = history.enter(); + native::get(&native::agent(), "local-file:/nonexistent-1155.pmtiles", &[]).unwrap_err(); + assert!(history.records().is_empty(), "{:?}", history.records()); + } + #[test] fn network_diagnostics_ignore_cancellation_and_keep_browser_causes_truthful() { use rustgis_data::diagnostics::{History, NetworkKind}; From 21c8b1f1bb62e35fcbf3753d121afbb18975d2c1 Mon Sep 17 00:00:00 2001 From: Daniel Hofbauer Date: Mon, 2 Nov 2026 16:51:32 +0100 Subject: [PATCH 02/13] fix(diagnostics): header lines, absence notes, 410 tiles, Basic prose, source labels - A kept response header is redacted on its own line: a www-authenticate header no longer drops the headers after it (cf-ray, x-amzn-requestid). - An absence note that leaves the 500-record history frees its server (and the 'further servers' note) to be said again, not only Clear. - Map tiles omit 410 as well as 404: the map draws both as a gap. - 'Basic API configuration failed' keeps its words: a word after Basic is base64-shaped with a digit or punctuation, 16+ characters, or 8+ with a capital inside. - The pane translates the source words Application, Network and Plugin. - Docs: a mid-line 'password: several words' is kept; extra blank lines. --- crates/data/src/diagnostics.rs | 79 +++++++++++++++++++++++------ crates/tiles/src/cache.rs | 44 ++++++++++++---- crates/ui-egui/src/diagnosticlog.rs | 9 +++- docs/privacy.md | 6 ++- docs/user-guide/index.md | 3 +- 5 files changed, 111 insertions(+), 30 deletions(-) diff --git a/crates/data/src/diagnostics.rs b/crates/data/src/diagnostics.rs index eebb64d31..9d626906b 100644 --- a/crates/data/src/diagnostics.rs +++ b/crates/data/src/diagnostics.rs @@ -83,10 +83,11 @@ struct State { generation: u64, records: VecDeque, /// Servers whose expected absences have been said (one note each, so a second misconfigured - /// tile source isn't silent once the first has spoken). Bounded; cleared with the records. - absences_said: Vec, - /// Whether servers past [`MAX_ABSENT_SERVERS`] have been said to go unlisted. - absences_overflow_said: bool, + /// tile source isn't silent once the first has spoken), each with its note's record id: once + /// the note leaves the history, the server is said again. Bounded; cleared with the records. + absences_said: Vec<(String, u64)>, + /// The note that servers past [`MAX_ABSENT_SERVERS`] go unlisted, while the history holds it. + absences_overflow_said: Option, } static CLOCK: Mutex u64>> = Mutex::new(None); @@ -197,7 +198,7 @@ impl History { state.records.clear(); // A source said before the clear is said again: the pane must not stay silent about it. state.absences_said.clear(); - state.absences_overflow_said = false; + state.absences_overflow_said = None; state.generation = state.generation.wrapping_add(1); } @@ -246,17 +247,17 @@ impl Capture { .unwrap_or_default(); // Checked, marked and stored under one lock: a Clear between can't leave the note behind. let mut guard = lock(&state); - if guard.absences_said.contains(&server) { + if guard.absences_said.iter().any(|(said, _)| *said == server) { return; } if guard.absences_said.len() >= MAX_ABSENT_SERVERS { // Past the cap a new server isn't listed, and that is said once. - if std::mem::replace(&mut guard.absences_overflow_said, true) { + if guard.absences_overflow_said.is_some() { return; } let message = format!("expected absences from further servers are not listed: this history names the first {MAX_ABSENT_SERVERS} servers"); - store( + let id = store( &mut guard, Record { id: 0, @@ -268,9 +269,9 @@ impl Capture { network: None, }, ); + guard.absences_overflow_said = Some(id); return; } - guard.absences_said.push(server); // The address without its query or fragment, or a secret in its path, as a failure's // record keeps it. let url = path_secrets_redacted(url.get(..url.find(['?', '#']).unwrap_or(url.len())).unwrap_or("")); @@ -292,7 +293,8 @@ impl Capture { kind: NetworkKind::Http, }), }; - store(&mut guard, record); + let id = store(&mut guard, record); + guard.absences_said.push((server, id)); } /// Retains one sanitized failure only in the originating live session. Callers supply local @@ -321,15 +323,22 @@ impl Capture { } } -/// Adds a record to a locked history: numbered, the oldest let go past [`MAX_RECORDS`]. -fn store(state: &mut State, mut record: Record) { +/// Adds a record to a locked history: numbered, the oldest let go past [`MAX_RECORDS`]. An +/// absence note let go frees its server to be said again. Returns the record's id. +fn store(state: &mut State, mut record: Record) -> u64 { state.next_id = state.next_id.saturating_add(1); state.generation = state.generation.wrapping_add(1); record.id = state.next_id; - if state.records.len() >= MAX_RECORDS { - state.records.pop_front(); + if state.records.len() >= MAX_RECORDS + && let Some(gone) = state.records.pop_front() + { + state.absences_said.retain(|(_, id)| *id != gone.id); + if state.absences_overflow_said == Some(gone.id) { + state.absences_overflow_said = None; + } } state.records.push_back(record); + state.next_id } /// An address (without its query) with the value of each `name=value` in its path that names a @@ -485,9 +494,12 @@ fn credential_shaped(word: &str) -> bool { } /// Whether a word after a scheme word (`Basic`) looks like its base64 credential: shaped like a -/// credential, or a capital past its first letter ("dXNlcjpwYXNz"), so "Basic settings" keeps. +/// credential, or 8 characters or more with a capital past its first letter ("dXNlcjpwYXNz"), so +/// "Basic settings" and "Basic API configuration" keep. A shorter one with neither digit nor +/// padding (`YTpi`, "a:b") reads as a word and keeps too. fn base64_shaped(word: &str) -> bool { - credential_shaped(word) || word.chars().skip(1).any(char::is_uppercase) + let word = word.trim_end_matches(['.', ',', '!', '?', ';', ':', ')']); + credential_shaped(word) || (word.chars().count() >= 8 && word.chars().skip(1).any(char::is_uppercase)) } /// Sanitizes before bounding. Oversized input is omitted *whole*, so a secret can never be @@ -845,6 +857,8 @@ mod tests { "Exported to alice@example.org: 3 rows", "Loaded a@2x.png and parcels@2020", "Basic settings were saved", + "Basic API configuration failed", + "Basic Settings were saved", "Column 'key' is missing", "Layer 'Body Mass' failed", "The 'password' field is required.", @@ -997,6 +1011,39 @@ mod tests { assert_eq!(history.records().len(), MAX_ABSENT_SERVERS + 1); } + #[test] + fn absences_are_said_again_once_their_note_leaves_the_history() { + let history = History::default(); + let capture = history.capture(); + let notes = |url: &str| history.records().iter().filter(|r| r.network.as_ref().is_some_and(|n| n.url == url)).count(); + let url = "https://tiles.example.org/0/0/0.png"; + capture.absence_said(url, 404); + capture.absence_said(url, 404); + assert_eq!(notes(url), 1, "said once while its note is in the history"); + for i in 0..MAX_RECORDS { + history.push(Severity::Info, "Application", &format!("filler {i}"), ""); + } + assert_eq!(notes(url), 0); + capture.absence_said(url, 404); + assert_eq!(notes(url), 1, "said again once its note was let go"); + // So is the note that further servers go unlisted. + let unlisted = + |history: &History| history.records().iter().filter(|r| r.message.contains("further servers are not listed")).count(); + for i in 0..MAX_ABSENT_SERVERS + 1 { + capture.absence_said(&format!("https://tiles{i}.example.org/0/0/0.png"), 404); + } + assert_eq!(unlisted(&history), 1); + for i in 0..MAX_RECORDS { + history.push(Severity::Info, "Application", &format!("filler {i}"), ""); + } + capture.absence_said("https://another.example.org/0/0/0.png", 404); + assert_eq!(history.records().iter().filter(|r| r.network.is_some()).count(), 1, "a freed server is listed"); + for i in 0..MAX_ABSENT_SERVERS { + capture.absence_said(&format!("https://more{i}.example.org/0/0/0.png"), 404); + } + assert_eq!(unlisted(&history), 1, "the unlisted note is said again"); + } + #[test] fn absences_without_a_web_origin_are_said_per_bucket() { let history = History::default(); diff --git a/crates/tiles/src/cache.rs b/crates/tiles/src/cache.rs index 8d0bd7ea8..4c435579e 100644 --- a/crates/tiles/src/cache.rs +++ b/crates/tiles/src/cache.rs @@ -183,12 +183,14 @@ pub fn get(url: &str, want: Want) -> State { request(url, want, &[]) } -/// Like [`get`], for a map tile: a server's 404 for it is routine (the map asks past an area's -/// edges), so it makes no network record of its own; each would flood the history and evict -/// real failures. A picture that isn't a tile (a single-image layer, a layout's picture, a -/// style's sprite sheet) goes through [`get`], so its 404 is recorded. +/// Like [`get`], for a map tile: a server's 404 (no tile there) or 410 (no longer there) for it +/// is routine (the map asks past an area's edges, and the map draws both as a gap: see +/// [`crate::warp::missing`]), so it isn't recorded as a failure; each would flood the history +/// and evict real failures. The omission is noted once per server instead. A picture that isn't +/// a tile (a single-image layer, a layout's picture, a style's sprite sheet) goes through +/// [`get`], so its 404 is recorded. pub fn get_tile(url: &str, want: Want) -> State { - request(url, want, &[404]) + request(url, want, &[404, 410]) } /// [`get`] with the statuses that are expected answers for this fetch. @@ -684,12 +686,12 @@ impl<'a> RequestDiagnostics<'a> { _ => String::new(), }; // Explicitly exclude Answer::excerpt: an arbitrary server response is not diagnostics. + // Each header is redacted on its own line: a credential header (`www-authenticate`) ends + // redaction of the text it is in, which would drop the headers after it. if let Some(answer) = &failure.answer { for (name, value) in &answer.headers { if crate::failure::KEPT_HEADERS.contains(&name.as_str()) { - details.push_str(name); - details.push_str(": "); - details.push_str(value); + details.push_str(&rustgis_data::diagnostics::redact(&format!("{name}: {value}"))); details.push('\n'); } } @@ -1867,6 +1869,28 @@ mod tests { assert!(network_records(&history, &url).is_empty()); } + /// A credential header in a refusal is taken out by itself: the headers after it (which + /// say who refused) are kept. + #[cfg(not(target_arch = "wasm32"))] + #[test] + fn network_diagnostics_keep_headers_after_a_credential_header() { + let history = rustgis_data::diagnostics::History::default(); + let _scope = history.enter(); + let base = serve(vec![ + "HTTP/1.1 403 Forbidden\r\nWWW-Authenticate: Basic realm=\"private-realm\"\r\nRetry-After: 30\r\ncf-ray: 8a1-BOG\r\nX-Amzn-RequestId: amzn-1155\r\nContent-Length: 0\r\nConnection: close\r\n\r\n" + .into(), + ]); + let url = format!("{base}/refused"); + native::head_with(&native::agent(), &url).unwrap_err(); + let records = network_records(&history, &url); + assert_eq!(records.len(), 1); + let details = &records[0].details; + for kept in ["retry-after: 30", "cf-ray: 8a1-BOG", "x-amzn-requestid: amzn-1155"] { + assert!(details.contains(kept), "{kept} in {details:?}"); + } + assert!(!details.contains("private-realm"), "{details:?}"); + } + #[cfg(not(target_arch = "wasm32"))] #[test] fn network_diagnostics_skip_routine_tile_404s_and_keep_other_statuses() { @@ -2468,8 +2492,8 @@ mod tests { let _ = get("test://omit/style.json", Want::Json); let omits = std::mem::take(&mut *OMITS.lock().unwrap()); let omit = |name: &str| omits.iter().find(|(u, _)| u.ends_with(name)).map(|(_, o)| *o); - assert_eq!(omit("tile.png"), Some(&[404][..])); - assert_eq!(omit("tile.pbf"), Some(&[404][..])); + assert_eq!(omit("tile.png"), Some(&[404, 410][..])); + assert_eq!(omit("tile.pbf"), Some(&[404, 410][..])); assert_eq!(omit("picture.png"), Some(&[][..])); assert_eq!(omit("style.json"), Some(&[][..])); } diff --git a/crates/ui-egui/src/diagnosticlog.rs b/crates/ui-egui/src/diagnosticlog.rs index 7a4e91278..53d705583 100644 --- a/crates/ui-egui/src/diagnosticlog.rs +++ b/crates/ui-egui/src/diagnosticlog.rs @@ -77,10 +77,17 @@ fn entry(ui: &mut Ui, record: &Record, t: &Tokens) { Severity::Warning => (tl!("Warning"), t.amber), Severity::Info => (tl!("Info"), t.accent_text), }; + // A source is a command id (kept as written) or one of the history's own words. + let source = match record.source.as_str() { + "Application" => tl!("Application"), + "Network" => tl!("Network"), + "Plugin" => tl!("Plugin"), + command => command, + }; ui.horizontal_wrapped(|ui| { ui.label(RichText::new(time(record.timestamp_ms)).small().monospace().color(t.text_dim)); ui.label(RichText::new(severity).small().strong().color(color)); - ui.label(RichText::new(record.source.as_str()).small().color(t.text_dim)); + ui.label(RichText::new(source).small().color(t.text_dim)); }); ui.add(egui::Label::new(&record.message).wrap().selectable(true)); if !record.details.is_empty() || record.network.is_some() { diff --git a/docs/privacy.md b/docs/privacy.md index 13096e01d..5b89a3ce5 100644 --- a/docs/privacy.md +++ b/docs/privacy.md @@ -13,12 +13,14 @@ diagnostics** explicitly copies the filtered entries; **Clear** removes the hist Credentials are removed before diagnostic text is retained. URL user information, all query strings (including signed URLs), fragments, credential fields, and structured payloads are -omitted conservatively. Request and response bodies are not captured by network diagnostics. +omitted conservatively. A credential field in the middle of a line is removed when its value is +one word or quoted (`… password: hunter2`), but a value of several words reads as prose and is +kept (`… password: correct horse battery` keeps all three), so check such text before sharing. +Request and response bodies are not captured by network diagnostics. Background requests carry their originating session's weak capture token; failures are not broadcast to other embedded or Python sessions. Layer names, file paths and URL paths can still identify private data, so review copied diagnostics before sharing them. - ## Online services - **Locate › By Place or Address** and place search send your query to OpenStreetMap's Nominatim by default. diff --git a/docs/user-guide/index.md b/docs/user-guide/index.md index 77655f0f8..cf66f1be4 100644 --- a/docs/user-guide/index.md +++ b/docs/user-guide/index.md @@ -137,9 +137,10 @@ older entries are discarded. Each message is limited to 2 KiB and technical deta Nothing is saved in `.rgis` projects or sent automatically. Credentials are removed before retention: addresses omit user information, query strings and fragments; credential fields and structured payloads are omitted conservatively, which can also remove surrounding context. +A credential field in the middle of a line is removed when its value is one word or quoted, but +a value of several words reads as prose and is kept (`… password: correct horse battery`). Review copied diagnostics for private layer names or file paths before sharing them. - ## The ribbon | Tab | Groups | From a535fb0a7baaeedde5a2012a60f4f36c83358f21 Mon Sep 17 00:00:00 2001 From: Daniel Hofbauer Date: Mon, 2 Nov 2026 17:45:05 +0100 Subject: [PATCH 03/13] refactor(diagnostics): code-quality pass on the history - A notice already redacted is only bounded when it enters the history (History::push_redacted, Session::notify_redacted), not redacted again. - redact no longer allocates per word: names with nothing to decode aren't copied, scheme words are compared without a String. Key normalisation, ellipsis cut, bidi controls and address cleaning are one helper each. - note_absence (was absence_said) names the request's real method and bounds an oversized address before parsing it. - The copied report prints plain words (status: 503; duration: 7 ms), leaving out what a request didn't have. - Removing URL user information keeps the sentence's colon. - download_blocking_expecting is download_blocking_omitting, like the rest. - Comments say an omitted status is noted once per server; network_failure says what it keeps. Docs: absence notes, the 64-server cap, report format. - Drop two engine test asserts that could not fail. --- apps/rustgis-web/src/web.rs | 4 +- crates/data/src/diagnostics.rs | 333 +++++++++++++++++++++++++-------- crates/engine/src/cmd/zarr.rs | 4 +- crates/engine/src/lib.rs | 17 +- crates/engine/src/tests.rs | 7 +- crates/tiles/src/cache.rs | 29 +-- crates/ui-egui/src/lib.rs | 3 +- docs/privacy.md | 3 +- docs/user-guide/index.md | 16 +- 9 files changed, 305 insertions(+), 111 deletions(-) diff --git a/apps/rustgis-web/src/web.rs b/apps/rustgis-web/src/web.rs index 25096fba0..55c593358 100644 --- a/apps/rustgis-web/src/web.rs +++ b/apps/rustgis-web/src/web.rs @@ -572,8 +572,8 @@ async fn unless_stalled( /// Downloads `url`, or the byte range it asks for (a PMTiles archive's part: see /// `rustgis_tiles::cache::split_range`). -/// A status in `omit` is an expected answer (a tile past a map's edge): its failure makes no -/// record of its own, as on the desktop. +/// A status in `omit` is an expected answer (a tile past a map's edge): its failure isn't +/// recorded, only noted once per server, as on the desktop. async fn fetch_bytes(url: &str, capture: Capture, omit: &'static [u16]) -> Result, String> { use rustgis_ui_egui::rustgis_engine::render::rustgis_tiles::{cache::RequestDiagnostics, failure::Failure}; let mut diagnostics = RequestDiagnostics::with_capture(url, "GET", capture); diff --git a/crates/data/src/diagnostics.rs b/crates/data/src/diagnostics.rs index 9d626906b..5eb2ac83f 100644 --- a/crates/data/src/diagnostics.rs +++ b/crates/data/src/diagnostics.rs @@ -26,6 +26,17 @@ pub enum Severity { Info, } +impl Severity { + /// The word the pane and the report show. + pub fn label(self) -> &'static str { + match self { + Self::Error => "Error", + Self::Warning => "Warning", + Self::Info => "Info", + } + } +} + #[derive(Clone, Copy, Debug, Default, PartialEq, Eq)] pub enum Filter { #[default] @@ -57,6 +68,18 @@ pub enum NetworkKind { Other, } +impl NetworkKind { + /// The words the pane (in English) and the report show. + pub fn label(self) -> &'static str { + match self { + Self::Http => "HTTP failure", + Self::Connection => "Connection failure", + Self::Timeout => "Timeout", + Self::Other => "Request failure", + } + } +} + #[derive(Clone, Debug, PartialEq, Eq)] pub struct Network { pub url: String, @@ -184,6 +207,21 @@ impl History { }); } + /// [`History::push`] for text already through [`redact`] (a notice the session redacted for + /// its status bar): it is only bounded, so a notice isn't redacted again for each copy. Text + /// that might hold a credential goes to `push`. + pub fn push_redacted(&self, severity: Severity, source: &str, message: &str, details: &str) { + self.retain(Record { + id: 0, + timestamp_ms: timestamp_ms(), + severity, + source: truncated(source, MAX_SOURCE_BYTES), + message: truncated(message, MAX_MESSAGE_BYTES), + details: truncated(details, MAX_DETAILS_BYTES), + network: None, + }); + } + fn retain(&self, record: Record) { store(&mut lock(&self.0), record); } @@ -207,23 +245,30 @@ impl History { lock(&self.0).generation } - /// Plain text suitable for a clipboard or a troubleshooting attachment. Timestamps are UTC - /// Unix milliseconds, explicitly labelled so they cannot be mistaken for local time. + /// Plain text suitable for a clipboard or a troubleshooting attachment, oldest entry first. + /// Timestamps are UTC Unix milliseconds, explicitly labelled so they cannot be mistaken for + /// local time; a status or duration the request didn't have is left out. pub fn report(&self, filter: Filter) -> String { let records = self.records(); let mut report = String::from("RustGIS Diagnostics\nTimestamps: Unix milliseconds (UTC)\n"); for record in records.iter().filter(|record| filter.matches(record)) { - let _ = - writeln!(report, "\n#{} {} {:?} [{}] {}", record.id, record.timestamp_ms, record.severity, record.source, record.message); + let _ = writeln!( + report, + "\n#{} {} {} [{}] {}", + record.id, + record.timestamp_ms, + record.severity.label(), + record.source, + record.message + ); if !record.details.is_empty() { let _ = writeln!(report, "{}", record.details); } if let Some(network) = &record.network { - let _ = writeln!( - report, - "{} {}\nKind: {:?}; status: {:?}; duration_ms: {:?}", - network.method, network.url, network.kind, network.status, network.duration_ms - ); + let mut facts = vec![format!("Kind: {}", network.kind.label())]; + facts.extend(network.status.map(|status| format!("status: {status}"))); + facts.extend(network.duration_ms.map(|ms| format!("duration: {ms} ms"))); + let _ = writeln!(report, "{} {}\n{}", network.method, network.url, facts.join("; ")); } } report @@ -233,13 +278,17 @@ impl History { impl Capture { /// Says once per server, per history, that expected absences (a tile past a map's edge, a /// store's absent chunk) are omitted: the pane is not silent about a misconfigured source - /// while the history stays free of their flood. Later absences from that server add nothing. - pub fn absence_said(&self, url: &str, status: u16) { + /// while the history stays free of their flood. Later absences from that server add nothing, + /// until its note leaves the history. + pub fn note_absence(&self, method: &str, url: &str, status: u16) { let Some(state) = self.0.upgrade() else { return }; // The server is the address's origin (scheme, host and port): two tile sources on one // host but different ports are two sources. An address without a web origin (`s3://`) - // is its scheme and host, so two buckets are two sources. - let server = url::Url::parse(url) + // is its scheme and host, so two buckets are two sources. An oversized address isn't + // parsed: it is one source of its own. + let server = (url.len() <= MAX_INPUT_BYTES) + .then(|| url::Url::parse(url).ok()) + .flatten() .map(|u| { let origin = u.origin(); if origin.is_tuple() { origin.ascii_serialization() } else { format!("{}://{}", u.scheme(), u.host_str().unwrap_or("")) } @@ -272,9 +321,7 @@ impl Capture { guard.absences_overflow_said = Some(id); return; } - // The address without its query or fragment, or a secret in its path, as a failure's - // record keeps it. - let url = path_secrets_redacted(url.get(..url.find(['?', '#']).unwrap_or(url.len())).unwrap_or("")); + let url = clean_address(url); let record = Record { id: 0, timestamp_ms: timestamp_ms(), @@ -285,32 +332,20 @@ impl Capture { MAX_MESSAGE_BYTES, ), details: String::new(), - network: Some(Network { - url: bounded(&url, MAX_URL_BYTES), - method: "GET".to_string(), - status: Some(status), - duration_ms: None, - kind: NetworkKind::Http, - }), + network: Some(Network { url, method: known_method(method), status: Some(status), duration_ms: None, kind: NetworkKind::Http }), }; let id = store(&mut guard, record); guard.absences_said.push((server, id)); } - /// Retains one sanitized failure only in the originating live session. Callers supply local - /// classifications/metadata only, never bodies, arbitrary server text, or header dumps. + /// Retains one sanitized failure only in the originating live session. Callers supply the + /// request's own facts (address, method, status, duration) and its failure's message: the + /// transport's, or the browser's. `details` may hold the few response headers a refusal + /// keeps. A response body is never passed. Everything is redacted and bounded here. pub fn network_failure(&self, source: &str, message: &str, details: &str, mut network: Network) { let Some(state) = self.0.upgrade() else { return }; - network.url = if network.url.len() > MAX_INPUT_BYTES { - "[oversized address omitted]".to_string() - } else { - let end = network.url.find(['?', '#']).unwrap_or(network.url.len()); - bounded(&path_secrets_redacted(network.url.get(..end).unwrap_or("")), MAX_URL_BYTES) - }; - network.method = match network.method.as_str() { - "GET" | "HEAD" | "POST" | "PUT" | "PATCH" | "DELETE" | "OPTIONS" => network.method, - _ => "OTHER".to_string(), - }; + network.url = clean_address(&network.url); + network.method = known_method(&network.method); History(state).retain(Record { id: 0, timestamp_ms: timestamp_ms(), @@ -323,6 +358,24 @@ impl Capture { } } +/// An address as a record keeps it: without its query or fragment, a secret in its path taken +/// out, bounded; one over [`MAX_INPUT_BYTES`] is omitted whole. +fn clean_address(url: &str) -> String { + if url.len() > MAX_INPUT_BYTES { + return "[oversized address omitted]".to_string(); + } + let end = url.find(['?', '#']).unwrap_or(url.len()); + bounded(&path_secrets_redacted(url.get(..end).unwrap_or("")), MAX_URL_BYTES) +} + +/// The request's method when it is one of the usual ones, else `OTHER`. +fn known_method(method: &str) -> String { + match method { + "GET" | "HEAD" | "POST" | "PUT" | "PATCH" | "DELETE" | "OPTIONS" => method.to_string(), + _ => "OTHER".to_string(), + } +} + /// Adds a record to a locked history: numbered, the oldest let go past [`MAX_RECORDS`]. An /// absence note let go frees its server to be said again. Returns the record's id. fn store(state: &mut State, mut record: Record) -> u64 { @@ -371,26 +424,57 @@ fn path_secrets_redacted(address: &str) -> String { /// Redacts, then bounds: a record's copy of a message or of details. fn bounded(text: &str, limit: usize) -> String { let mut clean = redact(text); - if clean.len() > limit { + cut_with_ellipsis(&mut clean, limit); + clean +} + +/// `text` cut to `limit` bytes at a character, an ellipsis marking the cut. +fn cut_with_ellipsis(text: &mut String, limit: usize) { + if text.len() > limit { let mut end = limit.saturating_sub(3); - while !clean.is_char_boundary(end) { + while !text.is_char_boundary(end) { end = end.saturating_sub(1); } - clean.truncate(end); - clean.push('…'); + text.truncate(end); + text.push('…'); } - clean +} + +fn truncated(text: &str, limit: usize) -> String { + let mut text = text.to_string(); + cut_with_ellipsis(&mut text, limit); + text +} + +/// A bidirectional embedding, override or isolate control: it reorders the text around it. +fn bidi_control(c: char) -> bool { + matches!(c, '\u{202a}'..='\u{202e}' | '\u{2066}'..='\u{2069}') +} + +/// A character kept out of retained text: a control or a bidi control. +fn hostile_control(c: char) -> bool { + c.is_control() || bidi_control(c) } fn separator(c: char) -> bool { c.is_whitespace() - || c.is_control() - || matches!(c, '\u{202a}'..='\u{202e}' | '\u{2066}'..='\u{2069}' | ':' | '=' | ';' | ',' | '?' | '&' | '#' | '@' | '"' | '\'' | '{' | '}' | '[' | ']' | '<' | '>' | '(' | ')' | '`') + || hostile_control(c) + || matches!(c, ':' | '=' | ';' | ',' | '?' | '&' | '#' | '@' | '"' | '\'' | '{' | '}' | '[' | ']' | '<' | '>' | '(' | ')' | '`') } -fn secret_name(key: &str) -> bool { - // Decode only names, never values: decoding a URL value could introduce whitespace and - // incorrectly end its redacted query. Work and allocation are capped by MAX_INPUT_BYTES. +/// A name's letters and digits in lower case: how credential names are compared, so `Pass-Word`, +/// `pass_word` and `PASSWORD` are one name. +fn normalized_key(key: &str) -> impl Iterator + '_ { + key.chars().filter(char::is_ascii_alphanumeric).map(|c| c.to_ascii_lowercase()) +} + +/// Whether `key` is `target` (lower case, letters and digits) however it is written. +fn key_is(key: &str, target: &str) -> bool { + normalized_key(key).eq(target.chars()) +} + +/// A name with its percent and `\u` escapes decoded, to the depth an attacker could nest them. +fn decoded_name(key: &str) -> String { let mut name = key.to_string(); for _ in 0..4 { let bytes = name.as_bytes(); @@ -423,8 +507,22 @@ fn secret_name(key: &str) -> bool { } name = decoded; } - let normalized: String = name.chars().filter(|c| c.is_ascii_alphanumeric()).flat_map(char::to_lowercase).collect(); - crate::source::credentials::secret_key(&name) + name +} + +fn secret_name(key: &str) -> bool { + // Decode only names, never values: decoding a URL value could introduce whitespace and + // incorrectly end its redacted query. Work and allocation are capped by MAX_INPUT_BYTES, and + // a name with nothing to decode is not copied. + let decoded; + let name = if key.contains(['%', '\\']) { + decoded = decoded_name(key); + decoded.as_str() + } else { + key + }; + let normalized: String = normalized_key(name).collect(); + crate::source::credentials::secret_key(name) || crate::source::credentials::secret_key(&normalized) || normalized.contains("credential") || normalized.contains("connectionstring") @@ -444,8 +542,9 @@ fn secret_name(key: &str) -> bool { /// Whether the name is an HTTP credential header: its value is always a scheme and a credential /// (or, for cookies, a list of them), however many words it holds. fn credential_header(key: &str) -> bool { - let normalized: String = key.chars().filter(|c| c.is_ascii_alphanumeric()).flat_map(char::to_lowercase).collect(); - matches!(normalized.as_str(), "authorization" | "proxyauthorization" | "wwwauthenticate" | "proxyauthenticate" | "cookie" | "setcookie") + ["authorization", "proxyauthorization", "wwwauthenticate", "proxyauthenticate", "cookie", "setcookie"] + .iter() + .any(|name| key_is(key, name)) } /// Whether `{`, `[` or `<` opens structured content (a JSON value or a tag) rather than an @@ -479,7 +578,7 @@ fn opens_structured(rest: &str) -> bool { /// A bare credential line is redacted only for an unambiguous secret name: `key`, `body` and /// their like read as ordinary words ("Key column 'id' is missing"). fn bare_name(key: &str) -> bool { - let normalized: String = key.chars().filter(|c| c.is_ascii_alphanumeric()).flat_map(char::to_lowercase).collect(); + let normalized: String = normalized_key(key).collect(); !matches!(normalized.as_str(), "key" | "body" | "userid" | "username" | "authentication" | "wwwauthenticate" | "proxyauthenticate") && !normalized.contains("body") && secret_name(key) @@ -587,9 +686,7 @@ pub fn redact(text: &str) -> String { && url.password().is_none() && url.query().is_none() && url.fragment().is_none() - && !address - .chars() - .any(|c| c.is_control() || matches!(c, '\u{202a}'..='\u{202e}' | '\u{2066}'..='\u{2069}')) => + && !address.chars().any(hostile_control) => { out.push_str(&path_secrets_redacted(address)); } @@ -599,6 +696,11 @@ pub fn redact(text: &str) -> String { url.set_query(None); url.set_fragment(None); out.push_str(&path_secrets_redacted(url.as_str())); + // An empty port's colon ends the address as a sentence's does, and + // parsing drops it; with a query, the note below puts it back. + if address.ends_with(':') && !url.as_str().ends_with(':') && !address.contains(['?', '#']) { + out.push(':'); + } } _ => out.push_str("[address omitted]"), } @@ -657,8 +759,7 @@ pub fn redact(text: &str) -> String { } } // A two-word key ("User ID=alice") is one field name across the space. - let name: String = atom.chars().filter(|c| c.is_ascii_alphanumeric()).flat_map(char::to_lowercase).collect(); - if name == "user" + if key_is(atom, "user") && after_space.split(separator).next().is_some_and(|next| { next.eq_ignore_ascii_case("id") && after_space.get(next.len()..).is_some_and(|value| value.trim_start().starts_with([':', '='])) @@ -670,7 +771,8 @@ pub fn redact(text: &str) -> String { // A `Bearer ` is a credential wherever it sits ("server: Bearer xyz"); a // `Basic ` too, but only after a field name, so "Basic plan" keeps. The token // goes with it, and the rest of the line keeps. - if (name == "bearer" || (name == "basic" && field_before)) && after.chars().next().is_some_and(char::is_whitespace) { + if (key_is(atom, "bearer") || (key_is(atom, "basic") && field_before)) && after.chars().next().is_some_and(char::is_whitespace) + { out.push_str(REDACTED); field_before = false; let value = after.trim_start(); @@ -703,20 +805,13 @@ pub fn redact(text: &str) -> String { i += end; } else { // Drop control/escape characters (including terminal escapes and bidi controls). - if c == '\n' || c == '\t' || !c.is_control() && !matches!(c, '\u{202a}'..='\u{202e}' | '\u{2066}'..='\u{2069}') { + if c == '\n' || c == '\t' || !hostile_control(c) { out.push(c); } i += c.len_utf8(); } } - if out.len() > MAX_DETAILS_BYTES { - let mut end = MAX_DETAILS_BYTES.saturating_sub(3); - while !out.is_char_boundary(end) { - end = end.saturating_sub(1); - } - out.truncate(end); - out.push('…'); - } + cut_with_ellipsis(&mut out, MAX_DETAILS_BYTES); out } @@ -996,7 +1091,7 @@ mod tests { let history = History::default(); let capture = history.capture(); for i in 0..MAX_ABSENT_SERVERS + 3 { - capture.absence_said(&format!("https://tiles{i}.example.org/0/0/0.png"), 404); + capture.note_absence("GET", &format!("https://tiles{i}.example.org/0/0/0.png"), 404); } let records = history.records(); // One note for each server up to the cap, then one saying further servers aren't listed. @@ -1006,7 +1101,7 @@ mod tests { // Clear says it again. history.clear(); for i in 0..MAX_ABSENT_SERVERS + 3 { - capture.absence_said(&format!("https://tiles{i}.example.org/0/0/0.png"), 404); + capture.note_absence("GET", &format!("https://tiles{i}.example.org/0/0/0.png"), 404); } assert_eq!(history.records().len(), MAX_ABSENT_SERVERS + 1); } @@ -1017,29 +1112,29 @@ mod tests { let capture = history.capture(); let notes = |url: &str| history.records().iter().filter(|r| r.network.as_ref().is_some_and(|n| n.url == url)).count(); let url = "https://tiles.example.org/0/0/0.png"; - capture.absence_said(url, 404); - capture.absence_said(url, 404); + capture.note_absence("GET", url, 404); + capture.note_absence("GET", url, 404); assert_eq!(notes(url), 1, "said once while its note is in the history"); for i in 0..MAX_RECORDS { history.push(Severity::Info, "Application", &format!("filler {i}"), ""); } assert_eq!(notes(url), 0); - capture.absence_said(url, 404); + capture.note_absence("GET", url, 404); assert_eq!(notes(url), 1, "said again once its note was let go"); // So is the note that further servers go unlisted. let unlisted = |history: &History| history.records().iter().filter(|r| r.message.contains("further servers are not listed")).count(); for i in 0..MAX_ABSENT_SERVERS + 1 { - capture.absence_said(&format!("https://tiles{i}.example.org/0/0/0.png"), 404); + capture.note_absence("GET", &format!("https://tiles{i}.example.org/0/0/0.png"), 404); } assert_eq!(unlisted(&history), 1); for i in 0..MAX_RECORDS { history.push(Severity::Info, "Application", &format!("filler {i}"), ""); } - capture.absence_said("https://another.example.org/0/0/0.png", 404); + capture.note_absence("GET", "https://another.example.org/0/0/0.png", 404); assert_eq!(history.records().iter().filter(|r| r.network.is_some()).count(), 1, "a freed server is listed"); for i in 0..MAX_ABSENT_SERVERS { - capture.absence_said(&format!("https://more{i}.example.org/0/0/0.png"), 404); + capture.note_absence("GET", &format!("https://more{i}.example.org/0/0/0.png"), 404); } assert_eq!(unlisted(&history), 1, "the unlisted note is said again"); } @@ -1048,9 +1143,9 @@ mod tests { fn absences_without_a_web_origin_are_said_per_bucket() { let history = History::default(); let capture = history.capture(); - capture.absence_said("s3://first-bucket/tiles/0/0/0.png", 404); - capture.absence_said("s3://first-bucket/tiles/1/1/1.png", 404); - capture.absence_said("s3://second-bucket/tiles/0/0/0.png", 404); + capture.note_absence("GET", "s3://first-bucket/tiles/0/0/0.png", 404); + capture.note_absence("GET", "s3://first-bucket/tiles/1/1/1.png", 404); + capture.note_absence("GET", "s3://second-bucket/tiles/0/0/0.png", 404); assert_eq!(history.records().len(), 2, "one note for each bucket"); } @@ -1058,7 +1153,7 @@ mod tests { fn secrets_in_a_url_path_are_redacted() { let history = History::default(); let capture = history.capture(); - capture.absence_said("https://tiles.example.org/token=hidden-path-value/0/0/0.png", 404); + capture.note_absence("GET", "https://tiles.example.org/token=hidden-path-value/0/0/0.png", 404); failed(&capture, "failed"); let report = history.report(Filter::All); assert!(!report.contains("hidden-path-value"), "{report}"); @@ -1083,7 +1178,87 @@ mod tests { fn addresses_without_anything_to_take_out_stay_as_written() { assert_eq!(redact("Could not reach https://example.org: timeout"), "Could not reach https://example.org: timeout"); assert_eq!(redact("failed https://example.org/a/b, retrying"), "failed https://example.org/a/b, retrying"); - // User info still goes, with the address rewritten without it. - assert_eq!(redact("Could not reach https://alice:hidden@example.org: timeout"), "Could not reach https://example.org/ timeout"); + } + + /// Taking user information out rewrites the address, and the punctuation that ended it in the + /// sentence stays with the sentence. + #[test] + fn removing_user_information_keeps_the_sentence_punctuation() { + assert_eq!(redact("Could not reach https://alice:hidden@example.org: timeout"), "Could not reach https://example.org/: timeout"); + assert_eq!( + redact("Could not reach https://alice:hidden@example.org/x. Retrying"), + "Could not reach https://example.org/x. Retrying" + ); + assert_eq!(redact("(see https://alice:hidden@example.org/x)"), "(see https://example.org/x)"); + } + + /// Redacting what is already redacted changes nothing: a notice is redacted once and bounded + /// afterwards, and a command's failure is told apart from its notice by the same text. + #[test] + fn redaction_is_idempotent() { + for text in [ + "Authorization: Bearer hidden", + "failed\ntoken abc123", + "Could not reach https://alice:hidden@example.org/x?token=a: timeout", + "x=1 password=hunter2 rest", + "B.earer hidden then more", + "Layer {2020} failed \u{1}\u{202e}", + "Basic dXNlcjpwYXNz", + "{\"a\": 1}", + ] { + let once = redact(text); + assert_eq!(redact(&once), once, "{text}"); + } + // A name written around its punctuation is still the name. + assert_eq!(redact("server: B.earer hidden"), "server: [REDACTED]"); + assert_eq!(redact("server: Ba-sic dXNlcjpwYXNz"), "server: [REDACTED]"); + assert_eq!(redact("pass%77ord=x"), "[REDACTED]"); + } + + #[test] + fn text_already_redacted_is_only_bounded() { + let history = History::default(); + let long = "word ".repeat(1000); + history.push_redacted(Severity::Info, "Application", &long, &long); + history.push(Severity::Info, "Application", &long, &long); + let records = history.records(); + assert_eq!(records[0].message, records[1].message); + assert_eq!(records[0].details, records[1].details); + assert!(records[0].message.len() <= MAX_MESSAGE_BYTES && records[0].message.ends_with('…')); + } + + #[test] + fn absence_notes_name_the_method_and_bound_oversized_addresses() { + let history = History::default(); + let capture = history.capture(); + capture.note_absence("HEAD", "https://tiles.example.org/0/0/0.png", 404); + capture.note_absence("BREW", "https://other.example.org/0/0/0.png", 404); + let long = format!("https://big.example.org/{}", "a".repeat(MAX_INPUT_BYTES)); + capture.note_absence("GET", &long, 404); + capture.note_absence("GET", &long, 404); + let records = history.records(); + let method = |i: usize| records[i].network.as_ref().map(|n| n.method.as_str()); + assert_eq!((method(0), method(1)), (Some("HEAD"), Some("OTHER"))); + assert_eq!(records.len(), 3, "an oversized address is one source, said once"); + assert_eq!(records[2].network.as_ref().map(|n| n.url.as_str()), Some("[oversized address omitted]")); + } + + #[test] + fn reports_print_plain_values_and_leave_out_what_is_absent() { + let history = History::default(); + let capture = history.capture(); + let network = |status, duration_ms| Network { + url: "https://example.org/x".into(), + method: "GET".into(), + status, + duration_ms, + kind: NetworkKind::Http, + }; + capture.network_failure("Network", "http status: 503", "", network(Some(503), Some(7))); + capture.network_failure("Network", "the browser couldn't reach it", "", network(None, None)); + let report = history.report(Filter::All); + assert!(report.contains("Kind: HTTP failure; status: 503; duration: 7 ms"), "{report}"); + assert!(report.contains("Kind: HTTP failure\n"), "{report}"); + assert!(!report.contains("Some(") && !report.contains("None"), "{report}"); } } diff --git a/crates/engine/src/cmd/zarr.rs b/crates/engine/src/cmd/zarr.rs index 0ea442ee8..a1dc508bf 100644 --- a/crates/engine/src/cmd/zarr.rs +++ b/crates/engine/src/cmd/zarr.rs @@ -104,10 +104,10 @@ fn fetch(root: &str, keys: &[String]) -> Result<(HashMap>, usize break; } // A missing file is an expected answer here (see `is_absent`), not a failure: its 404 - // or 403 makes no network record of its own (one note per server says the + // or 403 isn't recorded one by one (one note per server says the // omission, and a store refusing everything still says "access denied"), while // a real one (a 500, a timeout) does, with its address, status and duration. - let got = crate::render::rustgis_tiles::cache::download_blocking_expecting(&format!("{root}/{k}"), &[404, 403]); + let got = crate::render::rustgis_tiles::cache::download_blocking_omitting(&format!("{root}/{k}"), &[404, 403]); match got { Ok(b) => { let len = b.len() as u64; diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index d0dab5d78..a53e03579 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -1094,7 +1094,7 @@ impl Session { if let Err(error) = &res { let text = rustgis_data::diagnostics::redact(error); if !self.notices_since(notice_mark).iter().any(|notice| notice.error && notice.text == text) { - self.notify(text, true); + self.notify_redacted(text, true); } } res @@ -1501,9 +1501,20 @@ impl Session { fn notice(&mut self, text: String, severity: rustgis_data::diagnostics::Severity) { // Redacted for the status bar and the history alike. `redact` bounds the notice to 8 KiB // (an ellipsis marks the cut) and omits one over 16 KiB whole, so a clipped credential - // can't survive; the history's copy is bounded further, to a record's 2 KiB, in `push`. + // can't survive; the history's copy is bounded further, to a record's 2 KiB. let text = rustgis_data::diagnostics::redact(&text); - self.diagnostics.push(severity, self.diagnostic_source.unwrap_or("Application"), &text, ""); + self.notice_redacted(text, severity); + } + + /// [`Session::notify`] for text that is already through [`rustgis_data::diagnostics::redact`] + /// (a failure the caller redacted to compare it with the notices): not redacted again. + pub fn notify_redacted(&mut self, text: String, error: bool) { + use rustgis_data::diagnostics::Severity; + self.notice_redacted(text, if error { Severity::Error } else { Severity::Info }); + } + + fn notice_redacted(&mut self, text: String, severity: rustgis_data::diagnostics::Severity) { + self.diagnostics.push_redacted(severity, self.diagnostic_source.unwrap_or("Application"), &text, ""); self.notices.push(Notice { text, error: severity == rustgis_data::diagnostics::Severity::Error }); self.notices_given += 1; if self.notices.len() > 200 { diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 6670e7e5e..64e8b0f0f 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -11496,7 +11496,7 @@ fn notices_since_survives_the_cap() { } #[test] -fn diagnostics_capture_notices_without_an_open_pane_or_project_persistence() { +fn diagnostics_capture_notices_without_an_open_pane_and_survive_a_new_project() { use rustgis_data::diagnostics::{Filter, MAX_DETAILS_BYTES, MAX_MESSAGE_BYTES, Severity}; let mut s = Session::new(Project::default()); let mark = s.notice_mark(); @@ -11511,8 +11511,6 @@ fn diagnostics_capture_notices_without_an_open_pane_or_project_persistence() { assert_eq!(records.iter().map(|r| r.severity).collect::>(), [Severity::Info, Severity::Warning, Severity::Error]); assert!(records.iter().all(|r| r.source == "Application" && r.timestamp_ms > 0)); assert!(!s.diagnostics.report(Filter::All).contains("never-retain-this")); - let saved = serde_json::to_string(&s.project).unwrap(); - assert!(!saved.contains("diagnostics-session-only")); s.notify("界".repeat(3000), false); // The notice keeps its words to redaction's own bound (8 KiB, an ellipsis at the cut), not // the record's 2 KiB; only the history's record copy takes the message bound. @@ -11525,9 +11523,6 @@ fn diagnostics_capture_notices_without_an_open_pane_or_project_persistence() { let records = s.diagnostics.records(); assert!(records.iter().any(|r| r.message == "diagnostics-session-only information")); assert!(records.iter().any(|r| r.message == "diagnostics-session-only warning")); - // A separate session restoring the saved project must not inherit troubleshooting history. - let reopened = Session::new(serde_json::from_str(&saved).unwrap()); - assert!(!reopened.diagnostics.records().iter().any(|r| r.message.starts_with("diagnostics-session-only"))); } #[test] diff --git a/crates/tiles/src/cache.rs b/crates/tiles/src/cache.rs index 4c435579e..f82434103 100644 --- a/crates/tiles/src/cache.rs +++ b/crates/tiles/src/cache.rs @@ -622,7 +622,8 @@ pub struct RequestDiagnostics<'a> { status: Option, timed_out: bool, /// Statuses the caller expects (a tile past a map's edge, a store's absent chunk): their - /// failures make no network record, as each would flood the history. + /// failures aren't recorded one by one, as each would flood the history. One note per server + /// says that they're omitted ([`rustgis_data::diagnostics::Capture::note_absence`]). omit: &'static [u16], } @@ -636,7 +637,8 @@ impl<'a> RequestDiagnostics<'a> { Self { url, method, capture, started: Instant::now(), status: None, timed_out: false, omit: &[] } } - /// Statuses whose failures are expected answers, not records (see [`Self::omit`]). + /// Statuses whose failures are expected answers, noted once per server, not recorded each + /// (a tile past a map's edge, a store's absent chunk). pub fn omit_statuses(&mut self, omit: &'static [u16]) { self.omit = omit; } @@ -677,7 +679,7 @@ impl<'a> RequestDiagnostics<'a> { if let Some(status) = self.status && self.omit.contains(&status) { - self.capture.absence_said(url, status); + self.capture.note_absence(self.method, url, status); return; } let mut details = match (range, tail) { @@ -783,11 +785,11 @@ pub fn download_blocking_limited(url: &str, max: usize) -> Result, Strin download_blocking_within(url, max, BLOCKING_TIMEOUT).map_err(large_file_hint) } -/// Like [`download_blocking`], but a status in `omit` is an expected answer: its failure makes no -/// network record of its own (a store's absent chunk, which the caller counts itself). +/// Like [`download_blocking`], but a status in `omit` is an expected answer: its failure isn't +/// recorded, only noted once per server (a store's absent chunk, which the caller counts itself). #[cfg(not(target_arch = "wasm32"))] -pub fn download_blocking_expecting(url: &str, omit: &'static [u16]) -> Result, String> { - download_blocking_omitting(url, MAX_DOWNLOAD, BLOCKING_TIMEOUT, omit).map_err(large_file_hint) +pub fn download_blocking_omitting(url: &str, omit: &'static [u16]) -> Result, String> { + download_blocking_omitting_within(url, MAX_DOWNLOAD, BLOCKING_TIMEOUT, omit).map_err(large_file_hint) } /// A blocking download that ran out of time says how to add a large file instead. @@ -803,13 +805,14 @@ fn large_file_hint(e: String) -> String { /// Like [`download_blocking_limited`], giving up after `timeout` (with the bare error). #[cfg(not(target_arch = "wasm32"))] pub fn download_blocking_within(url: &str, max: usize, timeout: std::time::Duration) -> Result, String> { - download_blocking_omitting(url, max, timeout, &[]) + download_blocking_omitting_within(url, max, timeout, &[]) } /// Like [`download_blocking_within`], but a status in `omit` is an expected answer: its failure -/// makes no network record (a store's absent chunk, which the caller counts itself). +/// isn't recorded, only noted once per server (a store's absent chunk, which the caller counts +/// itself). #[cfg(not(target_arch = "wasm32"))] -fn download_blocking_omitting(url: &str, max: usize, timeout: std::time::Duration, omit: &'static [u16]) -> Result, String> { +fn download_blocking_omitting_within(url: &str, max: usize, timeout: std::time::Duration, omit: &'static [u16]) -> Result, String> { crate::address::check(url, None)?; native::get(&native::download_agent(timeout), &download_address(url, max), omit) } @@ -933,8 +936,8 @@ pub fn head(url: &str, done: HeadDone) { } /// Fetches an address's bytes, uncached (through the browser's fetcher, or the worker threads). -/// A status in `omit` is an expected answer: its failure makes no network record (a tile past a -/// map's edge). +/// A status in `omit` is an expected answer: its failure isn't recorded, only noted once per +/// server (a tile past a map's edge). pub(crate) fn fetch_bytes(url: String, done: Done, omit: &'static [u16]) { let f = fetcher().lock().unwrap_or_else(|e| e.into_inner()).clone(); match f { @@ -1491,7 +1494,7 @@ mod native { /// The answer to `url`; a refusal keeps the server's answer only when `detail` asks for it, /// as tiles a server doesn't have are routine and only their status is read. A status in - /// `omit` is an expected answer: its failure makes no network record. + /// `omit` is an expected answer: its failure isn't recorded, only noted once per server. fn fetch_answer(agent: &ureq::Agent, url: &str, detail: bool, omit: &'static [u16]) -> Result, Failure> { use std::io::Read; // Local file failures are not network traffic. diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index b070db732..518adc30c 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -547,6 +547,7 @@ pub struct UiState { pub right: Option, /// The right panes open as tabs, in the order they were opened (#303). pub right_open: Vec, + /// The Diagnostics pane's filter (it also picks what **Copy diagnostics** copies). pub diagnostic_filter: rustgis_engine::data::diagnostics::Filter, diagnostic_snapshot: Option<(u64, Vec)>, /// The Web Data service the Web Data pane shows (a `ui.dialog` name, see @@ -1777,7 +1778,7 @@ impl RustGisApp { // UI-only failures can return before an engine command emits a notice. let text = rustgis_engine::data::diagnostics::redact(e); if !self.session.notices_since(notice_mark).iter().any(|notice| notice.error && notice.text == text) { - self.session.notify(text, true); + self.session.notify_redacted(text, true); } } // Commands can replace the project, switch maps or change the renderer while the pane diff --git a/docs/privacy.md b/docs/privacy.md index 5b89a3ce5..e7864e519 100644 --- a/docs/privacy.md +++ b/docs/privacy.md @@ -7,7 +7,8 @@ send searches, coordinates, data or assistant context to those services, as desc ## Local troubleshooting history **Diagnostics** keeps a bounded, in-memory history of messages and failed tile, service and -data requests for the current session, even while its pane is closed. It is not telemetry: +data requests for the current session, even while its pane is closed (a map tile the server +doesn't have is routine: noted once per server, not listed). It is not telemetry: nothing is uploaded automatically, written to preferences, or saved in a project. **Copy diagnostics** explicitly copies the filtered entries; **Clear** removes the history. diff --git a/docs/user-guide/index.md b/docs/user-guide/index.md index cf66f1be4..cbc99d59a 100644 --- a/docs/user-guide/index.md +++ b/docs/user-guide/index.md @@ -127,10 +127,18 @@ details** for failed requests: their address, method, observed HTTP status and d available. Native connection failures and timeouts are identified when the transport knows them; an opaque browser failure does not claim a status or a specific DNS, CORS, or connection cause. -Use **All**, **Errors**, **Warnings**, **Info**, or **Network failures** to filter the history. -**Copy diagnostics** copies the filtered entries and their technical details, including folded -details. **Clear** clears all entries, not just the selected filter. Closing or reopening the -pane does not clear it. +Use **All**, **Errors**, **Warnings**, **Info**, or **Network failures** to filter the history +(**Network failures** also lists the notes below about omitted absences). **Copy diagnostics** +copies the filtered entries and their technical details, including folded details, as plain +text: oldest entry first, with UTC Unix-millisecond timestamps, where the pane shows the newest +first and a UTC clock time. **Clear** clears all entries, not just the selected filter. Closing +or reopening the pane does not clear it. + +A map tile a server doesn't have (HTTP 404, or 410 where it is gone) is routine, as is a Zarr +store's absent chunk, so each isn't listed. One **Network** note per server says that such +absences are omitted, naming the first address. Past 64 servers one more note says that further +servers aren't listed. A note that has left the 500 entries, or a **Clear**, lets its server be +noted again. The last 500 entries are kept in memory for this session, including across project changes; older entries are discarded. Each message is limited to 2 KiB and technical details to 8 KiB. From 09dcad9d860da0bb5fb1ad8ce85554ac659e7a4d Mon Sep 17 00:00:00 2001 From: Daniel Hofbauer Date: Mon, 2 Nov 2026 18:02:41 +0100 Subject: [PATCH 04/13] refactor(diagnostics): a Redacted type for the bound-only path; test 410 tiles History::push_redacted and Session::notify_redacted take a Redacted, which only redact can make, so the compiler checks that the text is clean rather than a doc comment. A test pins that a gone (410) map tile is noted, not recorded, and a server error still is. redact's doc says what a field is mid-line. --- crates/data/src/diagnostics.rs | 44 +++++++++++++++++++++++++--------- crates/engine/src/lib.rs | 19 +++++++-------- crates/tiles/src/cache.rs | 26 +++++++++++++++++++- crates/ui-egui/src/lib.rs | 4 ++-- 4 files changed, 69 insertions(+), 24 deletions(-) diff --git a/crates/data/src/diagnostics.rs b/crates/data/src/diagnostics.rs index 5eb2ac83f..b3df81968 100644 --- a/crates/data/src/diagnostics.rs +++ b/crates/data/src/diagnostics.rs @@ -207,17 +207,17 @@ impl History { }); } - /// [`History::push`] for text already through [`redact`] (a notice the session redacted for - /// its status bar): it is only bounded, so a notice isn't redacted again for each copy. Text - /// that might hold a credential goes to `push`. - pub fn push_redacted(&self, severity: Severity, source: &str, message: &str, details: &str) { + /// [`History::push`] for a message already through [`redact`] (a notice the session redacted + /// for its status bar): it is only bounded, so a notice isn't redacted again for each copy. + /// Only a [`Redacted`], which `redact` alone makes, gets this path. + pub fn push_redacted(&self, severity: Severity, source: &str, message: &Redacted) { self.retain(Record { id: 0, timestamp_ms: timestamp_ms(), severity, - source: truncated(source, MAX_SOURCE_BYTES), - message: truncated(message, MAX_MESSAGE_BYTES), - details: truncated(details, MAX_DETAILS_BYTES), + source: bounded(source, MAX_SOURCE_BYTES), + message: truncated(message.as_str(), MAX_MESSAGE_BYTES), + details: String::new(), network: None, }); } @@ -421,6 +421,25 @@ fn path_secrets_redacted(address: &str) -> String { out } +/// Text that has been through [`redact`]. Only [`Redacted::new`] makes one, so a path that takes +/// a `Redacted` (the history's bound-only [`History::push_redacted`]) can't be handed raw text. +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct Redacted(String); + +impl Redacted { + pub fn new(text: &str) -> Self { + Self(redact(text)) + } + + pub fn as_str(&self) -> &str { + &self.0 + } + + pub fn into_string(self) -> String { + self.0 + } +} + /// Redacts, then bounds: a record's copy of a message or of details. fn bounded(text: &str, limit: usize) -> String { let mut clean = redact(text); @@ -605,7 +624,9 @@ fn base64_shaped(word: &str) -> bool { /// retained because its identifying key lay beyond a clipped prefix. URL queries/fragments /// are all omitted, including unknown/encoded signing schemes. JSON/XML bodies are not logged. /// A credential field discards the remaining text: quoted, multiline and malformed values need -/// not be parsed correctly to remain private. Diagnostics intentionally prefer less context. +/// not be parsed correctly to remain private. Diagnostics intentionally prefer less context. What +/// counts as a field is narrower in the middle of a line: `name: several words` there reads as +/// prose and is kept (see `docs/privacy.md`). pub fn redact(text: &str) -> String { if text.len() > MAX_INPUT_BYTES { return "[oversized diagnostic omitted]".to_string(); @@ -1219,12 +1240,13 @@ mod tests { fn text_already_redacted_is_only_bounded() { let history = History::default(); let long = "word ".repeat(1000); - history.push_redacted(Severity::Info, "Application", &long, &long); - history.push(Severity::Info, "Application", &long, &long); + history.push_redacted(Severity::Info, "Application", &Redacted::new(&long)); + history.push(Severity::Info, "Application", &long, ""); let records = history.records(); assert_eq!(records[0].message, records[1].message); - assert_eq!(records[0].details, records[1].details); assert!(records[0].message.len() <= MAX_MESSAGE_BYTES && records[0].message.ends_with('…')); + // What the type holds has been through `redact`, whatever went in. + assert_eq!(Redacted::new("Authorization: Bearer hidden").as_str(), "[REDACTED]"); } #[test] diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index a53e03579..ae965fff5 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -1092,8 +1092,8 @@ impl Session { } // A refused command is said once, without secrets, as the front ends show it. if let Err(error) = &res { - let text = rustgis_data::diagnostics::redact(error); - if !self.notices_since(notice_mark).iter().any(|notice| notice.error && notice.text == text) { + let text = rustgis_data::diagnostics::Redacted::new(error); + if !self.notices_since(notice_mark).iter().any(|notice| notice.error && notice.text == text.as_str()) { self.notify_redacted(text, true); } } @@ -1502,20 +1502,19 @@ impl Session { // Redacted for the status bar and the history alike. `redact` bounds the notice to 8 KiB // (an ellipsis marks the cut) and omits one over 16 KiB whole, so a clipped credential // can't survive; the history's copy is bounded further, to a record's 2 KiB. - let text = rustgis_data::diagnostics::redact(&text); - self.notice_redacted(text, severity); + self.notice_redacted(rustgis_data::diagnostics::Redacted::new(&text), severity); } - /// [`Session::notify`] for text that is already through [`rustgis_data::diagnostics::redact`] - /// (a failure the caller redacted to compare it with the notices): not redacted again. - pub fn notify_redacted(&mut self, text: String, error: bool) { + /// [`Session::notify`] for text already redacted (a failure the caller redacted to compare it + /// with the notices): the type says so, so it isn't redacted again. + pub fn notify_redacted(&mut self, text: rustgis_data::diagnostics::Redacted, error: bool) { use rustgis_data::diagnostics::Severity; self.notice_redacted(text, if error { Severity::Error } else { Severity::Info }); } - fn notice_redacted(&mut self, text: String, severity: rustgis_data::diagnostics::Severity) { - self.diagnostics.push_redacted(severity, self.diagnostic_source.unwrap_or("Application"), &text, ""); - self.notices.push(Notice { text, error: severity == rustgis_data::diagnostics::Severity::Error }); + fn notice_redacted(&mut self, text: rustgis_data::diagnostics::Redacted, severity: rustgis_data::diagnostics::Severity) { + self.diagnostics.push_redacted(severity, self.diagnostic_source.unwrap_or("Application"), &text); + self.notices.push(Notice { text: text.into_string(), error: severity == rustgis_data::diagnostics::Severity::Error }); self.notices_given += 1; if self.notices.len() > 200 { self.notices.remove(0); diff --git a/crates/tiles/src/cache.rs b/crates/tiles/src/cache.rs index f82434103..c60152c47 100644 --- a/crates/tiles/src/cache.rs +++ b/crates/tiles/src/cache.rs @@ -190,9 +190,12 @@ pub fn get(url: &str, want: Want) -> State { /// a tile (a single-image layer, a layout's picture, a style's sprite sheet) goes through /// [`get`], so its 404 is recorded. pub fn get_tile(url: &str, want: Want) -> State { - request(url, want, &[404, 410]) + request(url, want, TILE_OMIT) } +/// The statuses a map tile's request omits ([`get_tile`]). +const TILE_OMIT: &[u16] = &[404, 410]; + /// [`get`] with the statuses that are expected answers for this fetch. fn request(url: &str, want: Want, omit: &'static [u16]) -> State { // A map tile of a layer in another system: its service's picture as it comes, or made from @@ -1872,6 +1875,27 @@ mod tests { assert!(network_records(&history, &url).is_empty()); } + /// A map tile a server no longer has (410) is routine, as a missing one (404) is: noted once + /// for the server, not recorded as a failure. A server error is still recorded. + #[cfg(not(target_arch = "wasm32"))] + #[test] + fn network_diagnostics_skip_gone_map_tiles_and_keep_server_errors() { + let history = rustgis_data::diagnostics::History::default(); + let _scope = history.enter(); + let answer = |status: &str| format!("HTTP/1.1 {status}\r\nContent-Length: 0\r\nConnection: close\r\n\r\n"); + let base = serve(vec![answer("410 Gone"), answer("500 Error")]); + let gone = format!("{base}/tiles/0/0/0.png"); + assert_eq!(native::get(&native::agent(), &gone, TILE_OMIT).unwrap_err(), "http status: 410"); + let notes = network_records(&history, &gone); + assert_eq!(notes.len(), 1); + assert_eq!(notes[0].severity, rustgis_data::diagnostics::Severity::Warning, "a note, not a failure"); + let failed = format!("{base}/tiles/1/1/1.png"); + assert_eq!(native::get(&native::agent(), &failed, TILE_OMIT).unwrap_err(), "http status: 500"); + let records = network_records(&history, &failed); + assert_eq!(records.len(), 1); + assert_eq!(records[0].severity, rustgis_data::diagnostics::Severity::Error); + } + /// A credential header in a refusal is taken out by itself: the headers after it (which /// say who refused) are kept. #[cfg(not(target_arch = "wasm32"))] diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index 518adc30c..d1b54886f 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -1776,8 +1776,8 @@ impl RustGisApp { if let Err(e) = &r { log::info!("{id}: {e}"); // UI-only failures can return before an engine command emits a notice. - let text = rustgis_engine::data::diagnostics::redact(e); - if !self.session.notices_since(notice_mark).iter().any(|notice| notice.error && notice.text == text) { + let text = rustgis_engine::data::diagnostics::Redacted::new(e); + if !self.session.notices_since(notice_mark).iter().any(|notice| notice.error && notice.text == text.as_str()) { self.session.notify_redacted(text, true); } } From bcf1a5ec57618e8a54f96b6cb9274412a0c1715f Mon Sep 17 00:00:00 2001 From: Daniel Hofbauer Date: Mon, 2 Nov 2026 18:52:15 +0100 Subject: [PATCH 05/13] style: format the inline request bodies; correct a comment Long string literals kept rustfmt from indenting the browser fetch_bytes body and one native error; they are split now. An oversized absence address counts with the unparseable ones, as the comment now says. --- apps/rustgis-web/src/web.rs | 262 +++++++++++++++++---------------- crates/data/src/diagnostics.rs | 2 +- crates/tiles/src/cache.rs | 8 +- 3 files changed, 138 insertions(+), 134 deletions(-) diff --git a/apps/rustgis-web/src/web.rs b/apps/rustgis-web/src/web.rs index 55c593358..dd3d53069 100644 --- a/apps/rustgis-web/src/web.rs +++ b/apps/rustgis-web/src/web.rs @@ -579,144 +579,146 @@ async fn fetch_bytes(url: &str, capture: Capture, omit: &'static [u16]) -> Resul let mut diagnostics = RequestDiagnostics::with_capture(url, "GET", capture); diagnostics.omit_statuses(omit); let answer = async { - let err = |e: wasm_bindgen::JsValue| format!("{e:?}"); - let window = web_sys::window().ok_or("no window")?; - use rustgis_ui_egui::rustgis_engine::render::rustgis_tiles::cache; - // A data download carries its caller's budget (at most MAX_DOWNLOAD); other fetches use 64 MB. - let (url, limit) = cache::split_download_limit(url); - let max = limit.map(|n| n as u64).unwrap_or(64 << 20); - let (url, tail) = cache::split_tail(url); - let (url, range) = cache::split_range(url); - // An `s3://` address becomes its web address, and a request to an S3 connection's endpoint - // is signed (the bucket's CORS rules must allow the Authorization and x-amz-* headers). - let prepared = rustgis_ui_egui::rustgis_engine::render::rustgis_tiles::s3::prepare(url)?; - let url = prepared.url.as_str(); - // An ArcGIS sign-in's token, for the portal and the servers it covers, renewed first when it - // is about to expire. It goes as the request's `token` parameter, not a header: ArcGIS's - // query endpoints answer the header's CORS preflight with an error. - if !prepared.signed { - crate::arcgis::renew_if_due(url).await; - } - let arcgis = if prepared.signed { None } else { rustgis_ui_egui::rustgis_engine::render::rustgis_tiles::arcgis_auth::with_token(url) }; - // Only the request carries it: messages name the address without it. - let request = arcgis.as_deref().unwrap_or(url); - let asked = match (range, tail) { - (Some((a, b)), _) => Some(format!("bytes={a}-{b}")), - (None, Some(n)) => Some(format!("bytes=-{n}")), - (None, None) => None, - }; - let init = web_sys::RequestInit::new(); - // A tile or document that stalls is given up (see `STALL_MS`). - let abort = if limit.is_none() { web_sys::AbortController::new().ok() } else { None }; - if let Some(a) = &abort { - init.set_signal(Some(&a.signal())); - } - let stalled = || format!("nothing came in {} seconds", STALL_MS / 1000); - if asked.is_some() || !prepared.headers.is_empty() { - let headers = web_sys::Headers::new().map_err(err)?; - if let Some(r) = &asked { - headers.set("Range", r).map_err(err)?; + let err = |e: wasm_bindgen::JsValue| format!("{e:?}"); + let window = web_sys::window().ok_or("no window")?; + use rustgis_ui_egui::rustgis_engine::render::rustgis_tiles::cache; + // A data download carries its caller's budget (at most MAX_DOWNLOAD); other fetches use 64 MB. + let (url, limit) = cache::split_download_limit(url); + let max = limit.map(|n| n as u64).unwrap_or(64 << 20); + let (url, tail) = cache::split_tail(url); + let (url, range) = cache::split_range(url); + // An `s3://` address becomes its web address, and a request to an S3 connection's endpoint + // is signed (the bucket's CORS rules must allow the Authorization and x-amz-* headers). + let prepared = rustgis_ui_egui::rustgis_engine::render::rustgis_tiles::s3::prepare(url)?; + let url = prepared.url.as_str(); + // An ArcGIS sign-in's token, for the portal and the servers it covers, renewed first when it + // is about to expire. It goes as the request's `token` parameter, not a header: ArcGIS's + // query endpoints answer the header's CORS preflight with an error. + if !prepared.signed { + crate::arcgis::renew_if_due(url).await; } - for (k, v) in &prepared.headers { - headers.set(k, v).map_err(err)?; + let arcgis = + if prepared.signed { None } else { rustgis_ui_egui::rustgis_engine::render::rustgis_tiles::arcgis_auth::with_token(url) }; + // Only the request carries it: messages name the address without it. + let request = arcgis.as_deref().unwrap_or(url); + let asked = match (range, tail) { + (Some((a, b)), _) => Some(format!("bytes={a}-{b}")), + (None, Some(n)) => Some(format!("bytes=-{n}")), + (None, None) => None, + }; + let init = web_sys::RequestInit::new(); + // A tile or document that stalls is given up (see `STALL_MS`). + let abort = if limit.is_none() { web_sys::AbortController::new().ok() } else { None }; + if let Some(a) = &abort { + init.set_signal(Some(&a.signal())); } - init.set_headers(&headers); - } - // A signed or authorized request goes only where its credentials are for. - if prepared.signed || arcgis.is_some() { - init.set_redirect(web_sys::RequestRedirect::Error); - } - // The token's address goes nowhere else: no referrer from it, no cookies with it. - if arcgis.is_some() { - init.set_referrer_policy(web_sys::ReferrerPolicy::NoReferrer); - init.set_credentials(web_sys::RequestCredentials::Omit); - } - let promise = window.fetch_with_str_and_init(request, &init); - // A refused fetch is an opaque TypeError ("Failed to fetch") by design: the browser doesn't - // say why, so the likely reasons are named, with the browser's own message but not its stack. - let resp: web_sys::Response = unless_stalled(promise, abort.as_ref()) - .await - .ok_or_else(|| { - diagnostics.timed_out(); - stalled() - })? - .map_err(|e| { - let why = e.dyn_ref::().map(|e| String::from(e.message())).unwrap_or_else(|| err(e)); - format!( - "the browser couldn't reach it (offline, blocked by a browser extension or the network, or the server doesn't accept requests from web pages): {why}" - ) - })? - .dyn_into() - .map_err(err)?; - diagnostics.answered(resp.status()); - if !resp.ok() { - return Err(format!("HTTP {}", resp.status())); - } - // The last bytes asked for and the whole file answered: fine when it is no longer. - let whole_tail = tail.filter(|_| resp.status() != 206); - let max = match whole_tail { - Some(n) => n.saturating_add(1).min(max), - None => max, - }; - // A range asked for and the whole file answered (200): fine for a range from the start that - // holds the whole file (a small archive), checked once it's read; else no ranges. - let whole = range.is_some() && resp.status() != 206; - if range.is_some_and(|(a, _)| whole && a > 0) { - return Err(format!("{url}: {}", cache::NO_RANGES)); - } - // Fonts are at most about 10 MB and plugins 64 MB; refuse anything larger than allowed, by - // header when there is one and by the downloaded size otherwise. - if let Ok(Some(len)) = resp.headers().get("content-length").map(|v| v.and_then(|s| s.parse::().ok())) - && len > max - { - // The last bytes asked for and a longer file answered: no ranges, so it is downloaded whole. - if whole_tail.is_some() { - return Err(format!("{url}: {}", cache::NO_RANGES)); + let stalled = || format!("nothing came in {} seconds", STALL_MS / 1000); + if asked.is_some() || !prepared.headers.is_empty() { + let headers = web_sys::Headers::new().map_err(err)?; + if let Some(r) = &asked { + headers.set("Range", r).map_err(err)?; + } + for (k, v) in &prepared.headers { + headers.set(k, v).map_err(err)?; + } + init.set_headers(&headers); } - return Err(format!("{len} bytes is too large")); - } - // Read a chunk at a time, so a body larger than allowed (whatever its header said) is cut - // off as soon as it is, never held whole. - // Sized by the header when there is one (checked above), so the chunks don't reallocate. - let len = resp.headers().get("content-length").ok().flatten().and_then(|s| s.parse::().ok()).unwrap_or(0); - let mut bytes: Vec = Vec::with_capacity(usize::try_from(len.min(max)).unwrap_or(0)); - if let Some(body) = resp.body() { - let reader: web_sys::ReadableStreamDefaultReader = - body.get_reader().dyn_into().map_err(|_| "the response has no readable body".to_string())?; - loop { - // Headers came and then the body stalled: given up as a fetch that doesn't answer. - // (Aborting the fetch has cancelled its body too.) - let Some(chunk) = unless_stalled(reader.read(), abort.as_ref()).await else { + // A signed or authorized request goes only where its credentials are for. + if prepared.signed || arcgis.is_some() { + init.set_redirect(web_sys::RequestRedirect::Error); + } + // The token's address goes nowhere else: no referrer from it, no cookies with it. + if arcgis.is_some() { + init.set_referrer_policy(web_sys::ReferrerPolicy::NoReferrer); + init.set_credentials(web_sys::RequestCredentials::Omit); + } + let promise = window.fetch_with_str_and_init(request, &init); + // A refused fetch is an opaque TypeError ("Failed to fetch") by design: the browser doesn't + // say why, so the likely reasons are named, with the browser's own message but not its stack. + let resp: web_sys::Response = unless_stalled(promise, abort.as_ref()) + .await + .ok_or_else(|| { diagnostics.timed_out(); - return Err(stalled()); - }; - let chunk = chunk.map_err(err)?; - if js_sys::Reflect::get(&chunk, &"done".into()).ok().and_then(|d| d.as_bool()).unwrap_or(true) { - break; + stalled() + })? + .map_err(|e| { + let why = e.dyn_ref::().map(|e| String::from(e.message())).unwrap_or_else(|| err(e)); + format!( + "the browser couldn't reach it (offline, blocked by a browser extension or the network, \ + or the server doesn't accept requests from web pages): {why}" + ) + })? + .dyn_into() + .map_err(err)?; + diagnostics.answered(resp.status()); + if !resp.ok() { + return Err(format!("HTTP {}", resp.status())); + } + // The last bytes asked for and the whole file answered: fine when it is no longer. + let whole_tail = tail.filter(|_| resp.status() != 206); + let max = match whole_tail { + Some(n) => n.saturating_add(1).min(max), + None => max, + }; + // A range asked for and the whole file answered (200): fine for a range from the start that + // holds the whole file (a small archive), checked once it's read; else no ranges. + let whole = range.is_some() && resp.status() != 206; + if range.is_some_and(|(a, _)| whole && a > 0) { + return Err(format!("{url}: {}", cache::NO_RANGES)); + } + // Fonts are at most about 10 MB and plugins 64 MB; refuse anything larger than allowed, by + // header when there is one and by the downloaded size otherwise. + if let Ok(Some(len)) = resp.headers().get("content-length").map(|v| v.and_then(|s| s.parse::().ok())) + && len > max + { + // The last bytes asked for and a longer file answered: no ranges, so it is downloaded whole. + if whole_tail.is_some() { + return Err(format!("{url}: {}", cache::NO_RANGES)); } - let value = js_sys::Uint8Array::new(&js_sys::Reflect::get(&chunk, &"value".into()).map_err(err)?); - let n = value.length() as usize; - if (bytes.len() + n) as u64 > max { - let _ = reader.cancel(); - if whole_tail.is_some() { - return Err(format!("{url}: {}", cache::NO_RANGES)); + return Err(format!("{len} bytes is too large")); + } + // Read a chunk at a time, so a body larger than allowed (whatever its header said) is cut + // off as soon as it is, never held whole. + // Sized by the header when there is one (checked above), so the chunks don't reallocate. + let len = resp.headers().get("content-length").ok().flatten().and_then(|s| s.parse::().ok()).unwrap_or(0); + let mut bytes: Vec = Vec::with_capacity(usize::try_from(len.min(max)).unwrap_or(0)); + if let Some(body) = resp.body() { + let reader: web_sys::ReadableStreamDefaultReader = + body.get_reader().dyn_into().map_err(|_| "the response has no readable body".to_string())?; + loop { + // Headers came and then the body stalled: given up as a fetch that doesn't answer. + // (Aborting the fetch has cancelled its body too.) + let Some(chunk) = unless_stalled(reader.read(), abort.as_ref()).await else { + diagnostics.timed_out(); + return Err(stalled()); + }; + let chunk = chunk.map_err(err)?; + if js_sys::Reflect::get(&chunk, &"done".into()).ok().and_then(|d| d.as_bool()).unwrap_or(true) { + break; } - return Err(format!("larger than {max} bytes")); + let value = js_sys::Uint8Array::new(&js_sys::Reflect::get(&chunk, &"value".into()).map_err(err)?); + let n = value.length() as usize; + if (bytes.len() + n) as u64 > max { + let _ = reader.cancel(); + if whole_tail.is_some() { + return Err(format!("{url}: {}", cache::NO_RANGES)); + } + return Err(format!("larger than {max} bytes")); + } + let at = bytes.len(); + bytes.resize(at + n, 0); + value.copy_to(bytes.get_mut(at..).unwrap_or_default()); } - let at = bytes.len(); - bytes.resize(at + n, 0); - value.copy_to(bytes.get_mut(at..).unwrap_or_default()); } - } - if let Some((a, b)) = range.filter(|_| whole) - && bytes.len() as u64 > b - a + 1 - { - return Err(format!("{url}: {}", cache::NO_RANGES)); - } - if whole_tail.is_some_and(|n| bytes.len() as u64 > n) { - return Err(format!("{url}: {}", cache::NO_RANGES)); - } - Ok::<_, String>(bytes) + if let Some((a, b)) = range.filter(|_| whole) + && bytes.len() as u64 > b - a + 1 + { + return Err(format!("{url}: {}", cache::NO_RANGES)); + } + if whole_tail.is_some_and(|n| bytes.len() as u64 > n) { + return Err(format!("{url}: {}", cache::NO_RANGES)); + } + Ok::<_, String>(bytes) } .await; diagnostics.record(answer.map_err(|e| Failure::new(url, e))).map_err(|failure| failure.message) diff --git a/crates/data/src/diagnostics.rs b/crates/data/src/diagnostics.rs index b3df81968..7a27f385e 100644 --- a/crates/data/src/diagnostics.rs +++ b/crates/data/src/diagnostics.rs @@ -285,7 +285,7 @@ impl Capture { // The server is the address's origin (scheme, host and port): two tile sources on one // host but different ports are two sources. An address without a web origin (`s3://`) // is its scheme and host, so two buckets are two sources. An oversized address isn't - // parsed: it is one source of its own. + // parsed: it counts with the addresses that don't parse, as one source. let server = (url.len() <= MAX_INPUT_BYTES) .then(|| url::Url::parse(url).ok()) .flatten() diff --git a/crates/tiles/src/cache.rs b/crates/tiles/src/cache.rs index c60152c47..9ec376b8f 100644 --- a/crates/tiles/src/cache.rs +++ b/crates/tiles/src/cache.rs @@ -1603,9 +1603,11 @@ mod native { // A signed request goes only where its credentials are for. if prepared.signed { return Err(format!( - "{url}: the S3 endpoint answered with a redirect (HTTP {}), which a signed request doesn't follow: check the connection's region and endpoint", - resp.status().as_u16() - ).into()); + "{url}: the S3 endpoint answered with a redirect (HTTP {}), which a signed request doesn't follow: \ + check the connection's region and endpoint", + resp.status().as_u16() + ) + .into()); } let location = resp.headers().get("location").and_then(|v| v.to_str().ok()).ok_or("a redirect with no Location")?; let next = crate::template::join(&at, location); From 2b85cbce9b29ae8004c8ec0e138f2cd9cad02d8c Mon Sep 17 00:00:00 2001 From: Daniel Hofbauer Date: Mon, 2 Nov 2026 20:17:37 +0100 Subject: [PATCH 06/13] test(engine): keep the check that a saved project holds no diagnostics --- crates/engine/src/tests.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 64e8b0f0f..7c35a5403 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -11496,7 +11496,7 @@ fn notices_since_survives_the_cap() { } #[test] -fn diagnostics_capture_notices_without_an_open_pane_and_survive_a_new_project() { +fn diagnostics_capture_notices_without_an_open_pane_or_project_persistence() { use rustgis_data::diagnostics::{Filter, MAX_DETAILS_BYTES, MAX_MESSAGE_BYTES, Severity}; let mut s = Session::new(Project::default()); let mark = s.notice_mark(); @@ -11511,6 +11511,9 @@ fn diagnostics_capture_notices_without_an_open_pane_and_survive_a_new_project() assert_eq!(records.iter().map(|r| r.severity).collect::>(), [Severity::Info, Severity::Warning, Severity::Error]); assert!(records.iter().all(|r| r.source == "Application" && r.timestamp_ms > 0)); assert!(!s.diagnostics.report(Filter::All).contains("never-retain-this")); + // A project's saved text holds none of the history (the docs promise it). + let saved = serde_json::to_string(&s.project).unwrap(); + assert!(!saved.contains("diagnostics-session-only")); s.notify("界".repeat(3000), false); // The notice keeps its words to redaction's own bound (8 KiB, an ellipsis at the cut), not // the record's 2 KiB; only the history's record copy takes the message bound. From f4cd8699bc51d46d3ccb9a696132d95491e4dda6 Mon Sep 17 00:00:00 2001 From: Daniel Hofbauer Date: Mon, 2 Nov 2026 20:17:37 +0100 Subject: [PATCH 07/13] fix(diagnostics): an oversized address keeps its own server's absence note The origin is parsed from the address's first 2 KiB, where its scheme and host are, so the size bound stays and two oversized servers aren't one. --- crates/data/src/diagnostics.rs | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/crates/data/src/diagnostics.rs b/crates/data/src/diagnostics.rs index 7a27f385e..2931c3ce3 100644 --- a/crates/data/src/diagnostics.rs +++ b/crates/data/src/diagnostics.rs @@ -284,11 +284,13 @@ impl Capture { let Some(state) = self.0.upgrade() else { return }; // The server is the address's origin (scheme, host and port): two tile sources on one // host but different ports are two sources. An address without a web origin (`s3://`) - // is its scheme and host, so two buckets are two sources. An oversized address isn't - // parsed: it counts with the addresses that don't parse, as one source. - let server = (url.len() <= MAX_INPUT_BYTES) - .then(|| url::Url::parse(url).ok()) - .flatten() + // is its scheme and host, so two buckets are two sources. They come first, so only the + // address's first part is parsed: an oversized address keeps its own server. + let mut head = url.len().min(MAX_URL_BYTES); + while !url.is_char_boundary(head) { + head -= 1; + } + let server = url::Url::parse(url.get(..head).unwrap_or("")) .map(|u| { let origin = u.origin(); if origin.is_tuple() { origin.ascii_serialization() } else { format!("{}://{}", u.scheme(), u.host_str().unwrap_or("")) } @@ -1256,12 +1258,14 @@ mod tests { capture.note_absence("HEAD", "https://tiles.example.org/0/0/0.png", 404); capture.note_absence("BREW", "https://other.example.org/0/0/0.png", 404); let long = format!("https://big.example.org/{}", "a".repeat(MAX_INPUT_BYTES)); + let other_long = format!("https://huge.example.org/{}", "a".repeat(MAX_INPUT_BYTES)); capture.note_absence("GET", &long, 404); capture.note_absence("GET", &long, 404); + capture.note_absence("GET", &other_long, 404); let records = history.records(); let method = |i: usize| records[i].network.as_ref().map(|n| n.method.as_str()); assert_eq!((method(0), method(1)), (Some("HEAD"), Some("OTHER"))); - assert_eq!(records.len(), 3, "an oversized address is one source, said once"); + assert_eq!(records.len(), 4, "each oversized address's server is said once"); assert_eq!(records[2].network.as_ref().map(|n| n.url.as_str()), Some("[oversized address omitted]")); } From 15475590859e806e9a52b0149ea355bfe2bccbd9 Mon Sep 17 00:00:00 2001 From: Daniel Hofbauer Date: Mon, 2 Nov 2026 22:40:27 +0100 Subject: [PATCH 08/13] docs: a short word after Basic is kept; the pane shows a UTC date and time --- docs/privacy.md | 2 ++ docs/user-guide/index.md | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/docs/privacy.md b/docs/privacy.md index e7864e519..e401ba778 100644 --- a/docs/privacy.md +++ b/docs/privacy.md @@ -17,6 +17,8 @@ strings (including signed URLs), fragments, credential fields, and structured pa omitted conservatively. A credential field in the middle of a line is removed when its value is one word or quoted (`… password: hunter2`), but a value of several words reads as prose and is kept (`… password: correct horse battery` keeps all three), so check such text before sharing. +After `Basic` at the start of a line, a short word (under 8 characters, with no digit or +padding) is kept as a word, so a very short Basic credential such as `YTpi` is not removed. Request and response bodies are not captured by network diagnostics. Background requests carry their originating session's weak capture token; failures are not broadcast to other embedded or Python sessions. Layer names, file paths and URL paths can diff --git a/docs/user-guide/index.md b/docs/user-guide/index.md index cbc99d59a..07f04777a 100644 --- a/docs/user-guide/index.md +++ b/docs/user-guide/index.md @@ -131,7 +131,7 @@ Use **All**, **Errors**, **Warnings**, **Info**, or **Network failures** to filt (**Network failures** also lists the notes below about omitted absences). **Copy diagnostics** copies the filtered entries and their technical details, including folded details, as plain text: oldest entry first, with UTC Unix-millisecond timestamps, where the pane shows the newest -first and a UTC clock time. **Clear** clears all entries, not just the selected filter. Closing +first and a UTC date and time. **Clear** clears all entries, not just the selected filter. Closing or reopening the pane does not clear it. A map tile a server doesn't have (HTTP 404, or 410 where it is gone) is routine, as is a Zarr From b152d4586dbac5f4a80c268c63453dc4927c0b85 Mon Sep 17 00:00:00 2001 From: Daniel Hofbauer Date: Mon, 2 Nov 2026 22:41:51 +0100 Subject: [PATCH 09/13] docs: state the Basic rule as the code applies it --- docs/privacy.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/docs/privacy.md b/docs/privacy.md index e401ba778..e96ac80ef 100644 --- a/docs/privacy.md +++ b/docs/privacy.md @@ -17,8 +17,9 @@ strings (including signed URLs), fragments, credential fields, and structured pa omitted conservatively. A credential field in the middle of a line is removed when its value is one word or quoted (`… password: hunter2`), but a value of several words reads as prose and is kept (`… password: correct horse battery` keeps all three), so check such text before sharing. -After `Basic` at the start of a line, a short word (under 8 characters, with no digit or -padding) is kept as a word, so a very short Basic credential such as `YTpi` is not removed. +After `Basic` (without a colon) at the start of a line, the next word is removed only when it +looks like base64: it has a digit or punctuation, has 16 characters or more, or has 8 or more +with a capital after the first letter. So a very short Basic credential such as `YTpi` is kept. Request and response bodies are not captured by network diagnostics. Background requests carry their originating session's weak capture token; failures are not broadcast to other embedded or Python sessions. Layer names, file paths and URL paths can From cbfd5c2bb97154b87436f4a8a6e09aa76610d1dc Mon Sep 17 00:00:00 2001 From: Daniel Hofbauer Date: Mon, 2 Nov 2026 22:43:35 +0100 Subject: [PATCH 10/13] docs: the Basic rule removes the rest of the text --- docs/privacy.md | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/docs/privacy.md b/docs/privacy.md index e96ac80ef..4e92e2769 100644 --- a/docs/privacy.md +++ b/docs/privacy.md @@ -17,9 +17,10 @@ strings (including signed URLs), fragments, credential fields, and structured pa omitted conservatively. A credential field in the middle of a line is removed when its value is one word or quoted (`… password: hunter2`), but a value of several words reads as prose and is kept (`… password: correct horse battery` keeps all three), so check such text before sharing. -After `Basic` (without a colon) at the start of a line, the next word is removed only when it -looks like base64: it has a digit or punctuation, has 16 characters or more, or has 8 or more -with a capital after the first letter. So a very short Basic credential such as `YTpi` is kept. +After `Basic` (without a colon) at the start of a line, the rest of the text is removed only +when the next word looks like base64: it has a digit or punctuation, has 16 characters or more, +or has 8 or more with a capital after the first letter. So a very short Basic credential such as +`YTpi` is kept. Request and response bodies are not captured by network diagnostics. Background requests carry their originating session's weak capture token; failures are not broadcast to other embedded or Python sessions. Layer names, file paths and URL paths can From 47e863d3f065494cea4d235ac1a4b07b59d622ac Mon Sep 17 00:00:00 2001 From: Daniel Hofbauer Date: Mon, 2 Nov 2026 23:20:37 +0100 Subject: [PATCH 11/13] fix(diagnostics): sentence punctuation after a redacted address stays outside it Removing user information re-parses the address; a trailing , . ; : or ) joined the host (https://example.org,/). It is set aside before parsing and put back after. The label doc comments name the copied report only. --- crates/data/src/diagnostics.rs | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/crates/data/src/diagnostics.rs b/crates/data/src/diagnostics.rs index 2931c3ce3..f75553520 100644 --- a/crates/data/src/diagnostics.rs +++ b/crates/data/src/diagnostics.rs @@ -27,7 +27,7 @@ pub enum Severity { } impl Severity { - /// The word the pane and the report show. + /// The word the copied report shows (the pane shows its own, translated). pub fn label(self) -> &'static str { match self { Self::Error => "Error", @@ -69,7 +69,7 @@ pub enum NetworkKind { } impl NetworkKind { - /// The words the pane (in English) and the report show. + /// The words the copied report shows (the pane shows its own, translated). pub fn label(self) -> &'static str { match self { Self::Http => "HTTP failure", @@ -714,16 +714,24 @@ pub fn redact(text: &str) -> String { out.push_str(&path_secrets_redacted(address)); } Ok(mut url) if url.has_host() => { + // Sentence punctuation after the address (`…@example.org, retrying`) + // isn't the address's: parsed with it, it would join the host (or be an + // empty port, dropped). It is set aside and put back after; with a query, + // the note below puts it back instead. + let mut kept = None; + if let Some(c) = address.chars().last().filter(|c| matches!(c, ':' | ',' | ';' | ')' | '.')) + && !address.contains(['?', '#']) + && let Ok(trimmed) = url::Url::parse(address.get(..address.len() - c.len_utf8()).unwrap_or("")) + && trimmed.has_host() + { + (url, kept) = (trimmed, Some(c)); + } let _ = url.set_username(""); let _ = url.set_password(None); url.set_query(None); url.set_fragment(None); out.push_str(&path_secrets_redacted(url.as_str())); - // An empty port's colon ends the address as a sentence's does, and - // parsing drops it; with a query, the note below puts it back. - if address.ends_with(':') && !url.as_str().ends_with(':') && !address.contains(['?', '#']) { - out.push(':'); - } + out.extend(kept); } _ => out.push_str("[address omitted]"), } @@ -1208,6 +1216,8 @@ mod tests { #[test] fn removing_user_information_keeps_the_sentence_punctuation() { assert_eq!(redact("Could not reach https://alice:hidden@example.org: timeout"), "Could not reach https://example.org/: timeout"); + assert_eq!(redact("Could not reach https://alice:hidden@example.org, retrying"), "Could not reach https://example.org/, retrying"); + assert_eq!(redact("Could not reach https://alice:hidden@example.org. Retrying"), "Could not reach https://example.org/. Retrying"); assert_eq!( redact("Could not reach https://alice:hidden@example.org/x. Retrying"), "Could not reach https://example.org/x. Retrying" From 85c7adafcea6e0fc255e261b96ebd0a6d69e580b Mon Sep 17 00:00:00 2001 From: Daniel Hofbauer Date: Mon, 2 Nov 2026 23:20:37 +0100 Subject: [PATCH 12/13] docs: the Basic rule ignores a sentence's closing marks --- docs/privacy.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/privacy.md b/docs/privacy.md index 4e92e2769..85826fb6d 100644 --- a/docs/privacy.md +++ b/docs/privacy.md @@ -18,9 +18,9 @@ omitted conservatively. A credential field in the middle of a line is removed wh one word or quoted (`… password: hunter2`), but a value of several words reads as prose and is kept (`… password: correct horse battery` keeps all three), so check such text before sharing. After `Basic` (without a colon) at the start of a line, the rest of the text is removed only -when the next word looks like base64: it has a digit or punctuation, has 16 characters or more, -or has 8 or more with a capital after the first letter. So a very short Basic credential such as -`YTpi` is kept. +when the next word looks like base64: it has a digit or punctuation (other than the +sentence-ending marks `.,!?;:)`), has 16 characters or more, or has 8 or more with a capital +after the first letter. So a very short Basic credential such as `YTpi` is kept. Request and response bodies are not captured by network diagnostics. Background requests carry their originating session's weak capture token; failures are not broadcast to other embedded or Python sessions. Layer names, file paths and URL paths can From 9de89f75f78fa18f072bf1e796a4b0fe89034509 Mon Sep 17 00:00:00 2001 From: Daniel Hofbauer Date: Mon, 2 Nov 2026 23:22:17 +0100 Subject: [PATCH 13/13] docs: only a closing mark at the word's end doesn't count --- docs/privacy.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/privacy.md b/docs/privacy.md index 85826fb6d..cfb8d419f 100644 --- a/docs/privacy.md +++ b/docs/privacy.md @@ -18,8 +18,8 @@ omitted conservatively. A credential field in the middle of a line is removed wh one word or quoted (`… password: hunter2`), but a value of several words reads as prose and is kept (`… password: correct horse battery` keeps all three), so check such text before sharing. After `Basic` (without a colon) at the start of a line, the rest of the text is removed only -when the next word looks like base64: it has a digit or punctuation (other than the -sentence-ending marks `.,!?;:)`), has 16 characters or more, or has 8 or more with a capital +when the next word looks like base64: it has a digit or punctuation (a sentence-ending mark +`.,!?;:)` at its end doesn't count), has 16 characters or more, or has 8 or more with a capital after the first letter. So a very short Basic credential such as `YTpi` is kept. Request and response bodies are not captured by network diagnostics. Background requests carry their originating session's weak capture token; failures are not