ros2-bridge msg-gen: per-element `string<=N` / `wstring<=N` bound is not enforced for array/sequence member defaults

[phil-opp]

Note: This issue was created by a scheduled automated Claude check that randomly selected libraries/extensions/ros2-bridge/msg-gen and reviewed it for correctness issues. The finding was verified by tracing the code paths against the parser's own tests. Please verify before acting on it.

Summary

When validating a default value for a member whose type is an array or sequence of bounded strings (string<=N[...] / wstring<=N[...]), the declared per-element length bound N is silently dropped and never checked. The equivalent scalar bounded-string path does enforce the bound. So an over-long element in an array/sequence default is wrongly accepted, while the same over-long value as a scalar default is correctly rejected.

This is distinct from the two previously-filed (and closed) ros2-bridge issues:

• ros2-bridge msg-gen: bounded string<=N / wstring<=N default validation uses byte length instead of character count #2363 was about the byte-vs-character metric of the scalar bound check (now intentionally byte-based for string<=N per the ROS 2 spec). This issue is about the array/sequence path skipping the bound entirely, regardless of metric.
• ros2-bridge msg-gen: string-array constant with mismatched element count panics during codegen instead of erroring #2826 was about the array element-count check (values.len() == size) for constants. The element count is checked for members here — the gap is the per-element length bound.

Location

libraries/extensions/ros2-bridge/msg-gen/src/parser/member.rs, array_type_default, lines 47-52:

NestableType::GenericString(_) => {
    let (rest, default) = literal::string_literal_sequence(default)
        .map_err(|_| RclMsgError::ParseDefaultValueError(default.into()))?;
    ensure!(rest.is_empty());
    Ok(default)
}

The GenericString(_) pattern discards the payload, which may be BoundedString(N) / BoundedWString(N). It then calls literal::string_literal_sequence, which takes no bound argument and performs no per-element length check (literal.rs:173).

Contrast the scalar path in the same file, nestable_type_default (lines 26-31), which threads the bound through:

NestableType::GenericString(t) => {
    let (rest, default) = literal::get_string_literal_parser(t)(default)  // `t` carries the bound

get_string_literal_parser (literal.rs:122-130) enforces s.len() <= max_size for BoundedString and s.encode_utf16().count() <= max_size for BoundedWString. That enforcement is bypassed for arrays and sequences.

Reachability

The array element type can be a bounded string — the parser's own test constructs exactly this shape (parser/types.rs:236-238, parsing string<=6[5]):

Array {
    value_type: GenericString::BoundedString(6).into(),
    size: 5,
}

So a .msg line like string<=6[2] field ["toolongvalue", "ok"] routes into the GenericString(_) arm above with the bound 6 ignored.

Triggering input (a .msg line)

• Scalar string<=3 field "hello" → correctly rejected ("hello" is 5 bytes > 3).
• Array string<=3[2] field ["hello", "hi"] → wrongly accepted; the over-long element "hello" passes because the bound is never checked.

Same applies to unbounded/bounded sequences of bounded strings (string<=3[], string<=3[<=4]) and to wstring<=N[...] (UTF-16 code-unit bound).

Impact

Default-value validation silently accepts out-of-bound bounded-string elements in array/sequence member defaults, in contradiction with the ROS 2 interface bound and inconsistent with the scalar path's own enforcement. Low severity (requires a bounded-string array/sequence field with an out-of-bound default), but a genuine validation gap.

Suggested fix

Thread the GenericString bound into the sequence element parser instead of discarding it, enforcing the per-element byte / UTF-16-code-unit bound for each element (mirroring get_string_literal_parser). For example, capture the variant in array_type_default:

NestableType::GenericString(t) => {
    let (rest, default) = literal::string_literal_sequence(default)
        .map_err(|_| RclMsgError::ParseDefaultValueError(default.into()))?;
    ensure!(rest.is_empty());
    // enforce the per-element bound, matching the scalar path
    if let GenericString::BoundedString(max) = t {
        ensure!(default.iter().all(|s| s.len() <= max));
    } else if let GenericString::BoundedWString(max) = t {
        ensure!(default.iter().all(|s| s.encode_utf16().count() <= max));
    }
    Ok(default)
}

(Or add a bound-aware variant of string_literal_sequence.) A regression test with an out-of-bound element under a tight string<=N[...] bound would prevent recurrence.