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

Various fixes and improvements regarding support of real-world MIBs #3

Merged
merged 18 commits into from
Sep 2, 2024

Commits on Aug 24, 2024

  1. Always use the logger for debugging output

    The pysmi library uses a proper logging facility to generate its
    debugging output in all cases, except for one location, which was
    changed by commit git-0b2378ec to use print(). The result of that
    change is that users of the library can no longer disable the output
    from that debugging statement, thereby requiring work-arounds whenever
    clean output is needed from the library.
    
    This commit changes back the debugging statement to use the logging
    facility as before.
    dcvmoole committed Aug 24, 2024
    Configuration menu
    Copy the full SHA
    de2da41 View commit details
    Browse the repository at this point in the history
  2. Fix cyclic dependencies resulting in freezes

    A MIB may attempt to import itself, and multiple MIBs may attempt to
    import each other. The pysmi compiler deals well with such cases as long
    as the MIB *file* name matches the MIB name *within* the file. However,
    the latter name is used to track whether a specific MIB has already been
    loaded (or failed to load), which means that if that name differs from
    the MIB's file name, pysmi may end up trying to load the MIB with the
    same file name more than once, which results in an endless loop of
    trying to load the same MIB file(s) repeatedly. In such cases, the
    "mibdump" tool will simply end up hanging forever.
    
    This commit adds basic protection for such freezes by ensuring that
    every MIB file is loaded at most once. Depending on the specific MIBs,
    circular imports may result in either successful or failed compilation.
    That is: this commit does not aim to ensure the best possible outcome in
    such cases, as they are rare and not generally supported. However, this
    commit does ensure that such cases do not block progress.
    dcvmoole committed Aug 24, 2024
    Configuration menu
    Copy the full SHA
    b53ab29 View commit details
    Browse the repository at this point in the history
  3. Sort imported symbols in generated pysnmp code

    For development, debugging, and regression testing purposes, it is
    useful for code generation to be deterministic, in that when the same
    (unchanged) MIB is compiled to Python code multiple times, the same code
    is generated each time--at least, aside from the header comments.
    
    The pysmi code generator currently already gets very close to that goal,
    but falls short when it comes to the generated lists of symbols per
    imported module. This commit makes generation of those lists
    deterministic as well.
    dcvmoole committed Aug 24, 2024
    Configuration menu
    Copy the full SHA
    b7a32fc View commit details
    Browse the repository at this point in the history
  4. Fix TEXTUAL-CONVENTION unit tests

    Due to being misnamed, the set of basic unit test cases for the
    TEXTUAL-CONVENTION macro were not being executed. From the way they were
    implemented, it is clear they were never tried in the first place,
    either.
    
    This commit rewrites the unit tests to work properly. As it turns out,
    they also reveal a bug: the Python code generation unintentionally
    omitted the REFERENCE clause from the generated code. This commit also
    fixes that aspect.
    dcvmoole committed Aug 24, 2024
    Configuration menu
    Copy the full SHA
    002bee5 View commit details
    Browse the repository at this point in the history
  5. Fix DEFVAL support for objects with Bits syntax

    The switch to Jinja2 based code generation in git-a42d170 inadvertently
    broke support for DEFVAL clauses of object types with syntax Bits. Ever
    since, MIB files that made use of DEFVAL for Bits types would cause
    pysmi's pysnmp code generation process to fail on an exception. JSON
    code generation did work, but did not produce the intended output.
    
    This commit fixes the DEFVAL support for Bits, thereby making various
    previously uncompilable MIB files not only compile, but also produce the
    correct default values for Bits objects for objects upon use. The commit
    also adds a pair of regression test cases.
    
    This commit leaves open the issue that a DEFVAL clause with an empty bit
    set will result in object instances that have no default value.
    
    Please note that as indicated above, this commit makes a small breaking
    change to the JSON output format structure for Bits DEFVAL values.
    dcvmoole committed Aug 24, 2024
    Configuration menu
    Copy the full SHA
    fc03123 View commit details
    Browse the repository at this point in the history
  6. Fix DEFVAL for zero integer values, empty bit sets

    Previously, DEFVAL values of 0 (zero) for integer syntaxes would be
    ignored, thereby causing instantiated objects to have no default value
    rather than the default value of 0. Similarly, Empty DEFVAL bit sets
    would be ignored, causing corresponding Bits-type object instances to
    have no default value rather than an empty bit set as default value.
    
    These problems, which have been in place since the inception of pysmi,
    appear to be due mainly to an if-condition that intended to test for
    an altogether different case, namely invalid object identifier values.
    
    This commit addresses both above problems, with changes to the parser.
    The test set is extended with new tests to cover the three involved
    cases. An existing test needed to be fixed as well, by removing its
    DEFVAL clause: the change revealed a latent bug in that test, which
    could also be triggered previously by using a non-zero DEFVAL value.
    dcvmoole committed Aug 24, 2024
    Configuration menu
    Copy the full SHA
    74389de View commit details
    Browse the repository at this point in the history
  7. Fix DEFVAL support for object identifier syntaxes

    For DEFVAL clauses in objects of syntax OBJECT IDENTIFIER, the pysnmp
    code generation process did does not generate the intended code, which
    resulted in nonsensical actual default values for such object instances,
    instead of usable OIDs. This issue has been in place since the switch
    to Jinja2 based code generation introduced by git-a42d170, which seems
    to expect named OIDs whereas this case involves only numeric OIDs, thus
    resulting in an unintended "stringification" of a Python tuple.
    
    This commit fixes the problem and adds a basic test case.
    dcvmoole committed Aug 24, 2024
    Configuration menu
    Copy the full SHA
    8fce06c View commit details
    Browse the repository at this point in the history
  8. Fix augmentations depending on future definitions

    An AUGMENTS clause causes the pysnmp code generation to issue a call on
    the object being augmented, from the context of the object performing
    the augumentation. However, if the former object is defined later on in
    the MIB, it does not yet exist when being augmented. The result is that
    loading the generated code fails on a NameError exception.
    
    This commit fixes the problem by issuing the augmentation-related calls
    in a second pass, after all the objects have been defined. During the
    second pass, the object being augmented is guaranteed to exist.
    dcvmoole committed Aug 24, 2024
    Configuration menu
    Copy the full SHA
    80b8afa View commit details
    Browse the repository at this point in the history
  9. Export MIB nodes using their original names

    MIB symbol names that contain hyphens cannot be used directly as
    identifier names in the generated Python code. Therefore, such hyphens
    are replaced with underscores when generating code for various MIB
    nodes.
    
    However, the generated export table also exports the symbols using the
    "Pythonized" name as symbol name, rather than the original name. Before
    the Jinja2 rewrite of git-a42d170, the original symbol names were then
    made available by generating calls to setLabel() on MIB nodes. The label
    generation got lost as part of that rewrite, with only some loose code
    fragments remaining. That caused import errors for various MIBs.
    
    Restoring the setLabel() calls would not be difficult. However, there is
    no reason to export symbols using their Pythonized name at all: instead,
    they can be exported by means of their original names from the MIB
    files. The effect is the same, with less code and fewer exceptions to be
    made. In addition, a next commit will enable exporting types. Types are
    not covered by the existing setLabel() facility, and therefore would
    always have required being exported by their real names.
    
    Therefore, this commit exports the MIB nodes with their original names,
    so that those nodes can be imported using those same names, as was
    already generally expected. Therefore, this commit gets rid of the
    aforementioned import errors, mainly by simplifying existing code.
    
    As part of this change, the Jinja2 template is cleaned up, as it
    performed many hyphen-to-underscore conversions that were entirely
    redundant: with imported symbols as only exception, the symbol names are
    always already converted to their Python versions before being passed to
    the template. The template clean-up is necessary to avoid confusing the
    reader as to which names are "Pythonized" or not already, and also
    serves as the basis for another next commit (regarding Python keywords).
    
    The existing tests are extended with new test cases to ensure that the
    use of hyphens results in generation of proper Python code and that the
    resulting MIB nodes' getLabel() method calls return the original names.
    
    Please note that this commit makes a breaking change to the JSON output:
    the "name" entries now contain the original, un-Pythonized version of
    their corresponding entities. Note that in the previous JSON format, the
    original node name from the MIB was not available at all.
    dcvmoole committed Aug 24, 2024
    Configuration menu
    Copy the full SHA
    f9fcd5e View commit details
    Browse the repository at this point in the history
  10. Export type declarations

    Many MIBs rely on the ability to import type declarations from other
    MIBs. For unknown reasons, the conversion to Jinja2 dropped the export
    of type declarations, therefore causing those MIBs to fail to load. That
    omission was likely unintentional; there are no unit tests that cover
    this aspect.
    
    This commit enables exporting type declarations from MIBs again, so that
    those can be imported from other MIBS as well. The commit also adds unit
    tests to avoid future regressions. Note that such tests necessarily rely
    on a bit more functionality from pysnmp.
    dcvmoole committed Aug 24, 2024
    Configuration menu
    Copy the full SHA
    c05fcbc View commit details
    Browse the repository at this point in the history
  11. Fix stacked TEXTUAL-CONVENTION declarations

    Even though RFC 2579 Sec. 3.5 explicitly forbids using a Textual
    Convention as SYNTAX value of another Textual Convention, deriving
    textual conventions from other textual conventions is widely used in
    practice. However, for such cases, pysmi currently generates Python
    code that cannot be loaded, as it leads to "Method Resolution Order"
    errors.
    
    The reason for those errors is that classes for textual conventions
    derive from both the TextualConvention base class and the underlying
    syntax class--in that order. In these "stacked" cases, the underlying
    syntax class itself also derives from the TextualConvention base class.
    
    This commit resolves those errors by changing the order of the base
    classes. The test set is extended to check various cases that should
    now be working properly as a result.
    dcvmoole committed Aug 24, 2024
    Configuration menu
    Copy the full SHA
    84be3ff View commit details
    Browse the repository at this point in the history
  12. Fix use of reserved Python keywords

    This commit fixes issues related to the use of Python keywords in MIB
    files, which cause problems in the generated Python code if left as is.
    Support for translating such reserved symbols to non-reserved symbols
    was already already in place to some degree, but inadequate, as there
    were still issues for both defining and importing symbols.
    
    For defined symbols, this change replaces the incomplete solution from
    git-b091479b with the original fix of git-00177b06, which was still in
    place for symtable.py but got dropped for intermediate.py as part of the
    Jinja2 rewrite of git-a42d170.
    
    For imported symbols, this change centralizes the symbol replacement and
    ensures that the Jinja2 template generates an import table with local
    symbol names that have been subject to appropriate replacement.
    
    The commit adds various new tests for both cases.
    dcvmoole committed Aug 24, 2024
    Configuration menu
    Copy the full SHA
    830b154 View commit details
    Browse the repository at this point in the history
  13. Properly enquote, escape, preserve text values

    Free-form text values, such as DESCRIPTION strings, may contain a few
    characters that can be problematic when transformed to Python strings:
    
     1. Line terminator characters may be used in any textual strings, as
        per RFC 2578 Sec. 3.1.1. The Jinja2 template uses triple-quote
        strings to support such characters in some of those textual strings,
        but not in all of them. When line terminator characters are used
        within strings that are transformed to single-quote Python strings,
        the resulting Python code can not be compiled.
    
     2. Backslash characters, which are similarly allowed in such textual
        strings, are interpreted within Python strings. They should instead
        be converted to literal characters. Note that the final character of
        a string may be a backslash, so use of Python's "r" string prefix
        does not fully solve this problem.
    
    This commit adds a Jinja2 filter that can be used to convert any MIB-
    provided textual string to a Python string with proper quoting and
    escaping of backslashes. That new filter is now used for all textual
    strings. To be exact: it is now used for all strings that are ultimately
    obtained via the QUOTED_STRING parser token, which is (or at least
    should be) the only way through which free-form text values can make
    their way from input MIBs into the code generator.
    
    As direct effect of this change, the proper functionality of mibdump's
    "--keep-texts-layout" switch is restored. Since the switch to Jinja2 of
    commit git-a42d1705, triple-quoted lines had an extra line terminator
    character added at the end, and were always getting word-wrapped. Both
    aspects are dropped with this change, so that "--keep-texts-layout"
    results in a compiled MIB that perfectly preserves the original text
    (aside from the original line terminator encodings, i.e., LF vs CR/LF).
    
    The existing tests have been changed to reflect the removal of the
    extra line terminator character as added with git-a42d1705. A few extra
    representative tests are added to ensure that the above two cases are
    handled properly in general and that the implementation of the
    "--keep-texts-layout" switch functions as expected.
    dcvmoole committed Aug 24, 2024
    Configuration menu
    Copy the full SHA
    1e4172e View commit details
    Browse the repository at this point in the history

Commits on Aug 27, 2024

  1. Add support for mistyped "SEQUENCE OF" type names

    As per RFC 2578, conceptual-table object types (i.e., xyzTable) are
    expected to have a "SEQUENCE OF" syntax whose type matches the syntax of
    the table row (i.e., of xyzEntry). The pysmi symbol table building
    procedure relies on that fact, and currently rejects any MIBs that do
    not conform to that rule. However, there are several real-world MIBs
    that have typos in their "SEQUENCE OF" type names.
    
    This commit adds a leniency exception to the code in order to allow MIBs
    with such typos to be compiled and loaded after all. The exception is
    applied only when compiling the MIB would certainly fail otherwise, so
    compilation of valid MIBs is guaranteed not to be affected by this
    change. The exception requires keeping track of SEQUENCE types, but as
    non-SEQUENCE types are already tracked as well, this is not considered a
    substantial resource drain. A test case is added to verify the leniency.
    dcvmoole committed Aug 27, 2024
    Configuration menu
    Copy the full SHA
    ebf0e1b View commit details
    Browse the repository at this point in the history
  2. Add support for SMIv1 index syntaxes

    SMIv2 requires that an INDEX clause lists a series of object-type names.
    Unlike SMIv2 however, SMIv1 (as per RFC 1212 Sec. 2) also supports that
    such a series contains certain index syntaxes instead of (or in addition
    to) object-type names. For example, SMIv1 allows "INDEX { INTEGER }".
    
    While pysmi's parser supports the latter case, its code generator has
    support in place in its symbol table builder only, with the intermediate
    module merely having a few loose and non-functional code fragments with
    the remainder being left as a to-do item.
    
    This commit finishes the support for SMIv1 index syntaxes, by properly
    generating "fake column" object-type instances for the index syntaxes,
    as was likely the original intention as well.
    
    A few duplicate class variables in the symbol table and intermediate
    modules are moved to the base class, as successful Python code
    generation relies on the fact that both subclasses use exactly the same
    values for those. While there, a small set of unused and obsolete class
    variables is removed altogether.
    
    The test set is extended with a new test module that covers the new
    SMIv1-only functionality.
    dcvmoole committed Aug 27, 2024
    Configuration menu
    Copy the full SHA
    61bf810 View commit details
    Browse the repository at this point in the history
  3. Fix rewriting imports from RFC1158-MIB

    As part of pysmi's code generation, various imports are rewritten from
    SMIv1 to SMIv2, so as to simplify the way in which standard MIBs are
    handled. However, an apparent copy-paste error prevents that imports
    from RFC1158-MIB are being rewritten as intended. The result is that
    MIBs importing from RFC1158-MIB could be compiled but not loaded.
    
    This commit fixes the copy-paste error, and adds a few basic test cases
    to ensure that each group of rewrites is working as intended.
    dcvmoole committed Aug 27, 2024
    Configuration menu
    Copy the full SHA
    21eded6 View commit details
    Browse the repository at this point in the history
  4. Deal properly with IMPORTS being absent

    Omitting the IMPORTS statement is valid, but pysmi does not deal with
    this case as it should. When there is an IMPORTS statement, pysmi
    extends the parsed imports table with necessary extra imports. When the
    IMPORTS statement is omitted however, that extension is applied to a
    temporary throw-away import table instead.
    
    For example, when a MIB without an IMPORTS table uses the INTEGER type,
    pysmi rewrites INTEGER to Integer32 as usual. However, since the MIB's
    imports table is not extended in this case, Integer32 itself is never
    imported from SNMPv2-SMI, resulting in a load-time error.
    
    This commit ensures that there is always a parsed import table, even if
    it is empty. The extensions are then applied to it in all cases, and the
    above example works as intended. A test case is added as well.
    dcvmoole committed Aug 27, 2024
    Configuration menu
    Copy the full SHA
    2523147 View commit details
    Browse the repository at this point in the history
  5. Allow tables to be used as table columns

    This fix is specifically for WIENER-CRATE-MIB, which uses a "SEQUENCE"
    declaration that includes a table object symbol. As a result, the
    corresponding object type is considered to be a table column, resulting
    in generated Python code that cannot be loaded.
    
    This commit adds an extra restriction for object types to be considered
    table columns, namely that they must not have a "SEQUENCE OF" type
    syntax. In other words, object types that are tables are no longer ever
    considered to be table columns. That restriction allows WIENER-CRATE-MIB
    to be loaded properly.
    
    The commit also adds a test case, which consists of a simplified version
    of the construction found in WIENER-CRATE-MIB.
    dcvmoole committed Aug 27, 2024
    Configuration menu
    Copy the full SHA
    88d214c View commit details
    Browse the repository at this point in the history