mbj4668 / pyang

An extensible YANG validator and converter in python
ISC License
537 stars 344 forks source link

Comment on line 1 breaks pyang #725

Open sardobi opened 3 years ago

sardobi commented 3 years ago

This was seen with pyang 2.4.0

Steps to replicate:

  1. Create a .yang file with anything you like in the body - I've seen this with well-formed, perfectly functional files as well as simplified example files. Here's a simple example, test.yang, which shows the behaviour:
    // A comment
    module test {}
  2. Run pyang -V --format yang test.yang
  3. Get the following output:
    # module search path: .:/home/developer/yang/modules:/home/developer/.virtualenvs/my-project/share/yang/modules
    # read test.yang (CL)
    Traceback (most recent call last):
    File "/home/developer/.virtualenvs/my-project/bin/pyang", line 533, in <module>
      run()
    File "/home/developer/.virtualenvs/my-project/bin/pyang", line 453, in run
      epos.top.arg not in modulenames and
    AttributeError: 'NoneType' object has no attribute 'arg'

Replacing the comment with random strings of alphanumeric characters produces a more normal-looking validation error:

  # module search path: .:/home/developer/yang/modules:/home/developer/.virtualenvs/my-project/share/yang/modules
  # read test.yang (CL)
  test.yang:2: error: unterminated statement definition for keyword "fiwee468236ghedwgb", looking at t

(though the error message seems truncated).

It isn't just comments on the first line which make this happen - any non-alphanumeric character being present anywhere on the first line (but not in an expected position) seems to cause it.

Happy to provide more info if needed, with the proviso that I know very little about yang or pyang, sorry!

fredgan commented 3 years ago

Hi @sardobi You are right. Pyang just accepts a grammar tree. So pyang is not able to deal the statements outside the grammar tree. I pushed a PR to ignore the comments outside the module. Anyway, it won't report the error now. Please try again with the latest master branch.

wlupton commented 3 years ago

But won't this mean that the comment is not present in yang output (with -f yang)? I feel it might be better instead not to support such comments at all (perhaps improve the error message though).

(Note: A long time ago I flirted with supporting such comments, but it was quite messy. As I recall, the approach was to create a fake root node that could then have comment and module/submodule child statements.)

fredgan commented 3 years ago

Hi @wlupton Yes, the comments outside the module would be ignored. It's a simple solution to resolve the error but not a thorough solution.

In fact, I thought of your approach, but I am not sure if it will affect other function or not. Anyway, I will try this approach later.

wlupton commented 3 years ago

Thanks. See PR #244.