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 <robert.varga@pantheon.tech>

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 b89661f0d9a265e104e4e50934f38c4f934d7439..6592a89697cce951cb14e43225f12f59b72650c5 100644 (file)
--- 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 c66cf73d1e841810c58194d85de468e98eb9d259..84d03bcfc8c0194ebada010e3a2a769cb88f6447 100644 (file)
--- 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> semVer = Optional.ofNullable(findSemanticVersion(module, source));
         final ImmutableSet<ModuleImport> imports = parseImports(module, source);
@@ -230,9 +231,9 @@ public abstract class YangModelDependencyInfo {
         final Set<ModuleImport> 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 2b07deb5cbb87266f2c4afb631c347fea3a23b02..71fd9a670e83b1a51f9d908d601361473feb657b 100644 (file)
--- 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 (file)
index 0000000..64cf208
--- /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 (file)
index 0000000..709980e
--- /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 (file)
index 0000000..35b3bbc
--- /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 (file)
index 0000000..cc6098d
--- /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;
+}