Summary
qualify_topic_name validates name components only on the relative and ~private branches. Absolute names — anything starting with / — are returned unchecked, so invalid components reach Zenoh unvalidated.
This affects every entity type, since publishers, subscribers, services and actions all route through this function.
Reproduction
node.create_client("/bad name", example_interfaces.AddTwoInts) # accepted
No error is raised at construction, and the client is created.
Where
crates/hiroz/src/topic_name.rs, in qualify_topic_name:
let qualified = if topic.starts_with('/') {
// Absolute topic - use as-is, but remove trailing slash if present
let topic = topic.strip_suffix('/').unwrap_or(topic);
if topic.is_empty() || topic == "/" { /* ... */ }
topic.to_string() // <-- no component validation
} else if topic.starts_with('~') {
// ... validates each component via is_valid_topic_component
} else {
// relative: also validates each component
}
is_valid_topic_component already encodes the right rule (must start with a letter or underscore, then alphanumerics/underscores). It is simply never applied to the absolute branch.
Related: the same function skips empty components rather than rejecting them (if !part.is_empty() && !is_valid_topic_component(part)), so //a//b passes ROS validation and fails later inside Zenoh's key-expression parser with an error that cites a cargo registry path and never names the offending topic.
Impact
Invalid names fail late and obscurely — at key-expression parsing or, worse, silently as a name that can never match a peer — instead of at construction with a clear error.
Why this wasn't fixed in #192
#192 (hiroz-py rclpy alignment) added a binding-level check for empty components and trailing slashes, because those produce the opaque Zenoh error. Extending validation to absolute names changes a function on the hot path of every entity in the project and could reject names existing users depend on, so it warrants its own change with its own review rather than riding along in a bindings PR.
Suggested fix
Apply is_valid_topic_component to the absolute branch, and reject empty components rather than skipping them. Both are breaking for anyone currently relying on lenient names, so this likely wants a deprecation note in the changelog.
Summary
qualify_topic_namevalidates name components only on the relative and~privatebranches. Absolute names — anything starting with/— are returned unchecked, so invalid components reach Zenoh unvalidated.This affects every entity type, since publishers, subscribers, services and actions all route through this function.
Reproduction
No error is raised at construction, and the client is created.
Where
crates/hiroz/src/topic_name.rs, inqualify_topic_name:is_valid_topic_componentalready encodes the right rule (must start with a letter or underscore, then alphanumerics/underscores). It is simply never applied to the absolute branch.Related: the same function skips empty components rather than rejecting them (
if !part.is_empty() && !is_valid_topic_component(part)), so//a//bpasses ROS validation and fails later inside Zenoh's key-expression parser with an error that cites a cargo registry path and never names the offending topic.Impact
Invalid names fail late and obscurely — at key-expression parsing or, worse, silently as a name that can never match a peer — instead of at construction with a clear error.
Why this wasn't fixed in #192
#192 (hiroz-py rclpy alignment) added a binding-level check for empty components and trailing slashes, because those produce the opaque Zenoh error. Extending validation to absolute names changes a function on the hot path of every entity in the project and could reject names existing users depend on, so it warrants its own change with its own review rather than riding along in a bindings PR.
Suggested fix
Apply
is_valid_topic_componentto the absolute branch, and reject empty components rather than skipping them. Both are breaking for anyone currently relying on lenient names, so this likely wants a deprecation note in the changelog.