From f32c8bdd78f5ad88692b6d96b9337ebc01ccae90 Mon Sep 17 00:00:00 2001
From: Havoc Pennington <hp@pobox.com>
Date: Tue, 15 Nov 2011 23:28:08 -0500
Subject: [PATCH] fix parsing of empty string path, and fix test suite which
 wasn't catching it

---
 .../java/com/typesafe/config/impl/Parser.java |  2 +
 .../typesafe/config/impl/ConfParserTest.scala | 50 ++++++++++++++-----
 .../com/typesafe/config/impl/PathTest.scala   | 14 ++++++
 3 files changed, 53 insertions(+), 13 deletions(-)

diff --git a/src/main/java/com/typesafe/config/impl/Parser.java b/src/main/java/com/typesafe/config/impl/Parser.java
index ec4e7165..7b904871 100644
--- a/src/main/java/com/typesafe/config/impl/Parser.java
+++ b/src/main/java/com/typesafe/config/impl/Parser.java
@@ -711,6 +711,8 @@ final class Parser {
     // we just have something like "foo" or "foo.bar"
     private static Path speculativeFastParsePath(String path) {
         String s = ConfigUtil.unicodeTrim(path);
+        if (s.isEmpty())
+            return null;
         if (hasUnsafeChars(s))
             return null;
         if (s.startsWith(".") || s.endsWith(".") || s.contains(".."))
diff --git a/src/test/scala/com/typesafe/config/impl/ConfParserTest.scala b/src/test/scala/com/typesafe/config/impl/ConfParserTest.scala
index af1675cb..e5e38f64 100644
--- a/src/test/scala/com/typesafe/config/impl/ConfParserTest.scala
+++ b/src/test/scala/com/typesafe/config/impl/ConfParserTest.scala
@@ -54,23 +54,47 @@ class ConfParserTest extends TestUtils {
     }
 
     private def parsePath(s: String): Path = {
+        var firstException: ConfigException = null
+        var secondException: ConfigException = null
+
         // parse first by wrapping into a whole document and using
         // the regular parser.
-        val tree = parseWithoutResolving("[${" + s + "}]")
-        val result = tree match {
-            case list: ConfigList =>
-                list.get(0) match {
-                    case subst: ConfigSubstitution =>
-                        subst.pieces().get(0) match {
-                            case p: Path => p
-                        }
-                }
+        val result = try {
+            val tree = parseWithoutResolving("[${" + s + "}]")
+            tree match {
+                case list: ConfigList =>
+                    list.get(0) match {
+                        case subst: ConfigSubstitution =>
+                            subst.pieces().get(0) match {
+                                case p: Path => p
+                            }
+                    }
+            }
+        } catch {
+            case e: ConfigException =>
+                firstException = e
+                null
         }
 
         // also parse with the standalone path parser and be sure the
         // outcome is the same.
-        val shouldBeSame = Parser.parsePath(s)
-        assertEquals(result, shouldBeSame)
+        try {
+            val shouldBeSame = Parser.parsePath(s)
+            assertEquals(result, shouldBeSame)
+        } catch {
+            case e: ConfigException =>
+                secondException = e
+        }
+
+        if (firstException == null && secondException != null)
+            throw new AssertionError("only the standalone path parser threw", secondException)
+        if (firstException != null && secondException == null)
+            throw new AssertionError("only the whole-document parser threw", firstException)
+
+        if (firstException != null)
+            throw firstException
+        if (secondException != null)
+            throw new RuntimeException("wtf, should have thrown because not equal")
 
         result
     }
@@ -102,14 +126,14 @@ class ConfParserTest extends TestUtils {
         assertEquals(path("a_c"), parsePath("a_c"))
         assertEquals(path("-"), parsePath("\"-\""))
 
-        for (invalid <- Seq("", "a.", ".b", "a..b", "a${b}c", "\"\".", ".\"\"")) {
+        for (invalid <- Seq("", " ", "  \n   \n  ", "a.", ".b", "a..b", "a${b}c", "\"\".", ".\"\"")) {
             try {
                 intercept[ConfigException.BadPath] {
                     parsePath(invalid)
                 }
             } catch {
                 case e =>
-                    System.err.println("failed on: " + invalid);
+                    System.err.println("failed on: '" + invalid + "'");
                     throw e;
             }
         }
diff --git a/src/test/scala/com/typesafe/config/impl/PathTest.scala b/src/test/scala/com/typesafe/config/impl/PathTest.scala
index 0b16dced..22bd903c 100644
--- a/src/test/scala/com/typesafe/config/impl/PathTest.scala
+++ b/src/test/scala/com/typesafe/config/impl/PathTest.scala
@@ -3,6 +3,7 @@ package com.typesafe.config.impl
 import org.junit.Assert._
 import org.junit._
 import scala.collection.JavaConverters._
+import com.typesafe.config.ConfigException
 
 class PathTest extends TestUtils {
 
@@ -62,4 +63,17 @@ class PathTest extends TestUtils {
         assertEquals(1, path("foo").length())
         assertEquals(2, path("foo", "bar").length())
     }
+
+    @Test
+    def pathsAreInvalid() {
+        // this test is just of the Path.newPath() wrapper, the extensive
+        // test of different paths is over in ConfParserTest
+        intercept[ConfigException.BadPath] {
+            Path.newPath("")
+        }
+
+        intercept[ConfigException.BadPath] {
+            Path.newPath("")
+        }
+    }
 }