Skip to content

Fix leafref binary search accepting entries excluded by xpath predicates - #700

Open
khromenokroman wants to merge 2 commits into
clicon:masterfrom
khromenokroman:fix/leafref-opt-predicate
Open

khromenokroman wants to merge 2 commits into
clicon:masterfrom
khromenokroman:fix/leafref-opt-predicate

Conversation

@khromenokroman

Copy link
Copy Markdown
Contributor

@olofhagsand Hello

Description:
Related to #669

Problem

leafref_opt_search_detect() enables binary search when all nodes in xvec
have the same YANG spec and share the same parent (leaf-list) or grandparent
(list key). leafref_opt_search() then searches among all siblings under
that parent, but xvec may be only a subset of them if the leafref path
contains a predicate on the list, e.g.:

  leaf ref {
    type leafref { path "../iface[enabled='true']/name"; }
  } 

A reference to an entry that the predicate excluded is then wrongly accepted
instead of failing with instance-required.

This already happened when exactly one entry matched the predicate. Since
ff7dafe (#669), which allows list entries with different parents, it happens
for any number of matches.

Example with LEAFREF_OPTIMIZE (default):

  <top xmlns="urn:t">
    <iface><name>a</name><enabled>true</enabled></iface>
    <iface><name>b</name><enabled>false</enabled></iface>
    <iface><name>c</name><enabled>true</enabled></iface>
    <ref>b</ref>   <!-- validates OK, but must fail -->
  </top>

With LEAFREF_OPTIMIZE undefined (linear search) the same data correctly
fails validation.

Fix

Use binary search only if xvec contains all siblings with the same YANG
spec, i.e. the number of such children of the common parent equals xlen.
If the xpath filtered anything out, fall back to the linear search.

Predicates above the common parent (e.g. selecting a context entry with
/contexts/context[name=current()/...]/...) do not filter the siblings and
keep the optimization. The extra check is a single pass over the parent's
children, the same order as the existing detection loop.

Testing

  • test/test_leafref.sh: added sender/active-template leafref
    /sender[enabled='true']/name; a reference to a disabled sender must fail
    validation, a reference to an enabled one must pass.

  • Verified with a standalone validation program (xml_yang_validate_all_top):

    case linear master this PR
    ref to disabled entry, 1 match fail pass fail
    ref to disabled entry, 2 matches fail pass fail
    ref to enabled entry pass pass pass
  • Performance unchanged for paths without predicates (the List binary search is broken #669 case):
    20000 list entries + 20000 leafrefs validate in 0.29s, vs 75s with linear search.

leafref_opt_search_detect() enabled binary search when all xvec entries
share the same yang and parent (leaf-list) or grandparent (list key).
leafref_opt_search() then searches among ALL siblings of that parent,
but xvec may be only a subset of them if the leafref path has a
predicate, eg "../iface[enabled='true']/name". A reference to an entry
excluded by the predicate was then wrongly accepted.

This was the case for a single match before, and for any number of
matches after the fix for clicon#669
allowed list entries with different parents.

Only use binary search if xvec contains all such siblings, ie the
number of siblings with the same yang equals xlen. Predicates above the
common parent (eg selecting a context) do not filter siblings and keep
the optimization.
…ries

Add sender/active-template leafref "/sender[enabled='true']/name" and
check that a reference to a disabled sender fails validation while a
reference to an enabled one passes.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant