[SHIRO-530] INI parser does not properly handled backslashes at end of values

bmarwell · bmarwell · commit b50b829a285b · 2020-04-01T07:24:19.000+02:00
- Do not skip escape characters for the value (new behaviour demanded by SHIRO-530). - rearrange and comment tests to reflect old and new, desired behaviour. Signed-off-by: Benjamin Marwell <bmarwell@gmail.com>
diff --git a/config/core/src/main/java/org/apache/shiro/config/Ini.java b/config/core/src/main/java/org/apache/shiro/config/Ini.java
@@ -587,7 +587,7 @@ protected static String[] splitKeyValue(String keyValueLine) {
                 } else {
                     if (valueBuffer.length() == 0 && isKeyValueSeparatorChar(c) && !isCharEscaped(line, i)) {
                         //swallow the separator chars before we start building the value
-                    } else if (!isCharEscaped(line, i)){
+                    } else {
                         valueBuffer.append(c);
                     }
                 }
diff --git a/config/core/src/test/groovy/org/apache/shiro/config/IniTest.groovy b/config/core/src/test/groovy/org/apache/shiro/config/IniTest.groovy
@@ -72,6 +72,22 @@ public class IniTest {
         assertTrue(Ini.Section.isContinued(line));
     }
 
+    @Test
+    public void testBackslash() {
+        String test = "Truth=Beauty\\\\";
+        Ini ini = new Ini();
+        ini.load(test);
+
+        assertNotNull(ini.getSections());
+        assertEquals(1, ini.getSections().size());
+
+        Ini.Section section = ini.getSections().iterator().next();
+        assertEquals(Ini.DEFAULT_SECTION_NAME, section.getName());
+        assertFalse(section.isEmpty());
+        assertEquals(1, section.size());
+        assertEquals("Beauty\\\\", section.get("Truth"));
+    }
+
     @Test
     public void testSplitKeyValue() {
         String test = "Truth Beauty";
@@ -119,24 +135,40 @@ public class IniTest {
         assertEquals("Truth", kv[0]);
         assertEquals("Beauty", kv[1]);
 
+        // Escape characters are to be removed from the key.
+        // This is different behaviour compared to the XML config.
         test = "Tru\\th=Beauty";
         kv = Ini.Section.splitKeyValue(test);
         assertEquals("Truth", kv[0]);
         assertEquals("Beauty", kv[1]);
 
-        test = "Truth\\=Beauty";
+        // SHIRO-530: Keep backslashes in value.
+        test = "Truth=Beau\\ty";
         kv = Ini.Section.splitKeyValue(test);
         assertEquals("Truth", kv[0]);
-        assertEquals("Beauty", kv[1]);
+        assertEquals("Beau\\ty", kv[1]);
 
-        test = "Truth=Beau\\ty";
+        // SHIRO-530: Keep backslashes in value.
+        test = "Truth=Beauty\\";
         kv = Ini.Section.splitKeyValue(test);
         assertEquals("Truth", kv[0]);
-        assertEquals("Beauty", kv[1]);
+        assertEquals("Beauty\\", kv[1]);
 
-        test = "Truth=Beauty\\";
+        // SHIRO-530: Keep backslashes in value.
+        test = "Truth= \\ Beauty\\";
         kv = Ini.Section.splitKeyValue(test);
         assertEquals("Truth", kv[0]);
+        assertEquals("\\ Beauty\\", kv[1]);
+    }
+
+    /**
+     * Tests if an escaped separator char will not be recognized as such.
+     */
+    @Test
+    public void testSplitKeyValueEscapedEquals()  {
+        String test = "Truth\\=Beauty";
+        String[] kv = Ini.Section.splitKeyValue(test);
+        assertEquals("Truth", kv[0]);
         assertEquals("Beauty", kv[1]);
     }
 

Original file line number	Diff line number	Diff line change
`@@ -587,7 +587,7 @@ protected static String[] splitKeyValue(String keyValueLine) {`
`587`	`587`	`} else {`
`588`	`588`	`if (valueBuffer.length() == 0 && isKeyValueSeparatorChar(c) && !isCharEscaped(line, i)) {`
`589`	`589`	`//swallow the separator chars before we start building the value`
`590`		`- } else if (!isCharEscaped(line, i)){`
	`590`	`+ } else {`
`591`	`591`	`valueBuffer.append(c);`
`592`	`592`	`}`
`593`	`593`	`}`