From 3c8482acb32913f2401f9cde69cf1bcba7d47fe7 Mon Sep 17 00:00:00 2001 From: Robert Varga Date: Mon, 28 May 2018 18:49:34 +0200 Subject: [PATCH] Fix YangModelDependencyInfo handling of arguments Malformed YANG models could cause a NPE, let's be careful around them and report IAE instead. Change-Id: Idf31e2e383142e1283c978eccdf123faa7e40617 Signed-off-by: Robert Varga --- .../rfc7950/repo/ArgumentContextUtils.java | 4 --- .../rfc7950/repo/YangModelDependencyInfo.java | 36 ++++++++++--------- .../repo/YangModelDependencyInfoTest.java | 20 +++++++++++ .../malformed-import-rev.yang | 5 +++ .../depinfo-malformed/malformed-import.yang | 5 +++ .../depinfo-malformed/malformed-module.yang | 4 +++ .../depinfo-malformed/malformed-rev.yang | 6 ++++ 7 files changed, 60 insertions(+), 20 deletions(-) create mode 100644 yang/yang-parser-rfc7950/src/test/resources/depinfo-malformed/malformed-import-rev.yang create mode 100644 yang/yang-parser-rfc7950/src/test/resources/depinfo-malformed/malformed-import.yang create mode 100644 yang/yang-parser-rfc7950/src/test/resources/depinfo-malformed/malformed-module.yang create mode 100644 yang/yang-parser-rfc7950/src/test/resources/depinfo-malformed/malformed-rev.yang diff --git a/yang/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/repo/ArgumentContextUtils.java b/yang/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/repo/ArgumentContextUtils.java index b89661f0d9..6592a89697 100644 --- a/yang/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/repo/ArgumentContextUtils.java +++ b/yang/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/repo/ArgumentContextUtils.java @@ -30,10 +30,6 @@ final class ArgumentContextUtils { throw new UnsupportedOperationException(); } - static String stringFromStringContext(final ArgumentContext context, final StatementSourceReference ref) { - return stringFromStringContext(context, YangVersion.VERSION_1, ref); - } - static String stringFromStringContext(final ArgumentContext context, final YangVersion yangVersion, final StatementSourceReference ref) { final StringBuilder sb = new StringBuilder(); diff --git a/yang/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/repo/YangModelDependencyInfo.java b/yang/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/repo/YangModelDependencyInfo.java index c66cf73d1e..84d03bcfc8 100644 --- a/yang/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/repo/YangModelDependencyInfo.java +++ b/yang/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/repo/YangModelDependencyInfo.java @@ -22,11 +22,13 @@ import java.util.Optional; import java.util.Set; import javax.annotation.Nullable; import org.antlr.v4.runtime.ParserRuleContext; +import org.opendaylight.yangtools.antlrv4.code.gen.YangStatementParser.ArgumentContext; import org.opendaylight.yangtools.antlrv4.code.gen.YangStatementParser.StatementContext; import org.opendaylight.yangtools.concepts.SemVer; import org.opendaylight.yangtools.openconfig.model.api.OpenConfigStatements; import org.opendaylight.yangtools.yang.common.QName; import org.opendaylight.yangtools.yang.common.Revision; +import org.opendaylight.yangtools.yang.common.YangVersion; import org.opendaylight.yangtools.yang.model.api.ModuleImport; import org.opendaylight.yangtools.yang.model.api.YangStmtMapping; import org.opendaylight.yangtools.yang.model.parser.api.YangSyntaxErrorException; @@ -215,8 +217,7 @@ public abstract class YangModelDependencyInfo { private static YangModelDependencyInfo parseModuleContext(final StatementContext module, final SourceIdentifier source) { - final String name = ArgumentContextUtils.stringFromStringContext(module.argument(), getReference(source, - module)); + final String name = safeStringArgument(source, module, "module name"); final String latestRevision = getLatestRevision(module, source); final Optional semVer = Optional.ofNullable(findSemanticVersion(module, source)); final ImmutableSet imports = parseImports(module, source); @@ -230,9 +231,9 @@ public abstract class YangModelDependencyInfo { final Set result = new HashSet<>(); for (final StatementContext subStatementContext : module.statement()) { if (IMPORT.equals(subStatementContext.keyword().getText())) { + final String importedModuleName = safeStringArgument(source, subStatementContext, + "imported module name"); final String revisionDateStr = getRevisionDateString(subStatementContext, source); - final String importedModuleName = ArgumentContextUtils.stringFromStringContext( - subStatementContext.argument(), getReference(source, subStatementContext)); final Revision revisionDate = Revision.ofNullable(revisionDateStr).orElse(null); final SemVer importSemVer = findSemanticVersion(subStatementContext, source); result.add(new ModuleImportImpl(importedModuleName, revisionDate, importSemVer)); @@ -246,8 +247,7 @@ public abstract class YangModelDependencyInfo { for (final StatementContext subStatement : statement.statement()) { final String subStatementName = trimPrefix(subStatement.keyword().getText()); if (OPENCONFIG_VERSION.equals(subStatementName)) { - semVerString = ArgumentContextUtils.stringFromStringContext(subStatement.argument(), - getReference(source, subStatement)); + semVerString = safeStringArgument(source, subStatement, "version string"); break; } } @@ -271,8 +271,8 @@ public abstract class YangModelDependencyInfo { for (final StatementContext subStatementContext : module.statement()) { if (INCLUDE.equals(subStatementContext.keyword().getText())) { final String revisionDateStr = getRevisionDateString(subStatementContext, source); - final String IncludeModuleName = ArgumentContextUtils.stringFromStringContext( - subStatementContext.argument(), getReference(source, subStatementContext)); + final String IncludeModuleName = safeStringArgument(source, subStatementContext, + "included submodule name"); final Revision revisionDate = Revision.ofNullable(revisionDateStr).orElse(null); result.add(new ModuleImportImpl(IncludeModuleName, revisionDate)); } @@ -284,8 +284,7 @@ public abstract class YangModelDependencyInfo { String revisionDateStr = null; for (final StatementContext importSubStatement : importStatement.statement()) { if (REVISION_DATE.equals(importSubStatement.keyword().getText())) { - revisionDateStr = ArgumentContextUtils.stringFromStringContext(importSubStatement.argument(), - getReference(source, importSubStatement)); + revisionDateStr = safeStringArgument(source, importSubStatement, "imported module revision-date"); } } return revisionDateStr; @@ -295,8 +294,7 @@ public abstract class YangModelDependencyInfo { String latestRevision = null; for (final StatementContext subStatementContext : module.statement()) { if (REVISION.equals(subStatementContext.keyword().getText())) { - final String currentRevision = ArgumentContextUtils.stringFromStringContext( - subStatementContext.argument(), getReference(source, subStatementContext)); + final String currentRevision = safeStringArgument(source, subStatementContext, "revision date"); if (latestRevision == null || latestRevision.compareTo(currentRevision) < 0) { latestRevision = currentRevision; } @@ -307,8 +305,7 @@ public abstract class YangModelDependencyInfo { private static YangModelDependencyInfo parseSubmoduleContext(final StatementContext submodule, final SourceIdentifier source) { - final String name = ArgumentContextUtils.stringFromStringContext(submodule.argument(), - getReference(source, submodule)); + final String name = safeStringArgument(source, submodule, "submodule name"); final String belongsTo = parseBelongsTo(submodule, source); final String latestRevision = getLatestRevision(submodule, source); @@ -321,13 +318,20 @@ public abstract class YangModelDependencyInfo { private static String parseBelongsTo(final StatementContext submodule, final SourceIdentifier source) { for (final StatementContext subStatementContext : submodule.statement()) { if (BELONGS_TO.equals(subStatementContext.keyword().getText())) { - return ArgumentContextUtils.stringFromStringContext(subStatementContext.argument(), - getReference(source, subStatementContext)); + return safeStringArgument(source, subStatementContext, "belongs-to module name"); } } return null; } + private static String safeStringArgument(final SourceIdentifier source, final StatementContext stmt, + final String desc) { + final StatementSourceReference ref = getReference(source, stmt); + final ArgumentContext arg = stmt.argument(); + checkArgument(arg != null, "Missing %s at %s", desc, ref); + return ArgumentContextUtils.stringFromStringContext(arg, YangVersion.VERSION_1, ref); + } + private static StatementSourceReference getReference(final SourceIdentifier source, final StatementContext context) { return DeclarationInTextSource.atPosition(source.getName(), context.getStart().getLine(), diff --git a/yang/yang-parser-rfc7950/src/test/java/org/opendaylight/yangtools/yang/parser/rfc7950/repo/YangModelDependencyInfoTest.java b/yang/yang-parser-rfc7950/src/test/java/org/opendaylight/yangtools/yang/parser/rfc7950/repo/YangModelDependencyInfoTest.java index 2b07deb5cb..71fd9a670e 100644 --- a/yang/yang-parser-rfc7950/src/test/java/org/opendaylight/yangtools/yang/parser/rfc7950/repo/YangModelDependencyInfoTest.java +++ b/yang/yang-parser-rfc7950/src/test/java/org/opendaylight/yangtools/yang/parser/rfc7950/repo/YangModelDependencyInfoTest.java @@ -78,4 +78,24 @@ public class YangModelDependencyInfoTest { "/no-revision/module-without-revision.yang"); assertNotEquals("hashcode", 31, info.hashCode()); } + + @Test(expected = IllegalArgumentException.class) + public void testMalformedImport() throws IOException, YangSyntaxErrorException { + YangModelDependencyInfo.forResource(getClass(), "/depinfo-malformed/malformed-import.yang"); + } + + @Test(expected = IllegalArgumentException.class) + public void testMalformedImportRev() throws IOException, YangSyntaxErrorException { + YangModelDependencyInfo.forResource(getClass(), "/depinfo-malformed/malformed-import-rev.yang"); + } + + @Test(expected = IllegalArgumentException.class) + public void testMalformedModule() throws IOException, YangSyntaxErrorException { + YangModelDependencyInfo.forResource(getClass(), "/depinfo-malformed/malformed-module.yang"); + } + + @Test(expected = IllegalArgumentException.class) + public void testMalformedRev() throws IOException, YangSyntaxErrorException { + YangModelDependencyInfo.forResource(getClass(), "/depinfo-malformed/malformed-rev.yang"); + } } diff --git a/yang/yang-parser-rfc7950/src/test/resources/depinfo-malformed/malformed-import-rev.yang b/yang/yang-parser-rfc7950/src/test/resources/depinfo-malformed/malformed-import-rev.yang new file mode 100644 index 0000000000..64cf208fef --- /dev/null +++ b/yang/yang-parser-rfc7950/src/test/resources/depinfo-malformed/malformed-import-rev.yang @@ -0,0 +1,5 @@ +module malformed-import-rev { + namespace "urn:test:foo"; + prefix aug; + import foo { revision-date; prefix foo; } +} diff --git a/yang/yang-parser-rfc7950/src/test/resources/depinfo-malformed/malformed-import.yang b/yang/yang-parser-rfc7950/src/test/resources/depinfo-malformed/malformed-import.yang new file mode 100644 index 0000000000..709980edf5 --- /dev/null +++ b/yang/yang-parser-rfc7950/src/test/resources/depinfo-malformed/malformed-import.yang @@ -0,0 +1,5 @@ +module malformed-import { + namespace "urn:test:foo"; + prefix aug; + import; +} diff --git a/yang/yang-parser-rfc7950/src/test/resources/depinfo-malformed/malformed-module.yang b/yang/yang-parser-rfc7950/src/test/resources/depinfo-malformed/malformed-module.yang new file mode 100644 index 0000000000..35b3bbc513 --- /dev/null +++ b/yang/yang-parser-rfc7950/src/test/resources/depinfo-malformed/malformed-module.yang @@ -0,0 +1,4 @@ +module { + namespace "urn:test:foo"; + prefix aug; +} diff --git a/yang/yang-parser-rfc7950/src/test/resources/depinfo-malformed/malformed-rev.yang b/yang/yang-parser-rfc7950/src/test/resources/depinfo-malformed/malformed-rev.yang new file mode 100644 index 0000000000..cc6098d873 --- /dev/null +++ b/yang/yang-parser-rfc7950/src/test/resources/depinfo-malformed/malformed-rev.yang @@ -0,0 +1,6 @@ +module malformed-import { + namespace "urn:test:opendaylight-mdsal45-aug"; + prefix aug; + + revision; +} -- 2.36.6