From fd3664cf606ce3b84c3a8617897d390ba995c561 Mon Sep 17 00:00:00 2001 From: Dominik Strebel Date: Sat, 24 Oct 2026 21:00:56 +0200 Subject: [PATCH 1/6] fix(map): QGIS TRUE/FALSE and exponents convert to RustGIS SQL, switched-off label rules aren't noted, unread source options and out-of-range graduated values are --- crates/engine/src/cmd/qgis.rs | 9 ++++++- crates/map/src/qgis/expr.rs | 12 ++++++--- crates/map/src/qgis/labels.rs | 7 +++--- crates/map/src/qgis/renderer.rs | 3 +++ crates/map/src/qgis/source.rs | 44 ++++++++++++++++++++++++--------- crates/map/src/qgis/tests.rs | 13 ++++++++-- 6 files changed, 68 insertions(+), 20 deletions(-) diff --git a/crates/engine/src/cmd/qgis.rs b/crates/engine/src/cmd/qgis.rs index dd6bcbbe3..418948e68 100644 --- a/crates/engine/src/cmd/qgis.rs +++ b/crates/engine/src/cmd/qgis.rs @@ -137,7 +137,14 @@ fn add_layer(s: &mut Session, layer: &qgis::ProjectLayer, has_dir: bool, report: } } match &layer.source { - Source::File { path, layer: table, .. } => { + Source::File { path, layer: table, ignored, .. } => { + if !ignored.is_empty() { + // `layerid=2`: RustGIS opens a layer by name, so a file's first layer comes in. + report.approximated( + name, + format!("its source options {} (not read: a file with several layers opens its first)", ignored.join(", ")), + ); + } let mut q = json!({"path": path, "name": name}); if let Some(t) = table { q["layer"] = json!(t); diff --git a/crates/map/src/qgis/expr.rs b/crates/map/src/qgis/expr.rs index ba7a524b6..9255a631e 100644 --- a/crates/map/src/qgis/expr.rs +++ b/crates/map/src/qgis/expr.rs @@ -123,8 +123,9 @@ fn tokenize(s: &str) -> Option> { } } let n: String = chars.get(start..i).unwrap_or_default().iter().collect(); - n.parse::().ok()?; - out.push(Token::Word(n)); + let v = n.parse::().ok()?; + // An exponent is written out (0.001 for 1e-3): RustGIS SQL reads plain decimals. + out.push(Token::Word(if n.contains(['e', 'E']) && v.is_finite() { format!("{v}") } else { n })); } c if c.is_alphabetic() || c == '_' => { let start = i; @@ -135,6 +136,8 @@ fn tokenize(s: &str) -> Option> { // A word followed by a bracket is a function call. let next = chars.get(i..).unwrap_or_default().iter().find(|c| !c.is_whitespace()); match keyword(&w) { + // TRUE and FALSE are values, bare as RustGIS SQL has them, not fields. + _ if w.eq_ignore_ascii_case("true") || w.eq_ignore_ascii_case("false") => out.push(Token::Word(w.to_lowercase())), Some(k) => out.push(Token::Word(k.into())), None if next == Some(&'(') => return None, None => out.push(Token::Field(w)), @@ -180,9 +183,12 @@ mod tests { assert_eq!(to_sql(r#""Symbol" = 'Ortsteil_20-49'"#).as_deref(), Some(r#""Symbol" = 'Ortsteil_20-49'"#)); assert_eq!( to_sql(r#"pop >= 1e3 AND ("name" ILIKE 'z%' OR kind IN ('a', 'b''s')) AND note IS NOT NULL"#).as_deref(), - Some(r#""pop" >= 1e3 AND ( "name" LIKE 'z%' OR "kind" IN ( 'a' , 'b''s' ) ) AND "note" IS NOT NULL"#) + Some(r#""pop" >= 1000 AND ( "name" LIKE 'z%' OR "kind" IN ( 'a' , 'b''s' ) ) AND "note" IS NOT NULL"#) ); assert_eq!(to_sql("a <> -2.5").as_deref(), Some(r#""a" <> -2.5"#)); + // Exponents written out, and TRUE and FALSE as values, as RustGIS SQL reads them. + assert_eq!(to_sql("d < 2.5E-4 OR d > -1e+2").as_deref(), Some(r#""d" < 0.00025 OR "d" > -100"#)); + assert_eq!(to_sql("flag = TRUE AND gone = false").as_deref(), Some(r#""flag" = true AND "gone" = false"#)); for refused in ["upper(\"name\") = 'X'", "\"a\" + 1 > 2", "\"a\" || 'x' = 'y'", "@map_scale < 5000", "'unclosed", "", "a ! b"] { assert_eq!(to_sql(refused), None, "{refused}"); } diff --git a/crates/map/src/qgis/labels.rs b/crates/map/src/qgis/labels.rs index 459bd92a2..9aac6d612 100644 --- a/crates/map/src/qgis/labels.rs +++ b/crates/map/src/qgis/labels.rs @@ -24,6 +24,10 @@ pub(super) fn read(l: Node, layer: &str, report: &mut Report) -> Option Option> = BTreeMap::new(); for (i, (.., c, label)) in ranges.iter().enumerate() { diff --git a/crates/map/src/qgis/source.rs b/crates/map/src/qgis/source.rs index f7649c6b8..f0361ad75 100644 --- a/crates/map/src/qgis/source.rs +++ b/crates/map/src/qgis/source.rs @@ -5,8 +5,9 @@ #[derive(Clone, Debug, PartialEq)] pub enum Source { /// A file (`ogr`, `gdal`, `spatialite`): its path, the layer or table in it, and the filter - /// QGIS's subset string sets (as QGIS has it). - File { path: String, layer: Option, subset: Option }, + /// QGIS's subset string sets (as QGIS has it), and the source's options RustGIS doesn't read + /// (`layerid=2`, `geometrytype=Point`), for the import to note. + File { path: String, layer: Option, subset: Option, ignored: Vec }, /// A delimited text file (`delimitedtext`): its path, and the fields its points (or WKT /// geometry) are in. Csv { path: String, x: Option, y: Option, wkt: Option, crs: Option }, @@ -29,7 +30,7 @@ pub fn read(provider: &str, datasource: &str, dir: &str) -> Source { "spatialite" => { let kv = conninfo(datasource); match (kv.iter().find(|(k, _)| k == "dbname"), table_of(datasource)) { - (Some((_, db)), Some(t)) => Source::File { path: absolute(db, dir), layer: Some(t), subset: None }, + (Some((_, db)), Some(t)) => Source::File { path: absolute(db, dir), layer: Some(t), subset: None, ignored: Vec::new() }, _ => Source::Other { provider: provider.into() }, } } @@ -77,20 +78,27 @@ fn file(datasource: &str, dir: &str) -> Source { if let Some(rest) = datasource.strip_prefix("GPKG:") { // GPKG:path:table, the path itself possibly with a drive letter's colon. if let Some((path, table)) = rest.rsplit_once(':') { - return Source::File { path: absolute(path, dir), layer: Some(table.to_string()), subset: None }; + return Source::File { path: absolute(path, dir), layer: Some(table.to_string()), subset: None, ignored: Vec::new() }; } } let mut parts = datasource.split('|'); let path = parts.next().unwrap_or_default().trim(); - let (mut layer, mut subset) = (None, None); - for p in parts { + let (mut layer, mut subset, mut ignored) = (None, None, Vec::new()); + for p in parts.take(32) { if let Some(v) = p.strip_prefix("layername=") { layer = Some(v.to_string()); } else if let Some(v) = p.strip_prefix("subset=") { subset = Some(v.to_string()).filter(|s| !s.trim().is_empty()); + } else if !p.trim().is_empty() && p.trim() != "layerid=0" { + // (layerid=0 is the first layer, which is what opens.) + ignored.push(p.trim().chars().take(100).collect()); } } - Source::File { path: absolute(path, dir), layer, subset } + // A layer named as well as numbered: the name is what's read. + if layer.is_some() { + ignored.retain(|o: &String| !o.starts_with("layerid=")); + } + Source::File { path: absolute(path, dir), layer, subset, ignored } } /// A `delimitedtext` source: `file:///path.csv?type=csv&xField=…&yField=…&crs=…`. @@ -197,14 +205,28 @@ mod tests { Source::File { path: "/data/project/roads.gpkg".into(), layer: Some("roads".into()), - subset: Some("\"type\" = 'motorway'".into()) + subset: Some("\"type\" = 'motorway'".into()), + ignored: Vec::new() } ); - assert_eq!(read("ogr", "/abs/rivers.shp", dir), Source::File { path: "/abs/rivers.shp".into(), layer: None, subset: None }); - assert_eq!(read("gdal", "C:/maps/dem.tif", dir), Source::File { path: "C:/maps/dem.tif".into(), layer: None, subset: None }); + assert_eq!( + read("ogr", "/abs/rivers.shp", dir), + Source::File { path: "/abs/rivers.shp".into(), layer: None, subset: None, ignored: Vec::new() } + ); + assert_eq!( + read("gdal", "C:/maps/dem.tif", dir), + Source::File { path: "C:/maps/dem.tif".into(), layer: None, subset: None, ignored: Vec::new() } + ); + // Options RustGIS doesn't read are kept for a note; the first layer's number isn't one. + let Source::File { ignored, .. } = read("ogr", "./multi.gpkg|layerid=2|geometrytype=Point", dir) else { panic!() }; + assert_eq!(ignored, ["layerid=2", "geometrytype=Point"]); + let Source::File { ignored, .. } = read("ogr", "./one.gpkg|layerid=0", dir) else { panic!() }; + assert!(ignored.is_empty()); + let Source::File { ignored, .. } = read("ogr", "./multi.gpkg|layername=roads|layerid=3", dir) else { panic!() }; + assert!(ignored.is_empty(), "the name is read: {ignored:?}"); assert_eq!( read("gdal", "GPKG:./m.gpkg:relief", dir), - Source::File { path: "/data/project/m.gpkg".into(), layer: Some("relief".into()), subset: None } + Source::File { path: "/data/project/m.gpkg".into(), layer: Some("relief".into()), subset: None, ignored: Vec::new() } ); assert_eq!( read("delimitedtext", "file:///data/pts%20a.csv?type=csv&xField=lon&yField=lat&crs=EPSG:4326", dir), diff --git a/crates/map/src/qgis/tests.rs b/crates/map/src/qgis/tests.rs index 8b3e6ee4a..7368078f6 100644 --- a/crates/map/src/qgis/tests.rs +++ b/crates/map/src/qgis/tests.rs @@ -77,6 +77,7 @@ const PROJECT: &str = r#" + @@ -117,7 +118,12 @@ fn a_project_reads_into_layers_in_tree_order_with_their_styles() { let land = &p.layers[2]; assert_eq!( land.source, - Source::File { path: "/maps/data/base.gpkg".into(), layer: Some("land".into()), subset: Some("\"kind\" <> 'sea'".into()) } + Source::File { + path: "/maps/data/base.gpkg".into(), + layer: Some("land".into()), + subset: Some("\"kind\" <> 'sea'".into()), + ignored: Vec::new() + } ); assert_eq!(land.definition_query.as_deref(), Some("\"kind\" <> 'sea'")); assert_eq!(land.crs.as_deref(), Some("EPSG:2056")); @@ -141,7 +147,7 @@ fn a_project_reads_into_layers_in_tree_order_with_their_styles() { // Roads: categories with stacked strokes in levels, a hidden one and the rest. let roads = &p.layers[1]; - assert_eq!(roads.source, Source::File { path: "/abs/roads.shp".into(), layer: None, subset: None }); + assert_eq!(roads.source, Source::File { path: "/abs/roads.shp".into(), layer: None, subset: None, ignored: Vec::new() }); let s = &roads.style; assert_eq!((s.stroke_levels, s.line_strokes.as_deref()), (Some(true), Some(&[][..]))); let Some(Renderer::Unique { field, classes, class_strokes, class_labels, other, .. }) = &s.renderer else { panic!() }; @@ -179,11 +185,14 @@ fn a_project_reads_into_layers_in_tree_order_with_their_styles() { let said = |layer: &str, kind: NoteKind, part: &str| report.notes.iter().any(|n| n.layer == layer && n.kind == kind && n.text.contains(part)); assert!(said("Land", NoteKind::Approximated, "line pattern")); + assert!(said("Land", NoteKind::Approximated, "above the last"), "values outside the ranges are noted"); assert!(said("Land", NoteKind::Approximated, "Liberation Sans Narrow")); assert!(said("Roads", NoteKind::Approximated, "dashed")); assert!(said("Roads", NoteKind::Skipped, "markers and hash marks")); assert!(said("Roads", NoteKind::Skipped, "length(")); assert!(said("Towns", NoteKind::Approximated, "font markers")); + // A switched-off label rule is never drawn, so its filter isn't noted. + assert!(!report.notes.iter().any(|n| n.text.contains("Switched off")), "{:?}", report.notes); } #[test] From 12935707cb490773c444589657d6eeea93892875 Mon Sep 17 00:00:00 2001 From: Dominik Strebel Date: Sat, 24 Oct 2026 21:01:10 +0200 Subject: [PATCH 2/6] fix(render): a font face whose OS/2 table can't be read is kept, at regular weight and upright --- crates/render/src/families.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/crates/render/src/families.rs b/crates/render/src/families.rs index 91e9b9d4b..5ac266512 100644 --- a/crates/render/src/families.rs +++ b/crates/render/src/families.rs @@ -309,7 +309,8 @@ fn face_info(r: &mut (impl Read + Seek), start: u32) -> Option { let style = read_name(17).or_else(|| read_name(2)).unwrap_or_default().to_lowercase(); let (weight, selection) = match table(b"OS/2") { Some((at, len)) => { - let os2 = read_at(r, at, len.min(64))?; + // An unreadable `OS/2` (cut short, past the end) keeps the face, at the defaults. + let os2 = read_at(r, at, len.min(64)).unwrap_or_default(); (u16_at(&os2, 4).unwrap_or(400), u16_at(&os2, 62).unwrap_or(0)) } None => (400, 0), From 9a254af629229742b842c561b4cf506fafcfbbeb Mon Sep 17 00:00:00 2001 From: Dominik Strebel Date: Sat, 24 Oct 2026 21:50:32 +0200 Subject: [PATCH 3/6] feat(map): a QGIS label's font family is the label class's font, drawn in it where installed --- crates/map/src/qgis/labels.rs | 7 +++---- crates/map/src/qgis/tests.rs | 4 +++- docs/user-guide/workspace.md | 6 ++++-- 3 files changed, 10 insertions(+), 7 deletions(-) diff --git a/crates/map/src/qgis/labels.rs b/crates/map/src/qgis/labels.rs index 9aac6d612..353458ba6 100644 --- a/crates/map/src/qgis/labels.rs +++ b/crates/map/src/qgis/labels.rs @@ -184,9 +184,8 @@ fn class( || weight >= 63.0 || text.attribute("fontBold") == Some("1"); let italic = style.contains("italic") || style.contains("oblique") || text.attribute("fontItalic") == Some("1"); - if let Some(family) = text.attribute("fontFamily").filter(|f| !f.is_empty()) { - report.approximated(layer, format!("labels in {family}, drawn in RustGIS's label font")); - } + // QGIS's font family: drawn in it where it is installed, else in RustGIS's label font. + let font = text.attribute("fontFamily").and_then(crate::symbology::font_name); if num(text, "fontLetterSpacing").is_some_and(|s| s != 0.0) { report.approximated(layer, "label letter spacing, left out"); } @@ -220,7 +219,7 @@ fn class( farthest: None, placement, italic, - font: None, + font, }) } diff --git a/crates/map/src/qgis/tests.rs b/crates/map/src/qgis/tests.rs index 7368078f6..ed2abd146 100644 --- a/crates/map/src/qgis/tests.rs +++ b/crates/map/src/qgis/tests.rs @@ -144,6 +144,7 @@ fn a_project_reads_into_layers_in_tree_order_with_their_styles() { assert!(labels.enabled && labels.classes.is_empty()); assert_eq!((d.field.as_str(), d.size, d.bold, d.italic, d.halo), ("name", 12.0, true, true, true)); assert_eq!((d.color, d.placement), (Rgba::rgb(20, 40, 60), LabelPlacement::AboveRight)); + assert_eq!(d.font.as_deref(), Some("Liberation Sans Narrow"), "QGIS's font family"); // Roads: categories with stacked strokes in levels, a hidden one and the rest. let roads = &p.layers[1]; @@ -186,7 +187,8 @@ fn a_project_reads_into_layers_in_tree_order_with_their_styles() { |layer: &str, kind: NoteKind, part: &str| report.notes.iter().any(|n| n.layer == layer && n.kind == kind && n.text.contains(part)); assert!(said("Land", NoteKind::Approximated, "line pattern")); assert!(said("Land", NoteKind::Approximated, "above the last"), "values outside the ranges are noted"); - assert!(said("Land", NoteKind::Approximated, "Liberation Sans Narrow")); + // A QGIS font family is the label class's font, drawn in it where it's installed. + assert!(!said("Land", NoteKind::Approximated, "Liberation Sans Narrow"), "no longer approximated"); assert!(said("Roads", NoteKind::Approximated, "dashed")); assert!(said("Roads", NoteKind::Skipped, "markers and hash marks")); assert!(said("Roads", NoteKind::Skipped, "length(")); diff --git a/docs/user-guide/workspace.md b/docs/user-guide/workspace.md index 2815c0d1e..aa4dfe82e 100644 --- a/docs/user-guide/workspace.md +++ b/docs/user-guide/workspace.md @@ -42,14 +42,16 @@ Each layer keeps its look: reference scale. - **Labels**: simple and rule-based, each rule a label class whose SQL clause is its filter and its parents' (an else rule's: none of its siblings'), with the text's field, size, colour, - bold and italic, its buffer as a halo, its placement (Parallel and Curved along lines, + bold and italic, its font family (drawn in it where it is installed, else in RustGIS's label + font), its buffer as a halo, its placement (Parallel and Curved along lines, around and over points) and its scale range. QGIS expressions carry over where they are SQL: fields compared with values, `LIKE`, `IN`, `IS NULL`, joined by `AND`, `OR` and `NOT`. What RustGIS doesn't draw as QGIS does is drawn as near as it can, or left out, and said: the import's notes list, layer by layer, what was approximated (pattern fills drawn as a tint of their colour, font and SVG markers as circles, -dashed lines solid, fonts other than RustGIS's label font) and what was skipped (marker lines, +dashed lines solid, values outside a graduated renderer's ranges in its first or last class, +file options such as `layerid` that RustGIS doesn't read) and what was skipped (marker lines, data-defined settings, labels from expressions, raster colouring, PostGIS, WFS and other sources RustGIS doesn't read). A database password in a project never reaches the notes. From cda567d2d5c965fc90bcf9a0467b5fb37706208c Mon Sep 17 00:00:00 2001 From: Dominik Strebel Date: Sat, 24 Oct 2026 22:03:37 +0200 Subject: [PATCH 4/6] fix(map): an else label rule leaves switched-off siblings out, as QGIS does --- crates/map/src/qgis/labels.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/crates/map/src/qgis/labels.rs b/crates/map/src/qgis/labels.rs index 353458ba6..f0c61dcf9 100644 --- a/crates/map/src/qgis/labels.rs +++ b/crates/map/src/qgis/labels.rs @@ -109,9 +109,10 @@ fn flatten<'a, 'i>( return; } let rules: Vec = node.children().filter(|c| c.has_tag_name("rule")).collect(); - // The filters an else rule here is the rest of. + // The filters an else rule here is the rest of: its switched-on siblings', as QGIS has it. let siblings: Vec = rules .iter() + .filter(|r| r.attribute("active") != Some("0")) .filter_map(|r| r.attribute("filter").map(str::trim)) .filter(|f| !f.is_empty() && !is_else(f)) .map(str::to_string) From 781f46839a768261a7e6e13a8186aea764c437bf Mon Sep 17 00:00:00 2001 From: Dominik Strebel Date: Sat, 24 Oct 2026 22:29:43 +0200 Subject: [PATCH 5/6] fix(map): QGIS source options are read past the noted ones, secret-like option values stay out of notes, the first-layer note only without a layer name --- crates/engine/src/cmd/qgis.rs | 9 ++++----- crates/map/src/qgis/renderer.rs | 3 ++- crates/map/src/qgis/source.rs | 31 ++++++++++++++++++++++++++++--- 3 files changed, 34 insertions(+), 9 deletions(-) diff --git a/crates/engine/src/cmd/qgis.rs b/crates/engine/src/cmd/qgis.rs index 418948e68..207906ee2 100644 --- a/crates/engine/src/cmd/qgis.rs +++ b/crates/engine/src/cmd/qgis.rs @@ -139,11 +139,10 @@ fn add_layer(s: &mut Session, layer: &qgis::ProjectLayer, has_dir: bool, report: match &layer.source { Source::File { path, layer: table, ignored, .. } => { if !ignored.is_empty() { - // `layerid=2`: RustGIS opens a layer by name, so a file's first layer comes in. - report.approximated( - name, - format!("its source options {} (not read: a file with several layers opens its first)", ignored.join(", ")), - ); + // `layerid=2`: RustGIS opens a layer by name, so without one a file's first layer + // comes in; with one, the options are only not read. + let why = if table.is_some() { "not read" } else { "not read: a file with several layers opens its first" }; + report.approximated(name, format!("its source options {} ({why})", ignored.join(", "))); } let mut q = json!({"path": path, "name": name}); if let Some(t) = table { diff --git a/crates/map/src/qgis/renderer.rs b/crates/map/src/qgis/renderer.rs index 3e89f0338..90f2d5511 100644 --- a/crates/map/src/qgis/renderer.rs +++ b/crates/map/src/qgis/renderer.rs @@ -175,7 +175,8 @@ fn graduated(r: Node, layer: &str, style: &mut Style, report: &mut Report, sym: } let min = ranges.first().map_or(0.0, |r| r.0); // QGIS draws nothing for a value outside every range; a RustGIS graduated renderer gives it the - // first or last class. + // first or last class. The import doesn't read the data, so it can't tell whether any value + // falls outside: the note is always given, on purpose. report.approximated(layer, "values below the first range or above the last, drawn in the first or last class (QGIS leaves them out)"); let mut class_labels = BTreeMap::new(); let mut class_strokes: BTreeMap> = BTreeMap::new(); diff --git a/crates/map/src/qgis/source.rs b/crates/map/src/qgis/source.rs index f0361ad75..4b85e9f93 100644 --- a/crates/map/src/qgis/source.rs +++ b/crates/map/src/qgis/source.rs @@ -84,14 +84,15 @@ fn file(datasource: &str, dir: &str) -> Source { let mut parts = datasource.split('|'); let path = parts.next().unwrap_or_default().trim(); let (mut layer, mut subset, mut ignored) = (None, None, Vec::new()); - for p in parts.take(32) { + // Every part is read for the layer and the filter; at most 32 others are kept for the note. + for p in parts { if let Some(v) = p.strip_prefix("layername=") { layer = Some(v.to_string()); } else if let Some(v) = p.strip_prefix("subset=") { subset = Some(v.to_string()).filter(|s| !s.trim().is_empty()); - } else if !p.trim().is_empty() && p.trim() != "layerid=0" { + } else if !p.trim().is_empty() && p.trim() != "layerid=0" && ignored.len() < 32 { // (layerid=0 is the first layer, which is what opens.) - ignored.push(p.trim().chars().take(100).collect()); + ignored.push(noted_option(p.trim())); } } // A layer named as well as numbered: the name is what's read. @@ -101,6 +102,21 @@ fn file(datasource: &str, dir: &str) -> Source { Source::File { path: absolute(path, dir), layer, subset, ignored } } +/// A source option as the import notes it: its name only when it may carry a secret (a key, a +/// token, a password, a credential), else its text, cut to 100 characters. +fn noted_option(option: &str) -> String { + match option.split_once('=') { + Some((key, _)) + if ["credential", "secret", "password", "passwd", "pwd", "token", "key", "auth"] + .iter() + .any(|w| key.to_ascii_lowercase().contains(w)) => + { + format!("{}=…", key.chars().take(100).collect::()) + } + _ => option.chars().take(100).collect(), + } +} + /// A `delimitedtext` source: `file:///path.csv?type=csv&xField=…&yField=…&crs=…`. fn csv(datasource: &str, dir: &str) -> Source { let (path, q) = datasource.split_once('?').unwrap_or((datasource, "")); @@ -224,6 +240,15 @@ mod tests { assert!(ignored.is_empty()); let Source::File { ignored, .. } = read("ogr", "./multi.gpkg|layername=roads|layerid=3", dir) else { panic!() }; assert!(ignored.is_empty(), "the name is read: {ignored:?}"); + // An option that may carry a secret is noted by its name only. + let Source::File { ignored, .. } = read("ogr", "/vsis3/b/x.gpkg|credential:AWS_SECRET_ACCESS_KEY=hunter2|token=abc", dir) else { + panic!() + }; + assert_eq!(ignored, ["credential:AWS_SECRET_ACCESS_KEY=…", "token=…"]); + // The layer and the filter are read past the options kept for the note. + let many = format!("./m.gpkg|{}|layername=late|subset=a = 1", vec!["x=1"; 40].join("|")); + let Source::File { layer, subset, ignored, .. } = read("ogr", &many, dir) else { panic!() }; + assert_eq!((layer.as_deref(), subset.as_deref(), ignored.len()), (Some("late"), Some("a = 1"), 32)); assert_eq!( read("gdal", "GPKG:./m.gpkg:relief", dir), Source::File { path: "/data/project/m.gpkg".into(), layer: Some("relief".into()), subset: None, ignored: Vec::new() } From 0947ae21d895b13526df0c5cb7b80310a26596b6 Mon Sep 17 00:00:00 2001 From: Dominik Strebel Date: Sat, 24 Oct 2026 22:58:48 +0200 Subject: [PATCH 6/6] fix(map): a QGIS SpatiaLite layer keeps its sql filter, unread options show values only when harmless, a multi-layer file's note says every layer comes in --- crates/engine/src/cmd/qgis.rs | 6 ++--- crates/map/src/qgis/source.rs | 50 +++++++++++++++++++++++++++-------- 2 files changed, 42 insertions(+), 14 deletions(-) diff --git a/crates/engine/src/cmd/qgis.rs b/crates/engine/src/cmd/qgis.rs index 207906ee2..dee40c793 100644 --- a/crates/engine/src/cmd/qgis.rs +++ b/crates/engine/src/cmd/qgis.rs @@ -139,9 +139,9 @@ fn add_layer(s: &mut Session, layer: &qgis::ProjectLayer, has_dir: bool, report: match &layer.source { Source::File { path, layer: table, ignored, .. } => { if !ignored.is_empty() { - // `layerid=2`: RustGIS opens a layer by name, so without one a file's first layer - // comes in; with one, the options are only not read. - let why = if table.is_some() { "not read" } else { "not read: a file with several layers opens its first" }; + // `layerid=2`: RustGIS opens a layer by name, so without one every layer of a file + // that has several comes in; with one, the options are only not read. + let why = if table.is_some() { "not read" } else { "not read: a file with several layers adds them all" }; report.approximated(name, format!("its source options {} ({why})", ignored.join(", "))); } let mut q = json!({"path": path, "name": name}); diff --git a/crates/map/src/qgis/source.rs b/crates/map/src/qgis/source.rs index 4b85e9f93..d255fa59c 100644 --- a/crates/map/src/qgis/source.rs +++ b/crates/map/src/qgis/source.rs @@ -28,9 +28,21 @@ pub fn read(provider: &str, datasource: &str, dir: &str) -> Source { match provider { "ogr" | "gdal" => file(datasource, dir), "spatialite" => { - let kv = conninfo(datasource); - match (kv.iter().find(|(k, _)| k == "dbname"), table_of(datasource)) { - (Some((_, db)), Some(t)) => Source::File { path: absolute(db, dir), layer: Some(t), subset: None, ignored: Vec::new() }, + // `dbname='…' key='id' srid=4326 type=Point table="t" (geom) sql=…`: the filter, if + // any, runs to the end. + let (head, sql) = datasource.rsplit_once(" sql=").unwrap_or((datasource, "")); + let subset = Some(sql.trim().to_string()).filter(|q| !q.is_empty()); + let kv = conninfo(head); + // What QGIS always writes and RustGIS reads or needn't. + const READ_OR_HARMLESS: [&str; 7] = ["dbname", "table", "key", "srid", "type", "estimatedmetadata", "checkPrimaryKeyUnicity"]; + let ignored = kv + .iter() + .filter(|(k, _)| !READ_OR_HARMLESS.iter().any(|r| r.eq_ignore_ascii_case(k))) + .take(32) + .map(|(k, v)| noted_option(&format!("{k}={v}"))) + .collect(); + match (kv.iter().find(|(k, _)| k == "dbname"), table_of(head)) { + (Some((_, db)), Some(t)) => Source::File { path: absolute(db, dir), layer: Some(t), subset, ignored }, _ => Source::Other { provider: provider.into() }, } } @@ -102,16 +114,14 @@ fn file(datasource: &str, dir: &str) -> Source { Source::File { path: absolute(path, dir), layer, subset, ignored } } -/// A source option as the import notes it: its name only when it may carry a secret (a key, a -/// token, a password, a credential), else its text, cut to 100 characters. +/// A source option as the import notes it: with its value only for the options known to carry +/// nothing private (`layerid=2`, `geometrytype=Point`), else by its name alone, since a value can +/// be a key, a token or a password under any name. fn noted_option(option: &str) -> String { + const SHOWN: [&str; 3] = ["layerid", "geometrytype", "uniquegeometrytype"]; match option.split_once('=') { - Some((key, _)) - if ["credential", "secret", "password", "passwd", "pwd", "token", "key", "auth"] - .iter() - .any(|w| key.to_ascii_lowercase().contains(w)) => - { - format!("{}=…", key.chars().take(100).collect::()) + Some((key, _)) if !SHOWN.iter().any(|k| k.eq_ignore_ascii_case(key.trim())) => { + format!("{}=…", key.trim().chars().take(100).collect::()) } _ => option.chars().take(100).collect(), } @@ -245,6 +255,24 @@ mod tests { panic!() }; assert_eq!(ignored, ["credential:AWS_SECRET_ACCESS_KEY=…", "token=…"]); + let Source::File { ignored, .. } = read("ogr", "./x.gpkg|header=Bearer abc|layerid=4", dir) else { panic!() }; + assert_eq!(ignored, ["header=…", "layerid=4"], "values only for options known to be harmless"); + // SpatiaLite: the table, its filter, and the options that matter noted. + let Source::File { path, layer, subset, ignored } = + read("spatialite", r#"dbname='./db.sqlite' key='id' srid=4326 type=Point table="towns" (geom) sql="pop" > 1000"#, dir) + else { + panic!() + }; + assert_eq!( + (path.as_str(), layer.as_deref(), subset.as_deref()), + ("/data/project/db.sqlite", Some("towns"), Some(r#""pop" > 1000"#)) + ); + assert!(ignored.is_empty(), "{ignored:?}"); + let Source::File { subset, ignored, .. } = read("spatialite", r#"dbname='/a.sqlite' password='s3cret' table="t" (geom) sql="#, dir) + else { + panic!() + }; + assert_eq!((subset, ignored), (None, vec!["password=…".to_string()])); // The layer and the filter are read past the options kept for the note. let many = format!("./m.gpkg|{}|layername=late|subset=a = 1", vec!["x=1"; 40].join("|")); let Source::File { layer, subset, ignored, .. } = read("ogr", &many, dir) else { panic!() };