From a425f18b89ba7b66d9fbc7e701c1c3bc4eb8c60e Mon Sep 17 00:00:00 2001 From: Lukas Sedlak Date: Mon, 22 Sep 2014 13:40:46 +0200 Subject: [PATCH] BUG-2022: String Type pattern parsing and resolving fix. Added fix during string type pattern restriction resolution in ParserListenerUtils. Now if pattern statement in yang model contains uncompilable string the warning is logged and pattern restriction is not among types restrictions. Each pattern is wrapped between "^"and "$" symbols. Modified exisiting tests and test resources for testing of transofrmation of incorrect pattern regular expressions. Change-Id: I86d6066b93e2f21b5c826729469228286a31965d Signed-off-by: Lukas Sedlak (cherry picked from commit 86fc7992c479c6a82859a8e26718f9adbf215c48) --- .../yang/parser/impl/ParserListenerUtils.java | 43 +++++++++-- .../yang/parser/impl/OrderingTest.java | 13 ++-- .../yang/parser/impl/TypesResolutionTest.java | 20 ++--- .../yang/parser/impl/YangParserTest.java | 74 ++++++++++++++++++- .../src/test/resources/model/bar.yang | 13 ++++ .../src/test/resources/model/foo.yang | 22 ++++++ 6 files changed, 159 insertions(+), 26 deletions(-) diff --git a/yang/yang-parser-impl/src/main/java/org/opendaylight/yangtools/yang/parser/impl/ParserListenerUtils.java b/yang/yang-parser-impl/src/main/java/org/opendaylight/yangtools/yang/parser/impl/ParserListenerUtils.java index 83b56314cb..493db98790 100644 --- a/yang/yang-parser-impl/src/main/java/org/opendaylight/yangtools/yang/parser/impl/ParserListenerUtils.java +++ b/yang/yang-parser-impl/src/main/java/org/opendaylight/yangtools/yang/parser/impl/ParserListenerUtils.java @@ -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 getPatternConstraint(final Type_body_stmtsContext ctx) { + private static List getPatternConstraint(final Type_body_stmtsContext ctx, final String moduleName) { List 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 description = Optional.absent(); Optional 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 rangeStatements = getRangeConstraints(typeBody, moduleBuilder.getName()); List lengthStatements = getLengthConstraints(typeBody, moduleBuilder.getName()); - List patternStatements = getPatternConstraint(typeBody); + List 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 lengthStatements = getLengthConstraints(typeBody, moduleName); - List patternStatements = getPatternConstraint(typeBody); + List patternStatements = getPatternConstraint(typeBody, moduleName); List rangeStatements = getRangeConstraints(typeBody, moduleName); TypeConstraints constraints = new TypeConstraints(moduleName, line); diff --git a/yang/yang-parser-impl/src/test/java/org/opendaylight/yangtools/yang/parser/impl/OrderingTest.java b/yang/yang-parser-impl/src/test/java/org/opendaylight/yangtools/yang/parser/impl/OrderingTest.java index bdf0989574..8aabb81fbd 100644 --- a/yang/yang-parser-impl/src/test/java/org/opendaylight/yangtools/yang/parser/impl/OrderingTest.java +++ b/yang/yang-parser-impl/src/test/java/org/opendaylight/yangtools/yang/parser/impl/OrderingTest.java @@ -30,8 +30,9 @@ public class OrderingTest { Set modules = TestUtils.loadModules(getClass().getResource("/model").toURI()); Module bar = TestUtils.findModule(modules, "bar"); Set> 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 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; diff --git a/yang/yang-parser-impl/src/test/java/org/opendaylight/yangtools/yang/parser/impl/TypesResolutionTest.java b/yang/yang-parser-impl/src/test/java/org/opendaylight/yangtools/yang/parser/impl/TypesResolutionTest.java index 2e7f932d3e..a2fa589a56 100644 --- a/yang/yang-parser-impl/src/test/java/org/opendaylight/yangtools/yang/parser/impl/TypesResolutionTest.java +++ b/yang/yang-parser-impl/src/test/java/org/opendaylight/yangtools/yang/parser/impl/TypesResolutionTest.java @@ -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 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 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 lengths = type.getLengthConstraints(); @@ -294,7 +294,7 @@ public class TypesResolutionTest { List 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(); diff --git a/yang/yang-parser-impl/src/test/java/org/opendaylight/yangtools/yang/parser/impl/YangParserTest.java b/yang/yang-parser-impl/src/test/java/org/opendaylight/yangtools/yang/parser/impl/YangParserTest.java index 2868497487..ebd6b6e524 100644 --- a/yang/yang-parser-impl/src/test/java/org/opendaylight/yangtools/yang/parser/impl/YangParserTest.java +++ b/yang/yang-parser-impl/src/test/java/org/opendaylight/yangtools/yang/parser/impl/YangParserTest.java @@ -239,7 +239,7 @@ public class YangParserTest { List 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 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 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 patterns = baseType2.getPatternConstraints(); assertEquals(1, patterns.size()); PatternConstraint pattern = patterns.iterator().next(); - assertEquals("[a-k]*", pattern.getRegularExpression()); + assertEquals("^[a-k]*$", pattern.getRegularExpression()); List baseType3Lengths = baseType2.getLengthConstraints(); assertEquals(1, baseType3Lengths.size()); length = baseType3Lengths.get(0); diff --git a/yang/yang-parser-impl/src/test/resources/model/bar.yang b/yang/yang-parser-impl/src/test/resources/model/bar.yang index 56de7b6f29..3501ff45a9 100644 --- a/yang/yang-parser-impl/src/test/resources/model/bar.yang +++ b/yang/yang-parser-impl/src/test/resources/model/bar.yang @@ -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; diff --git a/yang/yang-parser-impl/src/test/resources/model/foo.yang b/yang/yang-parser-impl/src/test/resources/model/foo.yang index e21fff1c4e..98ee7f2d91 100644 --- a/yang/yang-parser-impl/src/test/resources/model/foo.yang +++ b/yang/yang-parser-impl/src/test/resources/model/foo.yang @@ -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"; -- 2.36.6