search

markabrahams / node-net-snmp

JavaScript implementation of the Simple Network Management Protocol (SNMP)
206 stars 97 forks source link

MIB parsing: comment line detection and unclosed strings/quotations #222

Closed cdx50 closed 1 year ago

cdx50 commented 1 year ago

I've encountered some parsing issues with a few MIB files that I believe I've solved with PR #221 .

Commit e766d81 Part of my discovered issue is that when a line in a DESCRIPTION string contains --, it is erroneously stripped out as a comment. For example, see CISCO-LWAPP-CDP-MIB on lines 365 and 494. While the "horizontal lines" in the ASCII art are using --, these lines also include the terminating ", which causes some parsing issues. With this PR, the full DESCRIPTION text is preserved, and the closing " is not mistakenly dropped.

Commit d5189fd In addition to the above issue, I had another file that when importing had a missing/unmatched " for a string. See CISCO-LWAPP-REAP-MIB line 2032. This causes the CharBuffer object to begin processing the next file with invalid state values. This PR does a "reset" of the necessary CharBuffer object properties between each module being parsed.

Commit 86c329b While hunting down these issues with the parser, I came up with a basic way to detect an improperly formatted/parsed file and generate a console warning with an approximate location in the file as to where the issue is located. As this will print out a message with console.warn(), should this be something that is off by default and only toggled on during development/debugging through some method or option? Willing to accept some feedback on how this should work and the format of the warning and tune it up before promoting to full PR and merging.

Thanks

markabrahams commented 1 year ago

Hi @cdx50 - thanks for the PR!

I think that it's appropriate to console.warn() formatting/parsing problems in production. To align message formatting with the other (one!) occurrence of a console message in the MIB parser, I'd suggest leading with ModuleName e.g [ModuleName]: Incorrect formatting...

Other than that minor formatting change, I'm happy for you to finalize the PR, and I'll merge it.

cdx50 commented 1 year ago

Thanks for the feedback @markabrahams, I've updated the warning message and promoted the PR.

markabrahams commented 1 year ago

Thanks @cdx50 - I've merged the PR and published in version 3.9.2 of the npm.