Skip to content

feat(yang-push): normalize target xpath filters and add failed xpath resolution check with libyang #45

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
rodonile wants to merge 7 commits into
base: main
Choose a base branch
from
Draft

feat(yang-push): normalize target xpath filters and add failed xpath resolution check with libyang #45

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
209ee6e
fix(yang-push): resolve xpath-filter target modules by prefix
rodonile Aug 21, 2026
7d62119
fix(yang-push): scope xpath prefix module lookup by datastore
rodonile Aug 21, 2026
de80e56
refactor(netconf-proto): consolidate xpath helpers into new module
rodonile Aug 25, 2026
c45c5a9
feat(netconf-proto): add xpath normalize_path engine
rodonile Aug 25, 2026
cfda43b
feat(yang-push): normalize NETCONF-sourced target xpath filter
rodonile Aug 25, 2026
a041df5
feat(yang-push): normalize JSON-sourced target xpath filter
rodonile Aug 25, 2026
6a66ed9
feat(yang-push): warn on non-canonical or unresolvable target xpath
rodonile Aug 25, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions crates/netconf-proto/src/client.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -755,10 +755,14 @@ impl<T: AsyncRead + AsyncWrite + Unpin> NetConfSshClient<T> {
if let Some(RpcResponse::WellKnown(WellKnownRpcResponse::Data(data))) =
rpc_reply.reply().responses()
{
trace!("[{}] Raw <get> response for filters: `{data}`", self.peer);

let mut reader = NsReader::from_str(data);
reader.config_mut().trim_text(true);
let mut parser = crate::xml_utils::XmlParser::new(reader)?;
let filters = Filters::xml_deserialize(&mut parser)?;

trace!("[{}] Parsed filters: {filters:?}", self.peer);
return Ok(filters);
}
Err(NetConfSshClientError::UnexpectedMessage {
Expand Down Expand Up @@ -792,6 +796,10 @@ impl<T: AsyncRead + AsyncWrite + Unpin> NetConfSshClient<T> {
if let Some(RpcResponse::WellKnown(WellKnownRpcResponse::Data(data))) =
rpc_reply.reply().responses()
{
trace!(
"[{}] Raw <get> response for subscription {id}: `{data}`",
self.peer
);
// Parse the response streams if any returned, filters if any returned
// and then the subscription details
let mut reader = NsReader::from_str(data);
Expand All @@ -801,6 +809,10 @@ impl<T: AsyncRead + AsyncWrite + Unpin> NetConfSshClient<T> {
{
parser.open(Some(SUBSCRIBED_NOTIFICATIONS_NS), "subscriptions")?;
let mut subscription = Subscription::xml_deserialize(&mut parser)?;
trace!(
"[{}] Parsed subscription {id} before target-by-reference resolution: {subscription:?}",
self.peer
);
if let Target::Datastore(datastore_target) = &mut subscription.target
&& let DatastoreSelectionFilterObjects::ByReference(name) =
&datastore_target.selection
Expand All @@ -818,6 +830,13 @@ impl<T: AsyncRead + AsyncWrite + Unpin> NetConfSshClient<T> {
}
}
parser.close()?;
trace!(
"[{}] Final subscription {id}: target={:?} module_version={:?} yang_library_content_id={:?}",
self.peer,
subscription.target,
subscription.module_version,
subscription.yang_library_content_id
);
subscription
} else {
return Err(NetConfSshClientError::ParsingError(
Expand Down
2 changes: 2 additions & 0 deletions crates/netconf-proto/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
// Copyright (C) 2026-present The NetCalyx Authors.
// Copyright (C) 2025-present The NetGauze Authors.
//
// Licensed under the Apache License, Version 2.0 (the "License");
Expand Down Expand Up @@ -30,6 +31,7 @@ pub mod client;
pub mod codec;
pub mod protocol;
pub mod xml_utils;
pub mod xpath;
pub mod yang_push;
pub mod yanglib;
pub mod yangparser;
Expand Down
225 changes: 1 addition & 224 deletions crates/netconf-proto/src/xml_utils.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -764,7 +764,7 @@ impl<'a, R: io::BufRead> XmlParser<'a, R> {
all_namespaces.insert(prefix, String::from_utf8_lossy(ns).into_owned());
}
let path = self.tag_string()?;
let used_namespaces = Self::find_xpath_prefixes(&path);
let used_namespaces = crate::xpath::find_xpath_prefixes(&path);
let namespaces: IndexMap<String, String> = all_namespaces
.into_iter()
.filter(|(prefix, _)| used_namespaces.contains(prefix))
Expand Down Expand Up @@ -811,59 +811,6 @@ impl<'a, R: io::BufRead> XmlParser<'a, R> {
}
}
}

/// Find prefixes used within an Xpath expression
fn find_xpath_prefixes(xpath: &str) -> HashSet<String> {
let mut prefixes = HashSet::new();
let mut chars = xpath.char_indices().peekable();
let mut in_single = false;
let mut in_double = false;

while let Some((i, c)) = chars.next() {
// Skip over string literals — colons inside them aren't prefixes.
if in_single {
if c == '\'' {
in_single = false;
}
continue;
}
if in_double {
if c == '"' {
in_double = false;
}
continue;
}
match c {
'\'' => in_single = true,
'"' => in_double = true,
c if c.is_ascii_alphabetic() || c == '_' => {
let start = i;
let mut end = i + c.len_utf8();
while let Some(&(_, nc)) = chars.peek() {
if nc.is_ascii_alphanumeric() || nc == '_' || nc == '-' || nc == '.' {
chars.next();
end += nc.len_utf8();
} else {
break;
}
}
// A prefix is an NCName followed by exactly one ':'
// (two colons = axis specifier like `child::`).
if let Some(&(_, ':')) = chars.peek() {
let mut look = chars.clone();
look.next();
let is_axis = matches!(look.peek(), Some(&(_, ':')));
if !is_axis {
prefixes.insert(xpath[start..end].to_string());
chars.next(); // consume the ':'
}
}
}
_ => {}
}
}
prefixes
}
}

/// Format a `DateTime<Utc>` as YANG `date-and-time` (RFC 3339, UTC).
Expand Down Expand Up @@ -1467,176 +1414,6 @@ mod tests {
assert!(xml_writer.ns_applied);
}

fn set<const N: usize>(items: [&str; N]) -> HashSet<String> {
items.iter().map(|s| s.to_string()).collect()
}

fn assert_prefixes(expr: &str, expected: HashSet<String>) {
assert_eq!(
XmlParser::<io::Cursor<&[u8]>>::find_xpath_prefixes(expr),
expected,
"unexpected prefix set for: {expr}"
);
}

#[test]
fn test_find_xpath_prefixes_yields_empty_when_no_qnames_present() {
// Empty/whitespace input, unprefixed paths, pure numeric/operator
// expressions, the `current()` function, and bare node tests all
// contain no QNames — so nothing should be reported.
for expr in [
"",
" \n\t",
"/interfaces/interface/name",
"1 + 2.5 - 3 <= 4 and 5 != 6",
"current()",
"node() | text() | comment() | processing-instruction()",
] {
assert_prefixes(expr, HashSet::new());
}
}

#[test]
fn test_find_xpath_prefixes_test_extracts_prefixes_from_simple_location_paths() {
// Motivating Huawei debug case, RFC 8641 Figure 12 (`/ex:foo`),
// the subscribed-notifications `/int:interfaces` example,
// prefix deduplication, and multi-prefix paths.
let cases: &[(&str, HashSet<String>)] = &[
(
"/debug:debug/debug:board-resouce-states/debug:board-resouce-state",
set(["debug"]),
),
("/ex:foo", set(["ex"])),
("/int:interfaces", set(["int"])),
("/if:interfaces/if:interface/if:name", set(["if"])),
("/a:x/b:y/c:z", set(["a", "b", "c"])),
];
for (expr, expected) in cases {
assert_prefixes(expr, expected.clone());
}
}

#[test]
fn test_find_xpath_prefixes_recognizes_full_ncname_charset_in_prefixes() {
// NCName permits letters, digits, `_`, `-`, `.`
// (the last three may not start the name).
let cases: &[(&str, HashSet<String>)] = &[
// Hyphenated — common in OpenConfig.
(
"/oc-if:interfaces/oc-if:interface[oc-if:name='eth0']",
set(["oc-if"]),
),
// Dot in the middle (legal NCName, rare in practice).
("/a.b:c", set(["a.b"])),
// Underscore-leading.
("/_ns:leaf", set(["_ns"])),
];
for (expr, expected) in cases {
assert_prefixes(expr, expected.clone());
}
}

#[test]
fn test_find_xpath_prefixes_handles_prefixed_wildcards_and_attributes() {
let cases: &[(&str, HashSet<String>)] = &[
("/ex:*", set(["ex"])),
("//@ex:id", set(["ex"])),
("/if:interface[@nc:operation='delete']", set(["if", "nc"])),
];
for (expr, expected) in cases {
assert_prefixes(expr, expected.clone());
}
}

#[test]
fn test_find_xpath_prefixes_xpath_axes_are_never_reported_as_prefixes() {
// Every XPath 1.0 axis name followed by `::` must be skipped,
// since the `::` is an axis separator rather than a prefix colon.
const AXES: &[&str] = &[
"ancestor",
"ancestor-or-self",
"attribute",
"child",
"descendant",
"descendant-or-self",
"following",
"following-sibling",
"namespace",
"parent",
"preceding",
"preceding-sibling",
"self",
];
for axis in AXES {
assert_prefixes(&format!("{axis}::node()"), HashSet::new());
}
// Axes can still coexist with real prefixes in the same expression.
assert_prefixes("descendant::if:interface/child::if:name", set(["if"]));
}

#[test]
fn test_find_xpath_prefixes_skips_colons_inside_string_literals() {
// Single-quoted identityref comparisons (RFC 7950 §9.10),
// double-quoted variants, and mixed-quote expressions.
let cases: &[(&str, HashSet<String>)] = &[
("../crypto = 'mc:aes'", HashSet::new()),
("name() = \"ns:bogus\"", HashSet::new()),
("@a:x = 'p:q' or @b:y = \"r:s\"", set(["a", "b"])),
];
for (expr, expected) in cases {
assert_prefixes(expr, expected.clone());
}
}

#[test]
fn test_find_xpath_prefixes_handles_compound_expressions() {
// Function calls, leafref-style predicates with current(),
// unions, boolean ops across modules, and nested predicates.
let cases: &[(&str, HashSet<String>)] = &[
("ex:size(@id)", set(["ex"])),
(
"/if:interfaces/if:interface[if:name = current()/../if:name]",
set(["if"]),
),
("/a:foo | /b:bar", set(["a", "b"])),
(
"(/if:interfaces/if:interface/if:enabled = 'true') \
and count(/rt:routing/rt:routes) > 0",
set(["if", "rt"]),
),
("/a:x[a:y[b:z = '1']/a:w = c:fn()]", set(["a", "b", "c"])),
];
for (expr, expected) in cases {
assert_prefixes(expr, expected.clone());
}
}

#[test]
fn test_find_xpath_prefixes_real_world_yang_expressions() {
// ietf-interfaces-style `must`: only `if:` is a live prefix;
// the `ianaift:*` tokens are identityref values inside string
// literals and must not be reported.
let must_expr = "(/if:interfaces/if:interface[if:name=current()]/if:type \
= 'ianaift:ethernetCsmacd') \
or \
(/if:interfaces/if:interface[if:name=current()]/if:type \
= 'ianaift:ieee8023adLag')";
assert_prefixes(must_expr, set(["if"]));

// Multi-module subscriber filter for yp:datastore-xpath-filter.
let filter_expr = "/if:interfaces/if:interface[if:name='eth0'] \
| /rt:routing/rt:ribs/rt:rib[rt:name=current()/ref:rib]";
assert_prefixes(filter_expr, set(["if", "rt", "ref"]));
}

#[test]
fn test_find_xpath_prefixes_whitespace_between_ncname_and_colon_breaks_qname() {
// In XPath 1.0 a QName is lexically `NCName ':' NCName` with no
// whitespace. `if : interfaces` is three tokens, so `if` must not
// be reported as a prefix. This behavior is intentional.
assert_prefixes(" / if : interfaces ", HashSet::new());
}

#[test]
fn test_read_xpath_with_namespaces_basic_filter() {
// One prefix declared on the filter element, used in the path.
Expand Down
Loading
Loading