Skip to content

Junos: parse and warn on empty interface description#8972

Merged
dhalperi merged 2 commits intobatfish:masterfrom
0x-0ddc0de:master
Mar 18, 2024
Merged

Junos: parse and warn on empty interface description#8972
dhalperi merged 2 commits intobatfish:masterfrom
0x-0ddc0de:master

Conversation

@0x-0ddc0de
Copy link
Copy Markdown
Contributor

The prevents crashing when interface description is empty. Junos will ignore an interface statement with an empty description (ie the statement has no effect).

@batfish-bot
Copy link
Copy Markdown

This change is Reviewable

@codecov
Copy link
Copy Markdown

codecov bot commented Mar 15, 2024

Codecov Report

Merging #8972 (2d73c10) into master (c943c1b) will increase coverage by 0.00%.
The diff coverage is n/a.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #8972   +/-   ##
=======================================
  Coverage   72.51%   72.51%           
=======================================
  Files        3323     3323           
  Lines      169630   169630           
  Branches    19914    19914           
=======================================
+ Hits       123004   123010    +6     
+ Misses      37462    37455    -7     
- Partials     9164     9165    +1     

see 3 files with indirect coverage changes

Copy link
Copy Markdown
Member

@dhalperi dhalperi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed all commit messages.
Reviewable status: 0 of 3 files reviewed, 1 unresolved discussion (waiting on @zergling-aws)


projects/batfish/src/main/java/org/batfish/grammar/flatjuniper/ConfigurationBuilder.java line 4929 at r1 (raw file):

    } catch (NullPointerException e) {
      warn(ctx, "Batfish does not support an empty description");
    }

The better way to do this would be to check for null.

if (ctx.description() == null) {
   warn...
} else {
    setDesc...
}

But I wonder -- why accept description missing vs just not parse it?

Code quote:

    try {
      String text = toString(ctx.description());
      _currentInterfaceOrRange.setDescription(text);
    } catch (NullPointerException e) {
      warn(ctx, "Batfish does not support an empty description");
    }

Copy link
Copy Markdown
Member

@dhalperi dhalperi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 4 of 4 files at r2, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @zergling-aws)

@dhalperi dhalperi merged commit ff994be into batfish:master Mar 18, 2024
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.

3 participants