Bug 6180 - Parser: Backslash double-quote in double-quoted string not recognized 90/44490/3
authorVratko Polak <vrpolak@cisco.com>
Mon, 22 Aug 2016 13:23:19 +0000 (15:23 +0200)
committerRobert Varga <nite@hq.sk>
Tue, 6 Sep 2016 10:22:12 +0000 (10:22 +0000)
This is a simplistic cherry-pick from Boron:

Statement parser removes all double or single quotes from strings,
what is incorrect. This fix removes only first and last quotes if they are
present. All other quotes in the string should be escaped already, since the
ANTLR parser recognizes only correctly enclosed strings.

Further, substitution of backslash-escaped characters in
double-quoted strings is done.

Change-Id: I2f596c73be05178dc5cbebaed12e25e428454219
Signed-off-by: Peter Kajsa <pkajsa@cisco.com>
Signed-off-by: Vratko Polak <vrpolak@cisco.com>
yang/yang-parser-impl/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/rfc6020/Utils.java
yang/yang-parser-impl/src/test/java/org/opendaylight/yangtools/yang/stmt/Bug6180Test.java [new file with mode: 0644]
yang/yang-parser-impl/src/test/resources/bugs/bug6180/double-quotes-single-inside.yang [new file with mode: 0644]
yang/yang-parser-impl/src/test/resources/bugs/bug6180/double-quotes.yang [new file with mode: 0644]
yang/yang-parser-impl/src/test/resources/bugs/bug6180/single-quotes.yang [new file with mode: 0644]
yang/yang-parser-impl/src/test/resources/bugs/bug6180/string-test.yang [new file with mode: 0644]

index c4b01e1f7a42a32c4b53f2d5dddb8c5dffb76f0c..442c7fdcfa9207ba4a36af78ed017def56546b9c 100644 (file)
@@ -69,8 +69,6 @@ import org.slf4j.LoggerFactory;
 public final class Utils {
     private static final int UNICODE_SCRIPT_FIX_COUNTER = 30;
     private static final Logger LOG = LoggerFactory.getLogger(Utils.class);
-    private static final CharMatcher DOUBLE_QUOTE_MATCHER = CharMatcher.is('"');
-    private static final CharMatcher SINGLE_QUOTE_MATCHER = CharMatcher.is('\'');
     private static final CharMatcher LEFT_PARENTHESIS_MATCHER = CharMatcher.is('(');
     private static final CharMatcher RIGHT_PARENTHESIS_MATCHER = CharMatcher.is(')');
     private static final CharMatcher AMPERSAND_MATCHER = CharMatcher.is('&');
@@ -292,8 +290,8 @@ public final class Utils {
 
     private static final Map<String, Deviate> KEYWORD_TO_DEVIATE_MAP;
     static {
-        Builder<String, Deviate> keywordToDeviateMapBuilder = ImmutableMap.builder();
-        for (Deviate deviate : Deviation.Deviate.values()) {
+        final Builder<String, Deviate> keywordToDeviateMapBuilder = ImmutableMap.builder();
+        for (final Deviate deviate : Deviation.Deviate.values()) {
             keywordToDeviateMapBuilder.put(deviate.getKeyword(), deviate);
         }
         KEYWORD_TO_DEVIATE_MAP = keywordToDeviateMapBuilder.build();
@@ -320,7 +318,7 @@ public final class Utils {
 
     public static Collection<SchemaNodeIdentifier.Relative> transformKeysStringToKeyNodes(final StmtContext<?, ?, ?> ctx,
             final String value) {
-        List<String> keyTokens = SPACE_SPLITTER.splitToList(value);
+        final List<String> keyTokens = SPACE_SPLITTER.splitToList(value);
 
         // to detect if key contains duplicates
         if ((new HashSet<>(keyTokens)).size() < keyTokens.size()) {
@@ -328,11 +326,11 @@ public final class Utils {
             throw new SourceException(ctx.getStatementSourceReference(), "Duplicate value in list key: %s", value);
         }
 
-        Set<SchemaNodeIdentifier.Relative> keyNodes = new HashSet<>();
+        final Set<SchemaNodeIdentifier.Relative> keyNodes = new HashSet<>();
 
-        for (String keyToken : keyTokens) {
+        for (final String keyToken : keyTokens) {
 
-            SchemaNodeIdentifier.Relative keyNode = (Relative) SchemaNodeIdentifier.Relative.create(false,
+            final SchemaNodeIdentifier.Relative keyNode = (Relative) SchemaNodeIdentifier.Relative.create(false,
                     Utils.qNameFromArgument(ctx, keyToken));
             keyNodes.add(keyNode);
         }
@@ -352,7 +350,7 @@ public final class Utils {
         try {
             // TODO: we could capture the result and expose its 'evaluate' method
             xPath.compile(trimmed);
-        } catch (XPathExpressionException e) {
+        } catch (final XPathExpressionException e) {
             LOG.warn("Argument \"{}\" is not valid XPath string at \"{}\"", path, ctx.getStatementSourceReference(), e);
         }
 
@@ -360,11 +358,11 @@ public final class Utils {
     }
 
     public static QName trimPrefix(final QName identifier) {
-        String prefixedLocalName = identifier.getLocalName();
-        String[] namesParts = prefixedLocalName.split(":");
+        final String prefixedLocalName = identifier.getLocalName();
+        final String[] namesParts = prefixedLocalName.split(":");
 
         if (namesParts.length == 2) {
-            String localName = namesParts[1];
+            final String localName = namesParts[1];
             return QName.create(identifier.getModule(), localName);
         }
 
@@ -388,12 +386,12 @@ public final class Utils {
         if (stmtDef.get(identifier) != null) {
             return stmtDef.get(identifier).getStatementName();
         } else {
-            String prefixedLocalName = identifier.getLocalName();
-            String[] namesParts = prefixedLocalName.split(":");
+            final String prefixedLocalName = identifier.getLocalName();
+            final String[] namesParts = prefixedLocalName.split(":");
 
             if (namesParts.length == 2) {
-                String prefix = namesParts[0];
-                String localName = namesParts[1];
+                final String prefix = namesParts[0];
+                final String localName = namesParts[1];
                 if (prefixes != null && prefixes.get(prefix) != null
                         && stmtDef.get(QName.create(prefixes.get(prefix), localName)) != null) {
                     return QName.create(prefixes.get(prefix), localName);
@@ -406,11 +404,11 @@ public final class Utils {
     static SchemaNodeIdentifier nodeIdentifierFromPath(final StmtContext<?, ?, ?> ctx, final String path) {
         // FIXME: is the path trimming really necessary??
         final List<QName> qNames = new ArrayList<>();
-        for (String nodeName : SLASH_SPLITTER.split(trimSingleLastSlashFromXPath(path))) {
+        for (final String nodeName : SLASH_SPLITTER.split(trimSingleLastSlashFromXPath(path))) {
             try {
                 final QName qName = Utils.qNameFromArgument(ctx, nodeName);
                 qNames.add(qName);
-            } catch (Exception e) {
+            } catch (final Exception e) {
                 throw new IllegalArgumentException(
                     String.format("Failed to parse node '%s' in path '%s'", nodeName, path), e);
             }
@@ -420,24 +418,32 @@ public final class Utils {
     }
 
     public static String stringFromStringContext(final YangStatementParser.ArgumentContext context) {
-        StringBuilder sb = new StringBuilder();
+        final StringBuilder sb = new StringBuilder();
         List<TerminalNode> strings = context.STRING();
         if (strings.isEmpty()) {
             strings = Arrays.asList(context.IDENTIFIER());
         }
-        for (TerminalNode stringNode : strings) {
+        for (final TerminalNode stringNode : strings) {
             final String str = stringNode.getText();
-            char firstChar = str.charAt(0);
-            final CharMatcher quoteMatcher;
-            if (SINGLE_QUOTE_MATCHER.matches(firstChar)) {
-                quoteMatcher = SINGLE_QUOTE_MATCHER;
-            } else if (DOUBLE_QUOTE_MATCHER.matches(firstChar)) {
-                quoteMatcher = DOUBLE_QUOTE_MATCHER;
+            final char firstChar = str.charAt(0);
+            final char lastChar = str.charAt(str.length() - 1);
+            if (firstChar == '"' && lastChar == '"') {
+                final String innerStr = str.substring(1, str.length() - 1);
+                /*
+                 * Unescape escaped double quotes, tabs, new line and backslash
+                 * in the inner string and trim the result.
+                 */
+                sb.append(innerStr.replace("\\\"", "\"").replace("\\\\", "\\").replace("\\n", "\n")
+                        .replace("\\t", "\t"));
+            } else if (firstChar == '\'' && lastChar == '\'') {
+                /*
+                 * According to RFC6020 a single quote character cannot occur in
+                 * a single-quoted string, even when preceded by a backslash.
+                 */
+                sb.append(str.substring(1, str.length() - 1));
             } else {
                 sb.append(str);
-                continue;
             }
-            sb.append(quoteMatcher.removeFrom(str.substring(1, str.length() - 1)));
         }
         return sb.toString();
     }
@@ -451,7 +457,7 @@ public final class Utils {
         QNameModule qNameModule = null;
         String localName = null;
 
-        String[] namesParts = value.split(":");
+        final String[] namesParts = value.split(":");
         switch (namesParts.length) {
         case 1:
             localName = namesParts[0];
@@ -495,7 +501,7 @@ public final class Utils {
         final QNameModule qNameModule = ctx.getFromNamespace(ModuleIdentifierToModuleQName.class, modId);
 
         if (qNameModule == null && StmtContextUtils.producesDeclared(ctx.getRoot(), SubmoduleStatement.class)) {
-            String moduleName = ctx.getRoot().getFromNamespace(BelongsToPrefixToModuleName.class, prefix);
+            final String moduleName = ctx.getRoot().getFromNamespace(BelongsToPrefixToModuleName.class, prefix);
             return ctx.getFromNamespace(ModuleNameToModuleQName.class, moduleName);
         }
         return qNameModule;
@@ -559,7 +565,7 @@ public final class Utils {
 
     public static Date getLatestRevision(final Iterable<? extends StmtContext<?, ?, ?>> subStmts) {
         Date revision = null;
-        for (StmtContext<?, ?, ?> subStmt : subStmts) {
+        for (final StmtContext<?, ?, ?> subStmt : subStmts) {
             if (subStmt.getPublicDefinition().getDeclaredRepresentationClass().isAssignableFrom(RevisionStatement
                     .class)) {
                 if (revision == null && subStmt.getStatementArgument() != null) {
@@ -601,7 +607,7 @@ public final class Utils {
             try {
                 Pattern.compile(rawPattern);
                 return rawPattern;
-            } catch(PatternSyntaxException ex) {
+            } catch(final PatternSyntaxException ex) {
                 LOG.debug("Invalid regex pattern syntax in: {}", rawPattern, ex);
                 if (ex.getMessage().contains("Unknown character script name")) {
                     rawPattern = fixUnknownScripts(ex.getMessage(), rawPattern);
@@ -617,25 +623,25 @@ public final class Utils {
 
     private static String fixUnknownScripts(final String exMessage, final String rawPattern) {
         StringBuilder result = new StringBuilder(rawPattern);
-        Matcher matcher = BETWEEN_CURLY_BRACES_PATTERN.matcher(exMessage);
+        final Matcher matcher = BETWEEN_CURLY_BRACES_PATTERN.matcher(exMessage);
         if (matcher.find()) {
-            String capturedGroup = matcher.group(1);
+            final String capturedGroup = matcher.group(1);
             if (JAVA_UNICODE_BLOCKS.contains(capturedGroup)) {
-                int idx = rawPattern.indexOf("Is" + capturedGroup);
+                final int idx = rawPattern.indexOf("Is" + capturedGroup);
                 result = result.replace(idx, idx + 2, "In");
             }
         }
         return result.toString();
     }
 
-    public static boolean belongsToTheSameModule(QName targetStmtQName, QName sourceStmtQName) {
+    public static boolean belongsToTheSameModule(final QName targetStmtQName, final QName sourceStmtQName) {
         if (targetStmtQName.getModule().equals(sourceStmtQName.getModule())) {
             return true;
         }
         return false;
     }
 
-    public static boolean isPresenceContainer(StatementContextBase<?, ?, ?> targetCtx) {
+    public static boolean isPresenceContainer(final StatementContextBase<?, ?, ?> targetCtx) {
         if (!targetCtx.getPublicDefinition().equals(Rfc6020Mapping.CONTAINER)) {
             return false;
         }
diff --git a/yang/yang-parser-impl/src/test/java/org/opendaylight/yangtools/yang/stmt/Bug6180Test.java b/yang/yang-parser-impl/src/test/java/org/opendaylight/yangtools/yang/stmt/Bug6180Test.java
new file mode 100644 (file)
index 0000000..4f7b2ca
--- /dev/null
@@ -0,0 +1,110 @@
+/*
+ * Copyright (c) 2016 Cisco Systems, Inc. and others.  All rights reserved.
+ *
+ * This program and the accompanying materials are made available under the
+ * terms of the Eclipse Public License v1.0 which accompanies this distribution,
+ * and is available at http://www.eclipse.org/legal/epl-v10.html
+ */
+
+package org.opendaylight.yangtools.yang.stmt;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertTrue;
+
+import java.io.File;
+import java.io.FileNotFoundException;
+import java.net.URISyntaxException;
+import java.util.List;
+import java.util.Set;
+import java.util.regex.Pattern;
+import org.junit.Test;
+import org.opendaylight.yangtools.yang.common.QName;
+import org.opendaylight.yangtools.yang.model.api.ContainerSchemaNode;
+import org.opendaylight.yangtools.yang.model.api.DataSchemaNode;
+import org.opendaylight.yangtools.yang.model.api.Module;
+import org.opendaylight.yangtools.yang.model.api.RevisionAwareXPath;
+import org.opendaylight.yangtools.yang.model.api.SchemaContext;
+import org.opendaylight.yangtools.yang.model.api.TypeDefinition;
+import org.opendaylight.yangtools.yang.model.api.type.PatternConstraint;
+import org.opendaylight.yangtools.yang.model.api.type.StringTypeDefinition;
+import org.opendaylight.yangtools.yang.parser.spi.meta.ReactorException;
+import org.opendaylight.yangtools.yang.parser.spi.source.SourceException;
+import org.opendaylight.yangtools.yang.stmt.test.StmtTestUtils;
+
+public class Bug6180Test {
+
+    @Test
+    public void stringTest() throws ReactorException, SourceException, FileNotFoundException, URISyntaxException {
+        final SchemaContext schemaContext = StmtTestUtils.parseYangSources(new File(getClass().getResource(
+                "/bugs/bug6180/string-test.yang").toURI()));
+        assertNotNull(schemaContext);
+        assertEquals(1, schemaContext.getModules().size());
+        final Module module = schemaContext.getModules().iterator().next();
+        assertEquals("    1. this text contains \"string enclosed in double quotes\" and"+
+                " special characters: \\,\n,\t          2. this text contains \"string enclosed in double quotes\" and"+
+                " special characters: \\,\n,\n,                     3. this text contains \"string enclosed in double quotes\" and"+
+                " special characters: \\,\n,\t      ", module.getDescription());
+    }
+
+    @Test
+    public void doubleQuotesTest() throws ReactorException, SourceException, FileNotFoundException, URISyntaxException {
+        final SchemaContext schemaContext = StmtTestUtils.parseYangSources(new File(getClass().getResource(
+                "/bugs/bug6180/double-quotes.yang").toURI()));
+        assertNotNull(schemaContext);
+        verifyDoubleQuotesExpression(schemaContext);
+    }
+
+    @Test
+    public void doubleQuotesSinbleInsideTest() throws ReactorException, SourceException, FileNotFoundException,
+            URISyntaxException {
+        final SchemaContext schemaContext = StmtTestUtils.parseYangSources(new File(getClass().getResource(
+                "/bugs/bug6180/double-quotes-single-inside.yang").toURI()));
+        assertNotNull(schemaContext);
+        verifySingleQuotesExpression(schemaContext);
+    }
+
+    @Test
+    public void singleQuotesTest() throws ReactorException, SourceException, FileNotFoundException, URISyntaxException {
+        final SchemaContext schemaContext = StmtTestUtils.parseYangSources(new File(getClass().getResource(
+                "/bugs/bug6180/single-quotes.yang").toURI()));
+        assertNotNull(schemaContext);
+        verifyDoubleQuotesExpression(schemaContext);
+    }
+
+    private static void verifyDoubleQuotesExpression(final SchemaContext schemaContext) {
+        final DataSchemaNode dataNodeBar = schemaContext.getDataChildByName(QName.create("foo", "2016-07-11", "bar"));
+        assertTrue(dataNodeBar instanceof ContainerSchemaNode);
+        final ContainerSchemaNode bar = (ContainerSchemaNode) dataNodeBar;
+        final RevisionAwareXPath whenCondition = bar.getConstraints().getWhenCondition();
+        assertEquals("/foo != \"bar\"", whenCondition.toString());
+
+        final Set<TypeDefinition<?>> typeDefinitions = schemaContext.getTypeDefinitions();
+        assertEquals(1, typeDefinitions.size());
+        final TypeDefinition<?> type = typeDefinitions.iterator().next();
+        assertTrue(type instanceof StringTypeDefinition);
+        final List<PatternConstraint> patternConstraints = ((StringTypeDefinition) type).getPatternConstraints();
+        assertEquals(1, patternConstraints.size());
+        final PatternConstraint pattern = patternConstraints.iterator().next();
+        assertEquals("^\".*\"$", pattern.getRegularExpression());
+        assertTrue(Pattern.compile(pattern.getRegularExpression()).matcher("\"enclosed string in quotes\"").matches());
+    }
+
+    private static void verifySingleQuotesExpression(final SchemaContext schemaContext) {
+        final DataSchemaNode dataNodeBar = schemaContext.getDataChildByName(QName.create("foo", "2016-07-11", "bar"));
+        assertTrue(dataNodeBar instanceof ContainerSchemaNode);
+        final ContainerSchemaNode bar = (ContainerSchemaNode) dataNodeBar;
+        final RevisionAwareXPath whenCondition = bar.getConstraints().getWhenCondition();
+        assertEquals("/foo != 'bar'", whenCondition.toString());
+
+        final Set<TypeDefinition<?>> typeDefinitions = schemaContext.getTypeDefinitions();
+        assertEquals(1, typeDefinitions.size());
+        final TypeDefinition<?> type = typeDefinitions.iterator().next();
+        assertTrue(type instanceof StringTypeDefinition);
+        final List<PatternConstraint> patternConstraints = ((StringTypeDefinition) type).getPatternConstraints();
+        assertEquals(1, patternConstraints.size());
+        final PatternConstraint pattern = patternConstraints.iterator().next();
+        assertEquals("^'.*'$", pattern.getRegularExpression());
+        assertTrue(Pattern.compile(pattern.getRegularExpression()).matcher("'enclosed string in quotes'").matches());
+    }
+}
\ No newline at end of file
diff --git a/yang/yang-parser-impl/src/test/resources/bugs/bug6180/double-quotes-single-inside.yang b/yang/yang-parser-impl/src/test/resources/bugs/bug6180/double-quotes-single-inside.yang
new file mode 100644 (file)
index 0000000..6745046
--- /dev/null
@@ -0,0 +1,19 @@
+module double-quotes {
+    namespace "foo";
+    prefix "foo";
+    revision 2016-07-11;
+
+    typedef double-quoted {
+        type string {
+            pattern "'.*'";
+        }
+    }
+
+    leaf foo {
+        type string;
+    }
+
+    container bar {
+        when "/foo != 'bar'";
+    }
+}
diff --git a/yang/yang-parser-impl/src/test/resources/bugs/bug6180/double-quotes.yang b/yang/yang-parser-impl/src/test/resources/bugs/bug6180/double-quotes.yang
new file mode 100644 (file)
index 0000000..030bb6e
--- /dev/null
@@ -0,0 +1,19 @@
+module double-quotes {
+    namespace "foo";
+    prefix "foo";
+    revision 2016-07-11;
+
+    typedef double-quoted {
+        type string {
+            pattern "\".*\"";
+        }
+    }
+
+    leaf foo {
+        type string;
+    }
+
+    container bar {
+        when "/foo != \"bar\"";
+    }
+}
diff --git a/yang/yang-parser-impl/src/test/resources/bugs/bug6180/single-quotes.yang b/yang/yang-parser-impl/src/test/resources/bugs/bug6180/single-quotes.yang
new file mode 100644 (file)
index 0000000..3f7ddf2
--- /dev/null
@@ -0,0 +1,18 @@
+module single-quotes {
+    namespace "foo";
+    prefix "foo";
+    revision 2016-07-11;
+
+    typedef double-quoted {
+        type string {
+            pattern '".*"';
+        }
+    }
+
+    leaf foo {
+        type string;
+    }
+    container bar {
+        when '/foo != "bar"';
+    }
+}
diff --git a/yang/yang-parser-impl/src/test/resources/bugs/bug6180/string-test.yang b/yang/yang-parser-impl/src/test/resources/bugs/bug6180/string-test.yang
new file mode 100644 (file)
index 0000000..ae1e15a
--- /dev/null
@@ -0,0 +1,8 @@
+module string-test {
+    namespace "test";
+    prefix test;
+
+    description "    1. this text contains \"string enclosed in double quotes\" and special characters: \\,\n,\t      " +
+                "    2. this text contains \"string enclosed in double quotes\" and special characters: \\,\n,
+,                     3. this text contains \"string enclosed in double quotes\" and special characters: \\,\n,\t      ";
+}