Merge "BUG-2022: String Type pattern parsing and resolving fix."
authorTony Tkacik <ttkacik@cisco.com>
Thu, 23 Oct 2014 07:58:22 +0000 (07:58 +0000)
committerGerrit Code Review <gerrit@opendaylight.org>
Thu, 23 Oct 2014 07:58:22 +0000 (07:58 +0000)
yang/yang-parser-impl/src/main/java/org/opendaylight/yangtools/yang/parser/impl/ParserListenerUtils.java
yang/yang-parser-impl/src/test/java/org/opendaylight/yangtools/yang/parser/impl/OrderingTest.java
yang/yang-parser-impl/src/test/java/org/opendaylight/yangtools/yang/parser/impl/TypesResolutionTest.java
yang/yang-parser-impl/src/test/java/org/opendaylight/yangtools/yang/parser/impl/YangParserTest.java
yang/yang-parser-impl/src/test/resources/model/bar.yang
yang/yang-parser-impl/src/test/resources/model/foo.yang

index 83b56314cb93de7f7fbfd603e66d05b3966b5037..493db98790228763756264aef69cc54f752c3dc2 100644 (file)
@@ -19,6 +19,8 @@ import java.util.ArrayList;
 import java.util.Collections;
 import java.util.Iterator;
 import java.util.List;
+import java.util.regex.Pattern;
+import java.util.regex.PatternSyntaxException;
 import org.antlr.v4.runtime.ParserRuleContext;
 import org.antlr.v4.runtime.tree.ParseTree;
 import org.antlr.v4.runtime.tree.TerminalNode;
@@ -717,7 +719,7 @@ public final class ParserListenerUtils {
      *            type body
      * @return list of pattern constraints
      */
-    private static List<PatternConstraint> getPatternConstraint(final Type_body_stmtsContext ctx) {
+    private static List<PatternConstraint> getPatternConstraint(final Type_body_stmtsContext ctx, final String moduleName) {
         List<PatternConstraint> patterns = new ArrayList<>();
 
         for (int i = 0; i < ctx.getChildCount(); i++) {
@@ -726,7 +728,11 @@ public final class ParserListenerUtils {
                 for (int j = 0; j < stringRestrChild.getChildCount(); j++) {
                     ParseTree lengthChild = stringRestrChild.getChild(j);
                     if (lengthChild instanceof Pattern_stmtContext) {
-                        patterns.add(parsePatternConstraint((Pattern_stmtContext) lengthChild));
+                        final PatternConstraint constraint = parsePatternConstraint((Pattern_stmtContext) lengthChild,
+                            moduleName);
+                        if (constraint != null) {
+                            patterns.add(constraint);
+                        }
                     }
                 }
             }
@@ -741,7 +747,7 @@ public final class ParserListenerUtils {
      *            pattern context
      * @return PatternConstraint object
      */
-    private static PatternConstraint parsePatternConstraint(final Pattern_stmtContext ctx) {
+    private static PatternConstraint parsePatternConstraint(final Pattern_stmtContext ctx, final String moduleName) {
         Optional<String> description = Optional.absent();
         Optional<String> reference = Optional.absent();
         for (int i = 0; i < ctx.getChildCount(); i++) {
@@ -752,8 +758,31 @@ public final class ParserListenerUtils {
                 reference = Optional.of(stringFromNode(child));
             }
         }
-        String pattern = parsePatternString(ctx);
-        return BaseConstraints.newPatternConstraint(pattern, description, reference);
+        final String rawPattern = parsePatternString(ctx);
+        final String pattern = wrapPattern(rawPattern);
+        if (isValidPattern(pattern, ctx, moduleName)) {
+            return BaseConstraints.newPatternConstraint(pattern, description, reference);
+        }
+        return null;
+    }
+
+    private static String wrapPattern(String rawPattern) {
+        final StringBuilder wrapPatternBuilder = new StringBuilder(rawPattern.length() + 2);
+        wrapPatternBuilder.append('^');
+        wrapPatternBuilder.append(rawPattern);
+        wrapPatternBuilder.append('$');
+        return wrapPatternBuilder.toString();
+    }
+
+    private static boolean isValidPattern(final String pattern, final Pattern_stmtContext ctx, final String moduleName) {
+        try {
+            Pattern.compile(pattern);
+            return true;
+        } catch (PatternSyntaxException ex) {
+            LOG.warn("Unable to compile pattern defined in module {} at line {}. Error message: {}",
+                moduleName, ctx.getStart().getLine(), ex.getMessage());
+        }
+        return false;
     }
 
     /**
@@ -1057,7 +1086,7 @@ public final class ParserListenerUtils {
 
         List<RangeConstraint> rangeStatements = getRangeConstraints(typeBody, moduleBuilder.getName());
         List<LengthConstraint> lengthStatements = getLengthConstraints(typeBody, moduleBuilder.getName());
-        List<PatternConstraint> patternStatements = getPatternConstraint(typeBody);
+        List<PatternConstraint> patternStatements = getPatternConstraint(typeBody, moduleBuilder.getName());
         Integer fractionDigits = getFractionDigits(typeBody, moduleBuilder.getName());
 
         if (parent instanceof TypeDefinitionBuilder && !(parent instanceof UnionTypeBuilder)) {
@@ -1107,7 +1136,7 @@ public final class ParserListenerUtils {
 
         Integer fractionDigits = getFractionDigits(typeBody, moduleName);
         List<LengthConstraint> lengthStatements = getLengthConstraints(typeBody, moduleName);
-        List<PatternConstraint> patternStatements = getPatternConstraint(typeBody);
+        List<PatternConstraint> patternStatements = getPatternConstraint(typeBody, moduleName);
         List<RangeConstraint> rangeStatements = getRangeConstraints(typeBody, moduleName);
 
         TypeConstraints constraints = new TypeConstraints(moduleName, line);
index bdf0989574fb6928545b0c1e864c39545fa57006..8aabb81fbdcd2013f962bd09dc6597da96de938d 100644 (file)
@@ -30,8 +30,9 @@ public class OrderingTest {
         Set<Module> modules = TestUtils.loadModules(getClass().getResource("/model").toURI());
         Module bar = TestUtils.findModule(modules, "bar");
         Set<TypeDefinition<?>> typedefs = bar.getTypeDefinitions();
-        String[] expectedOrder = new String[] { "int32-ext1", "int32-ext2", "my-decimal-type", "my-union",
-                "my-union-ext", "nested-union2", "string-ext1", "string-ext2", "string-ext3", "string-ext4" };
+        String[] expectedOrder = new String[] { "int32-ext1", "int32-ext2", "invalid-string-pattern",
+            "multiple-pattern-string", "my-decimal-type", "my-union", "my-union-ext", "nested-union2",
+            "string-ext1", "string-ext2", "string-ext3", "string-ext4" };
         String[] actualOrder = new String[typedefs.size()];
 
         int i = 0;
@@ -73,9 +74,11 @@ public class OrderingTest {
         Module foo = TestUtils.findModule(modules, "foo");
 
         Collection<DataSchemaNode> childNodes = foo.getChildNodes();
-        String[] expectedOrder = new String[] { "int32-leaf", "string-leaf", "length-leaf", "decimal-leaf",
-                "decimal-leaf2", "ext", "union-leaf", "custom-union-leaf", "transfer", "datas", "mycont", "data",
-                "how", "address", "port", "addresses", "peer", "id", "foo-id","sub-ext", "sub-transfer", "sub-datas" };
+        String[] expectedOrder = new String[] { "int32-leaf", "string-leaf", "invalid-pattern-string-leaf",
+                "invalid-direct-string-pattern-def-leaf", "multiple-pattern-string-leaf",
+                "multiple-pattern-direct-string-def-leaf", "length-leaf", "decimal-leaf", "decimal-leaf2", "ext",
+                "union-leaf", "custom-union-leaf", "transfer", "datas", "mycont", "data", "how", "address", "port",
+                "addresses", "peer", "id", "foo-id","sub-ext", "sub-transfer", "sub-datas" };
         String[] actualOrder = new String[childNodes.size()];
 
         int i = 0;
index 2e7f932d3e500194d0d47db5c019b2c859cb3dba..a2fa589a564e9a130135edf0c6e9d1e2caf2abfb 100644 (file)
@@ -123,20 +123,20 @@ public class TypesResolutionTest {
 
         ExtendedType ipv4 = (ExtendedType) unionTypes.get(0);
         assertTrue(ipv4.getBaseType() instanceof StringTypeDefinition);
-        String expectedPattern = "(([0-9]|[1-9][0-9]|1[0-9][0-9]|2[0-4][0-9]|25[0-5])\\.){3}"
-                + "([0-9]|[1-9][0-9]|1[0-9][0-9]|2[0-4][0-9]|25[0-5])" + "(%[\\p{N}\\p{L}]+)?";
+        String expectedPattern = "^(([0-9]|[1-9][0-9]|1[0-9][0-9]|2[0-4][0-9]|25[0-5])\\.){3}"
+                + "([0-9]|[1-9][0-9]|1[0-9][0-9]|2[0-4][0-9]|25[0-5])" + "(%[\\p{N}\\p{L}]+)?$";
         assertEquals(expectedPattern, ipv4.getPatternConstraints().get(0).getRegularExpression());
 
         ExtendedType ipv6 = (ExtendedType) unionTypes.get(1);
         assertTrue(ipv6.getBaseType() instanceof StringTypeDefinition);
         List<PatternConstraint> ipv6Patterns = ipv6.getPatternConstraints();
-        expectedPattern = "((:|[0-9a-fA-F]{0,4}):)([0-9a-fA-F]{0,4}:){0,5}"
+        expectedPattern = "^((:|[0-9a-fA-F]{0,4}):)([0-9a-fA-F]{0,4}:){0,5}"
                 + "((([0-9a-fA-F]{0,4}:)?(:|[0-9a-fA-F]{0,4}))|" + "(((25[0-5]|2[0-4][0-9]|[01]?[0-9]?[0-9])\\.){3}"
-                + "(25[0-5]|2[0-4][0-9]|[01]?[0-9]?[0-9])))" + "(%[\\p{N}\\p{L}]+)?";
+                + "(25[0-5]|2[0-4][0-9]|[01]?[0-9]?[0-9])))" + "(%[\\p{N}\\p{L}]+)?$";
         assertEquals(expectedPattern, ipv6Patterns.get(0).getRegularExpression());
 
-        expectedPattern = "(([^:]+:){6}(([^:]+:[^:]+)|(.*\\..*)))|" + "((([^:]+:)*[^:]+)?::(([^:]+:)*[^:]+)?)"
-                + "(%.+)?";
+        expectedPattern = "^(([^:]+:){6}(([^:]+:[^:]+)|(.*\\..*)))|" + "((([^:]+:)*[^:]+)?::(([^:]+:)*[^:]+)?)"
+                + "(%.+)?$";
         assertEquals(expectedPattern, ipv6Patterns.get(1).getRegularExpression());
     }
 
@@ -148,8 +148,8 @@ public class TypesResolutionTest {
         assertTrue(type.getBaseType() instanceof StringTypeDefinition);
         List<PatternConstraint> patterns = type.getPatternConstraints();
         assertEquals(1, patterns.size());
-        String expectedPattern = "((([a-zA-Z0-9_]([a-zA-Z0-9\\-_]){0,61})?[a-zA-Z0-9]\\.)*"
-                + "([a-zA-Z0-9_]([a-zA-Z0-9\\-_]){0,61})?[a-zA-Z0-9]\\.?)" + "|\\.";
+        String expectedPattern = "^((([a-zA-Z0-9_]([a-zA-Z0-9\\-_]){0,61})?[a-zA-Z0-9]\\.)*"
+                + "([a-zA-Z0-9_]([a-zA-Z0-9\\-_]){0,61})?[a-zA-Z0-9]\\.?)" + "|\\.$";
         assertEquals(expectedPattern, patterns.get(0).getRegularExpression());
 
         List<LengthConstraint> lengths = type.getLengthConstraints();
@@ -294,7 +294,7 @@ public class TypesResolutionTest {
         List<PatternConstraint> patterns = testedType.getPatternConstraints();
         assertEquals(1, patterns.size());
         PatternConstraint pattern = patterns.get(0);
-        assertEquals("\\d*(\\.\\d*){1,127}", pattern.getRegularExpression());
+        assertEquals("^\\d*(\\.\\d*){1,127}$", pattern.getRegularExpression());
 
         QName testedTypeQName = testedType.getQName();
         assertEquals(URI.create("urn:ietf:params:xml:ns:yang:ietf-yang-types"), testedTypeQName.getNamespace());
@@ -306,7 +306,7 @@ public class TypesResolutionTest {
         assertEquals(1, patterns.size());
 
         pattern = patterns.get(0);
-        assertEquals("(([0-1](\\.[1-3]?[0-9]))|(2\\.(0|([1-9]\\d*))))(\\.(0|([1-9]\\d*)))*",
+        assertEquals("^(([0-1](\\.[1-3]?[0-9]))|(2\\.(0|([1-9]\\d*))))(\\.(0|([1-9]\\d*)))*$",
                 pattern.getRegularExpression());
 
         QName testedTypeBaseQName = testedTypeBase.getQName();
index 2868497487cdb70f6f46890c65c594cacf7dbb2a..ebd6b6e5246ec0a3b1db1b10c64256724c3e52e4 100644 (file)
@@ -239,7 +239,7 @@ public class YangParserTest {
         List<PatternConstraint> patterns = type.getPatternConstraints();
         assertEquals(1, patterns.size());
         PatternConstraint pattern = patterns.iterator().next();
-        assertEquals("[e-z]*", pattern.getRegularExpression());
+        assertEquals("^[e-z]*$", pattern.getRegularExpression());
         assertTrue(type.getLengthConstraints().isEmpty());
         assertTrue(type.getRangeConstraints().isEmpty());
 
@@ -253,7 +253,7 @@ public class YangParserTest {
         patterns = baseType1.getPatternConstraints();
         assertEquals(1, patterns.size());
         pattern = patterns.iterator().next();
-        assertEquals("[b-u]*", pattern.getRegularExpression());
+        assertEquals("^[b-u]*$", pattern.getRegularExpression());
         assertTrue(baseType1.getLengthConstraints().isEmpty());
         assertTrue(baseType1.getRangeConstraints().isEmpty());
 
@@ -282,7 +282,7 @@ public class YangParserTest {
         patterns = baseType3.getPatternConstraints();
         assertEquals(1, patterns.size());
         pattern = patterns.iterator().next();
-        assertEquals("[a-k]*", pattern.getRegularExpression());
+        assertEquals("^[a-k]*$", pattern.getRegularExpression());
         List<LengthConstraint> baseType3Lengths = baseType3.getLengthConstraints();
         assertEquals(1, baseType3Lengths.size());
         length = baseType3Lengths.get(0);
@@ -293,6 +293,72 @@ public class YangParserTest {
         assertTrue(baseType3.getBaseType() instanceof StringType);
     }
 
+    @Test
+    public void testTypedefInvalidPatternsResolving() {
+        Module foo = TestUtils.findModule(modules, "foo");
+        final LeafSchemaNode invalidPatternStringLeaf = (LeafSchemaNode) foo.getDataChildByName("invalid-pattern-string-leaf");
+        ExtendedType type = (ExtendedType) invalidPatternStringLeaf.getType();
+        QName typeQName = type.getQName();
+        assertEquals("invalid-string-pattern", typeQName.getLocalName());
+        assertEquals(barNS, typeQName.getNamespace());
+        assertEquals(barRev, typeQName.getRevision());
+        assertNull(type.getUnits());
+        assertNull(type.getDefaultValue());
+        List<PatternConstraint> patterns = type.getPatternConstraints();
+        assertTrue(patterns.isEmpty());
+
+        final LeafSchemaNode invalidDirectStringPatternDefLeaf = (LeafSchemaNode) foo.getDataChildByName("invalid-direct-string-pattern-def-leaf");
+        type = (ExtendedType) invalidDirectStringPatternDefLeaf.getType();
+        typeQName = type.getQName();
+        assertEquals("string", typeQName.getLocalName());
+        assertEquals(fooNS, typeQName.getNamespace());
+        assertEquals(fooRev, typeQName.getRevision());
+        assertNull(type.getUnits());
+        assertNull(type.getDefaultValue());
+        patterns = type.getPatternConstraints();
+        assertTrue(patterns.isEmpty());
+
+        final LeafSchemaNode multiplePatternStringLeaf = (LeafSchemaNode) foo.getDataChildByName("multiple-pattern-string-leaf");
+        type = (ExtendedType) multiplePatternStringLeaf.getType();
+        typeQName = type.getQName();
+        assertEquals("multiple-pattern-string", typeQName.getLocalName());
+        assertEquals(barNS, typeQName.getNamespace());
+        assertEquals(barRev, typeQName.getRevision());
+        assertNull(type.getUnits());
+        assertNull(type.getDefaultValue());
+        patterns = type.getPatternConstraints();
+        assertTrue(!patterns.isEmpty());
+        assertEquals(1, patterns.size());
+        PatternConstraint pattern = patterns.iterator().next();
+        assertEquals("^[e-z]*$", pattern.getRegularExpression());
+        assertTrue(type.getLengthConstraints().isEmpty());
+        assertTrue(type.getRangeConstraints().isEmpty());
+
+        final LeafSchemaNode multiplePatternDirectStringDefLeaf = (LeafSchemaNode) foo.getDataChildByName("multiple-pattern-direct-string-def-leaf");
+        type = (ExtendedType) multiplePatternDirectStringDefLeaf.getType();
+        typeQName = type.getQName();
+        assertEquals("string", typeQName.getLocalName());
+        assertEquals(fooNS, typeQName.getNamespace());
+        assertEquals(fooRev, typeQName.getRevision());
+        assertNull(type.getUnits());
+        assertNull(type.getDefaultValue());
+        patterns = type.getPatternConstraints();
+        assertTrue(!patterns.isEmpty());
+        assertEquals(2, patterns.size());
+
+        boolean isEZPattern = false;
+        boolean isADPattern = false;
+        for (final PatternConstraint patternConstraint : patterns) {
+            if (patternConstraint.getRegularExpression().equals("^[e-z]*$")) {
+                isEZPattern = true;
+            } else if (patternConstraint.getRegularExpression().equals("^[a-d]*$")) {
+                isADPattern = true;
+            }
+        }
+        assertTrue(isEZPattern);
+        assertTrue( isADPattern);
+    }
+
     @Test
     public void testTypedefLengthsResolving() {
         Module foo = TestUtils.findModule(modules, "foo");
@@ -339,7 +405,7 @@ public class YangParserTest {
         List<PatternConstraint> patterns = baseType2.getPatternConstraints();
         assertEquals(1, patterns.size());
         PatternConstraint pattern = patterns.iterator().next();
-        assertEquals("[a-k]*", pattern.getRegularExpression());
+        assertEquals("^[a-k]*$", pattern.getRegularExpression());
         List<LengthConstraint> baseType3Lengths = baseType2.getLengthConstraints();
         assertEquals(1, baseType3Lengths.size());
         length = baseType3Lengths.get(0);
index 56de7b6f29c45c19fa06630c9a06297669a7558d..3501ff45a9e9e2bd7c15e08df2d87f4c9ac0bfbc 100644 (file)
@@ -50,6 +50,19 @@ module bar {
         }
     }
 
+    typedef invalid-string-pattern {
+        type string {
+            pattern "[[A-1*-%22!^^}";
+        }
+    }
+
+    typedef multiple-pattern-string {
+        type string {
+            pattern "[[A-1*-%22!^^}";
+            pattern "[e-z]*";
+        }
+    }
+
     typedef my-decimal-type {
         type decimal64 {
             fraction-digits 6;
index e21fff1c4e0ced75a13c100073035d6c008723f5..98ee7f2d91122a68f5db6c312c9655a113a7ef30 100644 (file)
@@ -36,6 +36,28 @@ module foo {
         type br:string-ext4;
     }
 
+    leaf invalid-pattern-string-leaf {
+        type br:invalid-string-pattern;
+    }
+
+    leaf invalid-direct-string-pattern-def-leaf {
+        type string {
+            pattern "[[A-1*-%22!^^}";
+        }
+    }
+
+    leaf multiple-pattern-string-leaf {
+        type br:multiple-pattern-string;
+    }
+
+    leaf multiple-pattern-direct-string-def-leaf {
+        type string {
+            pattern "[e-z]*";
+            pattern "[[A-1*-%22!^^}";
+            pattern "[a-d]*";
+        }
+    }
+
     leaf length-leaf {
         type br:string-ext2 {
             length "7..max";