Repository: decorators-squad/eo-yaml (java)

Issue:
Misparsing of mapping entries containing colons
If i parse this:

```
root:
  no-colon-in-value: example.org
  colon-in-value: example.org:80
```

Then the second entry inside root has a key `colon-in-value: example.org` and a value `example.org:80`.

Perhaps the second entry is ambiguous. But that the `example.org` part appears in both the key and the value, which cannot possibly be right!

Here's a small program which reproduces this:

```
import com.amihaiemil.eoyaml.Yaml;
import com.amihaiemil.eoyaml.YamlMapping;
import com.amihaiemil.eoyaml.YamlNode;

import java.io.IOException;

public class ColonMisparseDemo {

    public static void main(String[] args) throws IOException {
        String input = """
                root:
                  no-colon-in-value: example.org
                  colon-in-value: example.org:80
                  """;

        YamlMapping document = Yaml.createYamlInput(input).readYamlMapping();

        YamlMapping root = document.yamlMapping("root");

        for (YamlNode key : root.keys()) {
            YamlNode value = root.value(key);
            System.out.println("[" + key.asScalar().value() + "] = [" + value.asScalar().value() + "]");
        }
    }

}
```

This prints:

```
[no-colon-in-value] = [example.org]
[colon-in-value: example.org] = [example.org:80]
```

I think it should print:

```
[no-colon-in-value] = [example.org]
[colon-in-value] = [example.org:80]
```


Pull request:
#517 The key of an entry must be followed by a space
The current version parses keys wrong if a scalar contains a colon. This PR will fix this by checking for a space after the colon of the key name.

This will fix #517

Patch:
diff --git a/src/main/java/com/amihaiemil/eoyaml/ReadYamlMapping.java b/src/main/java/com/amihaiemil/eoyaml/ReadYamlMapping.java
index bd525f6..a9f8f68 100644
--- a/src/main/java/com/amihaiemil/eoyaml/ReadYamlMapping.java
+++ b/src/main/java/com/amihaiemil/eoyaml/ReadYamlMapping.java
@@ -27,12 +27,16 @@
  */
 package com.amihaiemil.eoyaml;
 
-import static com.amihaiemil.eoyaml.YamlLine.UNKNOWN_LINE_NUMBER;
-
 import com.amihaiemil.eoyaml.exceptions.YamlReadingException;
-import java.util.*;
+
+import java.util.Iterator;
+import java.util.LinkedHashSet;
+import java.util.Set;
+import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
+import static com.amihaiemil.eoyaml.YamlLine.UNKNOWN_LINE_NUMBER;
+
 /**
  * YamlMapping read from somewhere. YAML directives and
  * document start/end markers are ignored. This is assumed
@@ -44,6 +48,8 @@ import java.util.regex.Pattern;
  */
 final class ReadYamlMapping extends BaseYamlMapping {
 
+    private static final Pattern KEY_PATTERN = Pattern.compile("^-?\\s*(?<key>.+):(|\\s.*)$");
+
     /**
      * Yaml line just previous to the one where this mapping starts. E.g.
      * <pre>
@@ -158,18 +164,12 @@ final class ReadYamlMapping extends BaseYamlMapping {
                 if(!trimmed.contains(":")) {
                     continue;
                 }
-                final String key;
-                if(trimmed.startsWith("-")) {
-                    key = trimmed.substring(
-                        1, trimmed.lastIndexOf(":")
-                    ).trim();
-                } else {
-                    key = trimmed.substring(
-                        0, trimmed.lastIndexOf(":")
-                    ).trim();
-                }
-                if(!key.isEmpty()) {
-                    keys.add(new PlainStringScalar(key));
+                final Matcher matcher = KEY_PATTERN.matcher(trimmed);
+                if (matcher.matches()) {
+                    final String key = matcher.group("key");
+                    if (!key.isEmpty()) {
+                        keys.add(new PlainStringScalar(key));
+                    }
                 }
             }
             prev = line;
diff --git a/src/main/java/com/amihaiemil/eoyaml/ReadYamlSequence.java b/src/main/java/com/amihaiemil/eoyaml/ReadYamlSequence.java
index 82c8170..50d7d34 100644
--- a/src/main/java/com/amihaiemil/eoyaml/ReadYamlSequence.java
+++ b/src/main/java/com/amihaiemil/eoyaml/ReadYamlSequence.java
@@ -237,8 +237,8 @@ final class ReadYamlSequence extends BaseYamlSequence {
      */
     private boolean mappingStartsAtDash(final YamlLine dashLine) {
         final String trimmed = dashLine.trimmed();
-        final boolean escapedScalar = trimmed.matches("^[ ]*\\-[ ]*\".*\"$")
-            || trimmed.matches("^[ ]*\\-[ ]*\'.*\'$");
-        return trimmed.matches("^.*\\-.*\\:.*$") && !escapedScalar;
+        final boolean escapedScalar = trimmed.matches("^\\s*-\\s*\".*\"$")
+            || trimmed.matches("^\\s*-\\s*'.*'$");
+        return trimmed.matches("^.*-.+:(|\\s.*)$") && !escapedScalar;
     }
 }
diff --git a/src/main/java/com/amihaiemil/eoyaml/RtYamlInput.java b/src/main/java/com/amihaiemil/eoyaml/RtYamlInput.java
index 10771a4..1bdbd81 100644
--- a/src/main/java/com/amihaiemil/eoyaml/RtYamlInput.java
+++ b/src/main/java/com/amihaiemil/eoyaml/RtYamlInput.java
@@ -193,8 +193,8 @@ final class RtYamlInput implements YamlInput {
     private boolean mappingStartsAtDash(final String line){
         //line without indentation.
         final String trimmed = line.trim();
-        final boolean escapedScalar = trimmed.matches("^[ ]*-[ ]*\".*\"$")
-            || trimmed.matches("^[ ]*-[ ]*'.*'$");
-        return trimmed.matches("^[ ]*-.*:.+$") && !escapedScalar;
+        final boolean escapedScalar = trimmed.matches("^\\s*-\\s*\".*\"$")
+            || trimmed.matches("^\\s*-\\s*'.*'$");
+        return trimmed.matches("^\\s*-.+:\\s.*$") && !escapedScalar;
     }
 }
diff --git a/src/main/java/com/amihaiemil/eoyaml/SameIndentationLevel.java b/src/main/java/com/amihaiemil/eoyaml/SameIndentationLevel.java
index 4ccc551..a554892 100644
--- a/src/main/java/com/amihaiemil/eoyaml/SameIndentationLevel.java
+++ b/src/main/java/com/amihaiemil/eoyaml/SameIndentationLevel.java
@@ -116,9 +116,9 @@ final class SameIndentationLevel implements YamlLines {
      */
     private boolean mappingStartsAtDash(final YamlLine dashLine) {
         final String trimmed = dashLine.trimmed();
-        final boolean escapedScalar = trimmed.matches("^[ ]*\\-[ ]*\".*\"$")
-            || trimmed.matches("^[ ]*\\-[ ]*\'.*\'$");
-        return trimmed.matches("^[ ]*\\-.*\\:.+$") && !escapedScalar;
+        final boolean escapedScalar = trimmed.matches("^\\s*-\\s*\".*\"$")
+            || trimmed.matches("^\\s*-\\s*'.*'$");
+        return trimmed.matches("^\\s*-.*:\\s.+$") && !escapedScalar;
     }
 
     /**
@@ -128,9 +128,9 @@ final class SameIndentationLevel implements YamlLines {
      */
     private boolean mapping(final YamlLine dashLine) {
         final String trimmed = dashLine.trimmed();
-        final boolean escapedScalar = trimmed.matches("^[ ]*\".*\"$")
-                || trimmed.matches("^[ ]*\'.*\'$");
-        return trimmed.matches("^[ ]*.*\\:.+$") && !escapedScalar;
+        final boolean escapedScalar = trimmed.matches("^\\s*\".*\"$")
+                || trimmed.matches("^\\s*'.*'$");
+        return trimmed.matches("^.*:\\s.+$") && !escapedScalar;
     }
 
 }
diff --git a/src/main/java/com/amihaiemil/eoyaml/WellIndented.java b/src/main/java/com/amihaiemil/eoyaml/WellIndented.java
index 5c56959..42d89bf 100644
--- a/src/main/java/com/amihaiemil/eoyaml/WellIndented.java
+++ b/src/main/java/com/amihaiemil/eoyaml/WellIndented.java
@@ -118,7 +118,7 @@ final class WellIndented implements YamlLines {
                 YamlLine line = iterator.next();
                 if(!(previous instanceof YamlLine.NullYamlLine)) {
                     int prevIndent = previous.indentation();
-                    if(previous.trimmed().matches("^[ ]*\\-.*\\:.*$")) {
+                    if(previous.trimmed().matches("^\\s*-.*:(|\\s.*)$")) {
                         prevIndent += 2;
                     }
                     int lineIndent = line.indentation();


Test patch:
diff --git a/src/test/java/com/amihaiemil/eoyaml/ReadYamlMappingTest.java b/src/test/java/com/amihaiemil/eoyaml/ReadYamlMappingTest.java
index 3bd0f55..9660d5e 100644
--- a/src/test/java/com/amihaiemil/eoyaml/ReadYamlMappingTest.java
+++ b/src/test/java/com/amihaiemil/eoyaml/ReadYamlMappingTest.java
@@ -27,6 +27,11 @@
  */
 package com.amihaiemil.eoyaml;
 
+import org.hamcrest.MatcherAssert;
+import org.hamcrest.Matchers;
+import org.junit.Ignore;
+import org.junit.Test;
+
 import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Collection;
@@ -34,11 +39,6 @@ import java.util.Iterator;
 import java.util.List;
 import java.util.Set;
 
-import org.hamcrest.MatcherAssert;
-import org.hamcrest.Matchers;
-import org.junit.Ignore;
-import org.junit.Test;
-
 /**
  * Unit tests for {@link ReadYamlMapping}.
  * @checkstyle MethodName (1500 lines)
@@ -1170,4 +1170,14 @@ public final class ReadYamlMappingTest {
         MatcherAssert.assertThat(copy.string("key2"), Matchers
             .equalTo("Some other value."));
     }
+
+    @Test
+    public void shouldReadKeyProperlyIfValueContainsColon() {
+        final List<YamlLine> lines = new ArrayList<>();
+        lines.add(new RtYamlLine("key: value:with-colon", 0));
+        ReadYamlMapping mapping = new ReadYamlMapping(new AllYamlLines(lines));
+
+        MatcherAssert.assertThat(mapping.keys(), Matchers.hasSize(1));
+        MatcherAssert.assertThat(mapping.keys().iterator().next().asScalar().value(), Matchers.equalTo("key"));
+    }
 }
diff --git a/src/test/java/com/amihaiemil/eoyaml/ReadYamlSequenceTest.java b/src/test/java/com/amihaiemil/eoyaml/ReadYamlSequenceTest.java
index 7bf247a..e5511d7 100644
--- a/src/test/java/com/amihaiemil/eoyaml/ReadYamlSequenceTest.java
+++ b/src/test/java/com/amihaiemil/eoyaml/ReadYamlSequenceTest.java
@@ -27,16 +27,16 @@
  */
 package com.amihaiemil.eoyaml;
 
-import java.util.ArrayList;
-import java.util.Collection;
-import java.util.Iterator;
-import java.util.List;
-
 import org.hamcrest.MatcherAssert;
 import org.hamcrest.Matchers;
 import org.junit.Ignore;
 import org.junit.Test;
 
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.Iterator;
+import java.util.List;
+
 /**
  * Unit tests for {@link ReadYamlSequence}.
  * @author Mihai Andronache (amihaiemil@gmail.com)
@@ -484,4 +484,18 @@ public final class ReadYamlSequenceTest {
         );
         MatcherAssert.assertThat(sequence.toString(), Matchers.isEmptyString());
     }
+
+    @Test
+    public void dontReturnMappingForScalarWithColon() {
+        final List<YamlLine> lines = new ArrayList<>();
+        lines.add(new RtYamlLine("- scalar:with-colon", 0));
+        final YamlSequence sequence = new ReadYamlSequence(
+                new AllYamlLines(lines)
+        );
+
+        MatcherAssert.assertThat(sequence.values(), Matchers.hasSize(1));
+        YamlNode sequenceItem = sequence.values().iterator().next();
+        MatcherAssert.assertThat(sequenceItem.type(), Matchers.equalTo(Node.SCALAR));
+        MatcherAssert.assertThat(sequenceItem.asScalar().value(), Matchers.equalTo("scalar:with-colon"));
+    }
 }
diff --git a/src/test/java/com/amihaiemil/eoyaml/RtYamlInputTest.java b/src/test/java/com/amihaiemil/eoyaml/RtYamlInputTest.java
index 79c723e..4b87887 100644
--- a/src/test/java/com/amihaiemil/eoyaml/RtYamlInputTest.java
+++ b/src/test/java/com/amihaiemil/eoyaml/RtYamlInputTest.java
@@ -27,17 +27,20 @@
  */
 package com.amihaiemil.eoyaml;
 
-import java.io.*;
-import java.util.Collection;
-import java.util.Iterator;
-import java.util.Set;
-
 import org.apache.commons.io.IOUtils;
 import org.hamcrest.MatcherAssert;
 import org.hamcrest.Matchers;
-import org.junit.Ignore;
 import org.junit.Test;
 
+import java.io.ByteArrayInputStream;
+import java.io.File;
+import java.io.FileInputStream;
+import java.io.FileNotFoundException;
+import java.io.IOException;
+import java.util.Collection;
+import java.util.Iterator;
+import java.util.Set;
+
 /**
  * Unit tests for {@link RtYamlInput}.
  * @author Mihai Andronache (amihaiemil@gmail.com)
@@ -939,18 +942,15 @@ public final class RtYamlInputTest {
     }
 
     /**
-     * Do a round-trip test on sample file containing a scalar using Spring
-     * property reference syntax.
+     * Do a round-trip test on sample file containing a scalar using Spring property reference syntax.
      *
-     * Unit test for
      * <a href="https://github.com/decorators-squad/eo-yaml/issues/518">#518</a>
+     * based on
+     * <a href="https://github.com/decorators-squad/eo-yaml/issues/515">PR</a>
      *
-     * @todo #518:60min Fix the bug reported in 518, so that this unit test
-     *  passes.
      * @throws IOException When there's a problem reading the sample files.
      */
     @Test
-    @Ignore
     public void supportsSpringPropertyRef() throws IOException {
         final String filename = "issue_518_spring_property_ref.yml";
         final String fileContents = readTestResource(filename).trim();
@@ -977,6 +977,43 @@ public final class RtYamlInputTest {
         MatcherAssert.assertThat(pretty, Matchers.equalTo(fileContents));
     }
 
+    @Test
+    public void shouldReadKeysProperly() throws IOException {
+        final String filename = "issue_517_values_with_colons.yml";
+
+        final YamlMapping read = new RtYamlInput(
+                new FileInputStream("src/test/resources/" + filename)
+        ).readYamlMapping();
+
+        MatcherAssert.assertThat(read.type(), Matchers.equalTo(Node.MAPPING));
+        MatcherAssert.assertThat(
+                read.asMapping().keys(),
+                Matchers.hasSize(2));
+
+        final YamlNode topLevelMapping = read.asMapping().value("a_mapping");
+        MatcherAssert.assertThat(
+                topLevelMapping.type(),
+                Matchers.equalTo(Node.MAPPING));
+        MatcherAssert.assertThat(
+                topLevelMapping.asMapping().value("a_scalar").type(),
+                Matchers.equalTo(Node.SCALAR));
+        MatcherAssert.assertThat(
+                topLevelMapping.asMapping().value("a_scalar").asScalar().value(),
+                Matchers.equalTo("value:with-colon"));
+
+        YamlNode topLevelSequence = read.asMapping().value("a_sequence");
+        MatcherAssert.assertThat(
+                topLevelSequence.type(),
+                Matchers.equalTo(Node.SEQUENCE));
+        MatcherAssert.assertThat(topLevelSequence.asSequence().values(),
+                                 Matchers.hasSize(1));
+        YamlNode sequenceItem = topLevelSequence.asSequence().values().iterator().next();
+        MatcherAssert.assertThat(sequenceItem.type(),
+                                 Matchers.equalTo(Node.SCALAR));
+        MatcherAssert.assertThat(sequenceItem.asScalar().value(),
+                                 Matchers.equalTo("value:with-colon"));
+    }
+
     /**
      * Read a test resource file's contents.
      * @param fileName File to read.
diff --git a/src/test/resources/issue_517_values_with_colons.yml b/src/test/resources/issue_517_values_with_colons.yml
new file mode 100644
index 0000000..e6628de
--- /dev/null
+++ b/src/test/resources/issue_517_values_with_colons.yml
@@ -0,0 +1,5 @@
+a_mapping:
+  a_scalar: value:with-colon
+
+a_sequence:
+  - value:with-colon