Skip to content
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.

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

YIN parser doesn't handle import by revision (and it doesn't even matter) #850

Open
predi opened this issue Mar 30, 2023 · 4 comments
Open

Comments

@predi
Copy link

predi commented Mar 30, 2023

Hi,

pyang's YIN parser improperly handles imports by revision for my carefully crafted test case. It dismisses the second import as part of "import cycle detection code" in yin_parser.py Line 293.

                    if modname in mymodules:
                        # circular import; ignore here and detect in validation
                        pass
<?xml version="1.0" encoding="utf-8"?>
<module xmlns="urn:ietf:params:xml:ns:yang:yin:1" 
        xmlns:t="test:uri" 
        xmlns:t1="test-dep:uri" 
        xmlns:t2="test-dep:uri" 
        name="test">
  <yang-version value="1.1"/>
  <namespace uri="test:uri"/>
  <prefix value="t"/>
  <import module="test-dep">
    <revision-date date="2023-03-01"/>
    <prefix value="t1"/>
  </import>
  <import module="test-dep">
    <revision-date date="2023-03-30"/>
    <prefix value="t2"/>
  </import>
  <revision date="2023-03-30"/>
  <container name="top">
    <t2:extension/>
    <leaf name="foo">
      <type name="t1:type"/>
    </leaf>
  </container>
</module>
<?xml version="1.0" encoding="utf-8"?>
<module xmlns="urn:ietf:params:xml:ns:yang:yin:1"
        xmlns:td="test-dep:uri" 
        name="test-dep">
  <yang-version value="1.1"/>
  <namespace uri="test-dep:uri"/>
  <prefix value="td"/>
  <revision date="2023-03-01"/>
  <typedef name="type">
    <type name="uint8">
      <range value="1..5"/>
    </type>
  </typedef>
</module>
<?xml version="1.0" encoding="utf-8"?>
<module xmlns="urn:ietf:params:xml:ns:yang:yin:1"
        xmlns:td="test-dep:uri"
        name="test-dep">
  <yang-version value="1.1"/>
  <namespace uri="test-dep:uri"/>
  <prefix value="td"/>
  <revision date="2023-03-30"/>
  <revision date="2023-03-01"/>
  <typedef name="type">
    <type name="uint8">
      <range value="1..10"/>
    </type>
  </typedef>
  <extension name="extension"/>
</module>

Run with:

pyang_tool.py [email protected]

Result:

[email protected]:9: warning: imported module "test-dep" not used
[email protected]:15: error: extension "extension" is not defined in module test-dep

Expected:
no error or warning

The test case is actually meant to demonstrate a bug in RFC 7950, Section 13, YIN format: the YANG <-> YIN mapping cannot be considered bidirectional (RFC claims this to be true) due to the fact that XML namespace bindings (needed by extensions) cannot replace the prefix:module mappings in module header statements. IMO, it was a mistake to "encode" YANG extension usages in YIN the way they currently are, as the "encoding" is inherently ambiguous for import by revision. Not to mention it requires needlessly complex YIN parser implementations (pyang's own implementation speaks for itself in this regard) that need to know the extension's definition to properly parse what should essentially be keyword:argument pairs expressed as XML.

@mbj4668
Copy link
Owner

mbj4668 commented Mar 30, 2023

This is an unforeseen effect of allowing multiple imports of the same module in YANG 1.1.

In YANG 1.0 this wasn't allowed, and there the YIN mapping works.

@predi
Copy link
Author

predi commented Mar 31, 2023

The YIN parser implementation in pyang could be revised to choose the import with the newest revision in such a case (making it even more complex).

@mbj4668
Copy link
Owner

mbj4668 commented Mar 31, 2023

Yes it could, but as you indicated, this behavior is not specified in the RFC, and from an XML-perspective it is questionable.

In

<foo xmlns:p1="urn:example:p" xmlns:p2="urn:example:p">
  <p1:bar/>
  <p2:baz/>
</foo>

The elements "bar" and "baz" both have the same namespace.

@predi
Copy link
Author

predi commented Mar 31, 2023

Indeed. It doesn't help that p1 and p2 don't need to match the prefix mappings in a YANG module (or even be supplied by an XML parser implementation to the application).

Another way out of this would be to issue a warning for such cases. If you reverse the order of import statements in my test case, pyang will pretend that everything checks out, which is not ideal either.

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

No branches or pull requests

2 participants