Merge pull request #416 from Johnlon/master

Fixed some problems preventing tests passing on Windows

HOCON.md

@@ -1494,7 +1494,9 @@ environment variables generally are capitalized. This avoids
 naming collisions between environment variables and configuration
 properties. (While on Windows getenv() is generally not
 case-sensitive, the lookup will be case sensitive all the way
-until the env variable fallback lookup is reached.)
+until the env variable fallback lookup is reached).
+
+See also the notes below on Windows and case sensitivity.
 
 An application can explicitly block looking up a substitution in
 the environment by setting a value in the configuration, with the
@@ -1543,3 +1545,47 @@ Differences include but are probably not limited to:
    properties files only recognize comment characters if they
    occur as the first character on the line
  - HOCON interprets `${}` as a substitution
+
+## Note on Windows and case sensitivity of environment variables
+
+HOCON's lookup of environment variable values is always case sensitive, but
+Linux and Windows differ in their handling of case.
+
+Linux allows one to define multiple environment variables with the same
+name but with different case; so both "PATH" and "Path" may be defined
+simultaneously. HOCON's access to these environment variables on Linux
+is straightforward; ie just make sure you define all your vars with the required case.
+
+Windows is more confusing. Windows environment variables names may contain a
+mix of upper and lowercase characters, eg "Path", however Windows does not
+allow one to define multiple instances of the same name but differing in case.
+Whilst accessing env vars in Windows is case insensitive, accessing env vars in
+HOCON is case sensitive.
+So if you know that you HOCON needs "PATH" then you must ensure that
+the variable is defined as "PATH" rather than some other name such as
+"Path" or "path".
+However, Windows does not allow us to change the case of an existing env var; we can't
+simply redefine the var with an upper case name.
+The only way to ensure that your environment variables have the desired case
+is to first undefine all the env vars that you will depend on then redefine
+them with the required case.
+
+For example, the the ambient environment might have this defition ...
+
+```
+set Path=A;B;C
+```
+.. we just don't know. But if the HOCON needs "PATH", then the start script must
+take a precautionary approach and enforce the necessary case as follows ...
+
+```
+set OLDPATH=%PATH%
+set PATH=
+set PATH=%OLDPATH%
+
+%JAVA_HOME%/bin/java ....
+```
+
+You cannot know what ambient environment variables might exist in the ambient environment
+when your program is invoked, nor what case those definitions might have.
+Therefore the only safe thing to do is redefine all the vars you rely on as shown above.

config/src/test/scala/com/typesafe/config/impl/ConfigDocumentTest.scala

@@ -294,7 +294,7 @@ class ConfigDocumentTest extends TestUtils {
         val configDocument = ConfigDocumentFactory.parseFile(resourceFile("/test03.conf"))
         val fileReader = new BufferedReader(new FileReader("config/src/test/resources/test03.conf"))
         var line = fileReader.readLine()
-        var sb = new StringBuilder()
+        val sb = new StringBuilder()
         while (line != null) {
             sb.append(line)
             sb.append("\n")
@@ -302,9 +302,11 @@ class ConfigDocumentTest extends TestUtils {
         }
         fileReader.close()
         val fileText = sb.toString()
-        assertEquals(fileText, configDocument.render())
+        assertEquals(fileText, defaultLineEndingsToUnix(configDocument.render()))
     }
 
+    private def defaultLineEndingsToUnix(s: String): String = s.replaceAll(System.lineSeparator(), "\n")
+
     @Test
     def configDocumentReaderParse {
         val configDocument = ConfigDocumentFactory.parseReader(new FileReader(resourceFile("/test03.conf")))

config/src/test/scala/com/typesafe/config/impl/ConfigSubstitutionTest.scala

@@ -739,14 +739,17 @@ class ConfigSubstitutionTest extends TestUtils {
     }
 
     private val substEnvVarObject = {
+        // prefix the names of keys with "key_" to allow us to embed a case sensitive env var name
+        // in the key that wont therefore risk a naming collision with env vars themselves
         parseObject("""
 {
-    "home" : ${?HOME},
-    "pwd" : ${?PWD},
-    "shell" : ${?SHELL},
-    "lang" : ${?LANG},
-    "path" : ${?PATH},
-    "not_here" : ${?NOT_HERE}
+    "key_HOME" : ${?HOME},
+    "key_PWD" : ${?PWD},
+    "key_SHELL" : ${?SHELL},
+    "key_LANG" : ${?LANG},
+    "key_PATH" : ${?PATH},
+    "key_Path" : ${?Path}, // many windows machines use Path rather than PATH
+    "key_NOT_HERE" : ${?NOT_HERE}
 }
 """)
     }
@@ -759,7 +762,8 @@ class ConfigSubstitutionTest extends TestUtils {
 
         var existed = 0
         for (k <- resolved.root.keySet().asScala) {
-            val e = System.getenv(k.toUpperCase());
+            val envVarName = k.replace("key_", "")
+            val e = System.getenv(envVarName)
             if (e != null) {
                 existed += 1
                 assertEquals(e, resolved.getString(k))
@@ -782,7 +786,8 @@ class ConfigSubstitutionTest extends TestUtils {
         // { HOME : null } then ${HOME} should be null.
         val nullsMap = new java.util.HashMap[String, Object]
         for (k <- substEnvVarObject.keySet().asScala) {
-            nullsMap.put(k.toUpperCase(), null);
+            val envVarName = k.replace("key_", "")
+            nullsMap.put(envVarName, null)
         }
         val nulls = ConfigFactory.parseMap(nullsMap, "nulls map")
 
@@ -802,11 +807,12 @@ class ConfigSubstitutionTest extends TestUtils {
 
         values.put("a", substEnvVarObject.relativized(new Path("a")))
 
-        val resolved = resolve(new SimpleConfigObject(fakeOrigin(), values));
+        val resolved = resolve(new SimpleConfigObject(fakeOrigin(), values))
 
         var existed = 0
         for (k <- resolved.getObject("a").keySet().asScala) {
-            val e = System.getenv(k.toUpperCase());
+            val envVarName = k.replace("key_", "")
+            val e = System.getenv(envVarName)
             if (e != null) {
                 existed += 1
                 assertEquals(e, resolved.getConfig("a").getString(k))

config/src/test/scala/com/typesafe/config/impl/ConfigTest.scala

@@ -858,7 +858,7 @@ class ConfigTest extends TestUtils {
         if (home != null) {
             assertEquals(home, conf.getString("system.home"))
         } else {
-            assertEquals(nullValue, conf.getObject("system").get("home"))
+            assertEquals(null, conf.getObject("system").get("home"))
         }
     }
 
@@ -964,7 +964,7 @@ class ConfigTest extends TestUtils {
         if (home != null) {
             assertEquals(home, conf.getString("test01.system.home"))
         } else {
-            assertEquals(nullValue, conf.getObject("test01.system").get("home"))
+            assertEquals(null, conf.getObject("test01.system").get("home"))
         }
         val concatenated = conf.getString("test01.system.concatenated")
         assertTrue(concatenated.contains("Your Java version"))