From b70d4c1fc9a661a04aa616d09339168879dfdbf2 Mon Sep 17 00:00:00 2001 From: Preben Ingvaldsen Date: Tue, 24 Mar 2015 10:13:23 -0700 Subject: [PATCH] Update single value parsing Update the single value parsing method to throw an exception when there is leading or trailing whitespace. Modify concatenation parsing in the ConfigDocumentParser to put back any trailing whitespace. Add a hashCode method for AbstractConfigNodes. --- .../com/typesafe/config/ConfigDocument.java | 3 +- .../config/impl/AbstractConfigNode.java | 5 ++ .../config/impl/ConfigDocumentParser.java | 30 ++++++++--- .../impl/ConfigDocumentParserTest.scala | 54 +++++++++++++++---- .../config/impl/ConfigDocumentTest.scala | 2 +- 5 files changed, 75 insertions(+), 19 deletions(-) diff --git a/config/src/main/java/com/typesafe/config/ConfigDocument.java b/config/src/main/java/com/typesafe/config/ConfigDocument.java index 4524e8e2..a5261ae4 100644 --- a/config/src/main/java/com/typesafe/config/ConfigDocument.java +++ b/config/src/main/java/com/typesafe/config/ConfigDocument.java @@ -27,7 +27,8 @@ public interface ConfigDocument { * @param newValue the value to set at the desired path, represented as a string. This * string will be parsed into a ConfigNode using the same options used to * parse the entire document, and the text will be inserted - * as-is into the document, with leading and trailing whitespace removed. + * as-is into the document. Leading and trailing comments, whitespace, or + * newlines are not allowed, and if present an exception will be thrown. * If a concatenation is passed in for newValue but the document was parsed * with JSON, the first value in the concatenation will be parsed and inserted * into the ConfigDocument. diff --git a/config/src/main/java/com/typesafe/config/impl/AbstractConfigNode.java b/config/src/main/java/com/typesafe/config/impl/AbstractConfigNode.java index 01f9d196..01ee45ae 100644 --- a/config/src/main/java/com/typesafe/config/impl/AbstractConfigNode.java +++ b/config/src/main/java/com/typesafe/config/impl/AbstractConfigNode.java @@ -21,4 +21,9 @@ abstract class AbstractConfigNode implements ConfigNode { final public boolean equals(Object other) { return other instanceof AbstractConfigNode && render().equals(((AbstractConfigNode)other).render()); } + + @Override + final public int hashCode() { + return render().hashCode(); + } } diff --git a/config/src/main/java/com/typesafe/config/impl/ConfigDocumentParser.java b/config/src/main/java/com/typesafe/config/impl/ConfigDocumentParser.java index 06339236..d2fa760e 100644 --- a/config/src/main/java/com/typesafe/config/impl/ConfigDocumentParser.java +++ b/config/src/main/java/com/typesafe/config/impl/ConfigDocumentParser.java @@ -139,7 +139,7 @@ final class ConfigDocumentParser { Token t = nextTokenIgnoringWhitespace(nodes); while (true) { AbstractConfigNodeValue v = null; - if (Tokens.isIgnoredWhitespace(t) || isUnquotedWhitespace(t)) { + if (Tokens.isIgnoredWhitespace(t)) { values.add(new ConfigNodeSingleToken(t)); t = nextToken(); continue; @@ -179,6 +179,16 @@ final class ConfigDocumentParser { return value; } + // Put back any trailing whitespace, as the parent object is responsible for tracking + // any leading/trailing whitespace + for (int i = values.size() - 1; i >= 0; i--) { + if (values.get(i) instanceof ConfigNodeSingleToken) { + putBack((new ArrayList(values.get(i).tokens())).get(0)); + values.remove(i); + } else { + break; + } + } return new ConfigNodeConcatenation(values); } @@ -637,8 +647,8 @@ final class ConfigDocumentParser { } t = nextToken(); - while (Tokens.isIgnoredWhitespace(t) || Tokens.isNewline(t) || isUnquotedWhitespace(t)) { - t = nextToken(); + if (Tokens.isIgnoredWhitespace(t) || Tokens.isNewline(t) || isUnquotedWhitespace(t) || Tokens.isComment(t)) { + throw parseError("The value from setValue cannot have leading or trailing newlines, whitespace, or comments"); } if (t == Tokens.END) { throw parseError("Empty value"); @@ -646,18 +656,22 @@ final class ConfigDocumentParser { if (flavor == ConfigSyntax.JSON) { AbstractConfigNodeValue node = parseValue(t); t = nextToken(); - while (Tokens.isIgnoredWhitespace(t) || Tokens.isNewline(t) || isUnquotedWhitespace(t)) { - t = nextToken(); - } if (t == Tokens.END) { return node; } else { - throw parseError("Tried to parse a concatenation. Concatenations not allowed in valid JSON"); + throw parseError("Parsing JSON and the value set in setValue was either a concatenation or " + + "had trailing whitespace, newlines, or comments"); } } else { putBack(t); ArrayList nodes = new ArrayList(); - return consolidateValues(nodes); + AbstractConfigNodeValue node = consolidateValues(nodes); + t = nextToken(); + if (t == Tokens.END) { + return node; + } else { + throw parseError("The value from setValue cannot have leading or trailing newlines, whitespace, or comments"); + } } } } diff --git a/config/src/test/scala/com/typesafe/config/impl/ConfigDocumentParserTest.scala b/config/src/test/scala/com/typesafe/config/impl/ConfigDocumentParserTest.scala index 1f6963a8..346e8457 100644 --- a/config/src/test/scala/com/typesafe/config/impl/ConfigDocumentParserTest.scala +++ b/config/src/test/scala/com/typesafe/config/impl/ConfigDocumentParserTest.scala @@ -61,6 +61,19 @@ class ConfigDocumentParserTest extends TestUtils { assertTrue(exceptionThrown) } + private def parseLeadingTrailingFailure(toReplace: String) { + var exceptionThrown = false + try { + ConfigDocumentParser.parseValue(tokenize(toReplace), ConfigParseOptions.defaults()) + } catch { + case e: Exception => + exceptionThrown = true + assertTrue(e.isInstanceOf[ConfigException]) + assertTrue(e.getMessage.contains("The value from setValue cannot have leading or trailing newlines, whitespace, or comments")) + } + assertTrue(exceptionThrown) + } + @Test def parseSuccess { parseTest("foo:bar") @@ -232,18 +245,10 @@ class ConfigDocumentParserTest extends TestUtils { parseSimpleValueTest("false") parseSimpleValueTest("null") - // Parse Simple Value throws out trailing and leading whitespace - parseSimpleValueTest(" 123", "123") - parseSimpleValueTest("123 ", "123") - parseSimpleValueTest(" 123 ", "123") - // Can parse complex values parseComplexValueTest("""{"a": "b"}""") parseComplexValueTest("""["a","b","c"]""") - parseSingleValueInvalidJSONTest("unquotedtext", "Token not allowed in valid JSON") - parseSingleValueInvalidJSONTest("${a.b}", "Substitutions (${} syntax) not allowed in JSON") - // Check that concatenations are handled by CONF parsing var origText = "123 456 unquotedtext abc" var node = ConfigDocumentParser.parseValue(tokenize(origText), ConfigParseOptions.defaults()) @@ -258,7 +263,38 @@ class ConfigDocumentParserTest extends TestUtils { case e: Exception => exceptionThrown = true assertTrue(e.isInstanceOf[ConfigException]) - assertTrue(e.getMessage.contains("Tried to parse a concatenation. Concatenations not allowed in valid JSON")) + assertTrue(e.getMessage.contains("Parsing JSON and the value set in setValue was either a concatenation or had trailing whitespace, newlines, or comments")) + } + assertTrue(exceptionThrown) + } + + @Test + def parseSingleValuesFailures { + // Parse Simple Value throws on leading and trailing whitespace, comments, or newlines + parseLeadingTrailingFailure(" 123") + parseLeadingTrailingFailure("123 ") + parseLeadingTrailingFailure(" 123 ") + parseLeadingTrailingFailure("\n123") + parseLeadingTrailingFailure("123\n") + parseLeadingTrailingFailure("\n123\n") + parseLeadingTrailingFailure("#thisisacomment\n123#comment") + + // Parse Simple Value correctly throws on whitespace after a concatenation + parseLeadingTrailingFailure("123 456 789 ") + + parseSingleValueInvalidJSONTest("unquotedtext", "Token not allowed in valid JSON") + parseSingleValueInvalidJSONTest("${a.b}", "Substitutions (${} syntax) not allowed in JSON") + + // Check that concatenations in JSON will throw an error + val origText = "123 456 789" + var exceptionThrown = false + try { + val node = ConfigDocumentParser.parseValue(tokenize(origText), ConfigParseOptions.defaults().setSyntax(ConfigSyntax.JSON)) + } catch { + case e: Exception => + exceptionThrown = true + assertTrue(e.isInstanceOf[ConfigException]) + assertTrue(e.getMessage.contains("Parsing JSON and the value set in setValue was either a concatenation or had trailing whitespace, newlines, or comments")) } assertTrue(exceptionThrown) } diff --git a/config/src/test/scala/com/typesafe/config/impl/ConfigDocumentTest.scala b/config/src/test/scala/com/typesafe/config/impl/ConfigDocumentTest.scala index 2c819f9a..3a395e37 100644 --- a/config/src/test/scala/com/typesafe/config/impl/ConfigDocumentTest.scala +++ b/config/src/test/scala/com/typesafe/config/impl/ConfigDocumentTest.scala @@ -226,7 +226,7 @@ class ConfigDocumentTest extends TestUtils { case e: Exception => exceptionThrown = true assertTrue(e.isInstanceOf[ConfigException]) - assertTrue(e.getMessage.contains("Tried to parse a concatenation. Concatenations not allowed in valid JSON")) + assertTrue(e.getMessage.contains("Parsing JSON and the value set in setValue was either a concatenation or had trailing whitespace, newlines, or comments")) } assertTrue(exceptionThrown) }