From a5036bf73c15604fb9881f13c27596810eb8e8e2 Mon Sep 17 00:00:00 2001 From: Maciej Myszkiewicz Date: Sun, 11 Oct 2026 22:25:22 +0200 Subject: [PATCH 1/2] fix: refuse a base container left concrete beside its own children MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `abstract` defaults to false in XTCE, so a base SequenceContainer that nobody marked — the ordinary CCSDS primary-header shape — is emitted as a concrete container. Its generated `matches` is `true` for any buffer long enough to hold it, and the module-level `decode` counts how many containers match. So every packet matched the base and its own child, and `decode` answered `Ambiguous` for all of them: a generated decoder that rejects every packet, from a definition that generated without a word. Refused at generation time now, naming the container and saying both ways out: error: cannot compile in FlightPacket: this container has no restriction criteria, so it matches every packet long enough to hold it; beside the other containers generated here that makes every packet ambiguous. Mark it abstract="true" if other containers inherit from it, or give it criteria that select it Only when there is more than one container: a lone concrete container with no criteria is the whole definition and decodes fine. Found by a review sweep and reproduced with the bundled definition, by removing one attribute from it. 43 tests, fmt and clippy clean. --- README.md | 1 + crates/xtce-flight/src/layout.rs | 19 ++++++++++++ crates/xtce-flight/tests/layout.rs | 48 ++++++++++++++++++++++++++++++ 3 files changed, 68 insertions(+) diff --git a/README.md b/README.md index a5a0779..92b8ff1 100644 --- a/README.md +++ b/README.md @@ -116,6 +116,7 @@ than one that stops, because the gap only shows up in flight. | Integers, 1–64 bits, all three signed codings | Yes | | IEEE-754 binary16, binary32, binary64 | Yes | | MIL-STD-1750A | Refused — reading it is many-to-one, so there is no inverse to write | +| A base container left concrete beside its own children | Refused by name — `abstract` defaults to false in XTCE, and a container with no restriction criteria matches every packet long enough to hold it, so the generated `decode` would answer `Ambiguous` for all of them | | Booleans, enumerations (as generated Rust enums) | Yes | | Fixed-size text: whole-buffer, terminated, length-prefixed | Yes | | Fixed-size binary | Yes | diff --git a/crates/xtce-flight/src/layout.rs b/crates/xtce-flight/src/layout.rs index 8350318..06deefe 100644 --- a/crates/xtce-flight/src/layout.rs +++ b/crates/xtce-flight/src/layout.rs @@ -309,6 +309,25 @@ pub fn build(plan: &Plan) -> Result { }); } + // A container with no restriction criteria matches every packet long enough to hold it — + // its generated `matches` is `true` — so a decoder holding it beside any other container + // answers `Ambiguous` for every packet it is ever given. XTCE's `abstract` defaults to + // false, which makes an unmarked base container, the ordinary CCSDS primary-header shape, + // exactly that. Refused here by name rather than generated into a decoder that rejects + // every packet once it is on the spacecraft. + if containers.len() > 1 + && let Some(unconditional) = containers.iter().find(|held| held.constants.is_empty()) + { + return Err(FlightError::Unsupported { + element: "SequenceContainer".to_owned(), + container: unconditional.xtce_name.clone(), + reason: "this container has no restriction criteria, so it matches every packet \ + long enough to hold it; beside the other containers generated here that \ + makes every packet ambiguous. Mark it abstract=\"true\" if other \ + containers inherit from it, or give it criteria that select it", + }); + } + Ok(Layout { containers, enums: builder.enums, diff --git a/crates/xtce-flight/tests/layout.rs b/crates/xtce-flight/tests/layout.rs index 9c83b9b..01b8f4e 100644 --- a/crates/xtce-flight/tests/layout.rs +++ b/crates/xtce-flight/tests/layout.rs @@ -234,6 +234,54 @@ fn a_data_dependent_width_is_refused() { ); } +/// A base left concrete matches everything, and `abstract` defaults to false. +/// +/// The generated `matches` for a container with no criteria is `true`, so a decoder holding it +/// beside its own children answers `Ambiguous` for every packet. Refused at generation time +/// rather than discovered on the spacecraft. +#[test] +fn a_base_container_that_is_not_abstract_is_refused_by_name() { + let body = r#" + + + + + + + + + + + + + + + + + + + + + + + + + + + + "#; + + let message = refusal(layout_of(body)); + assert!( + message.contains("Base"), + "the refusal has to name it: {message}" + ); + assert!( + message.contains("no restriction criteria"), + "unexpected refusal: {message}" + ); +} + #[test] fn restriction_criteria_become_constants_and_leave_the_struct() { let body = r#" From 0bdd69bad29122680de7b77a1a5f998183ebc610 Mon Sep 17 00:00:00 2001 From: Maciej Myszkiewicz Date: Sun, 11 Oct 2026 22:27:54 +0200 Subject: [PATCH 2/2] fixup: no let-chain, because this crate's floor is 1.85 `if a && let Some(b) = c` is unstable there, and a toolchain of 1.99 compiles it without a word. Checked against `cargo +1.85` this time rather than against the one that happens to be installed. --- crates/xtce-flight/src/layout.rs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/crates/xtce-flight/src/layout.rs b/crates/xtce-flight/src/layout.rs index 06deefe..428f313 100644 --- a/crates/xtce-flight/src/layout.rs +++ b/crates/xtce-flight/src/layout.rs @@ -315,9 +315,11 @@ pub fn build(plan: &Plan) -> Result { // false, which makes an unmarked base container, the ordinary CCSDS primary-header shape, // exactly that. Refused here by name rather than generated into a decoder that rejects // every packet once it is on the spacecraft. - if containers.len() > 1 - && let Some(unconditional) = containers.iter().find(|held| held.constants.is_empty()) - { + let unconditional = containers + .iter() + .find(|held| held.constants.is_empty()) + .filter(|_| containers.len() > 1); + if let Some(unconditional) = unconditional { return Err(FlightError::Unsupported { element: "SequenceContainer".to_owned(), container: unconditional.xtce_name.clone(),