diff --git a/spring-boot/src/main/java/org/springframework/boot/json/BasicJsonParser.java b/spring-boot/src/main/java/org/springframework/boot/json/BasicJsonParser.java index c5da1e13d8..ec43e0cb45 100644 --- a/spring-boot/src/main/java/org/springframework/boot/json/BasicJsonParser.java +++ b/spring-boot/src/main/java/org/springframework/boot/json/BasicJsonParser.java @@ -17,7 +17,6 @@ package org.springframework.boot.json; import java.util.ArrayList; -import java.util.HashMap; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; @@ -31,6 +30,7 @@ import org.springframework.util.StringUtils; * YAML are supported). * * @author Dave Syer + * @author Jean de Klerk * @since 1.2.0 * @see JsonParserFactory */ @@ -43,11 +43,8 @@ public class BasicJsonParser implements JsonParser { if (json.startsWith("{")) { return parseMapInternal(json); } - else if (json.equals("")) { - return new HashMap(); - } } - return null; + throw new IllegalArgumentException("Cannot parse JSON"); } @Override @@ -57,11 +54,8 @@ public class BasicJsonParser implements JsonParser { if (json.startsWith("[")) { return parseListInternal(json); } - else if (json.trim().equals("")) { - return new ArrayList(); - } } - return null; + throw new IllegalArgumentException("Cannot parse JSON"); } private List parseListInternal(String json) { diff --git a/spring-boot/src/main/java/org/springframework/boot/json/GsonJsonParser.java b/spring-boot/src/main/java/org/springframework/boot/json/GsonJsonParser.java index 4132d7a36f..7ca22a9d0c 100644 --- a/spring-boot/src/main/java/org/springframework/boot/json/GsonJsonParser.java +++ b/spring-boot/src/main/java/org/springframework/boot/json/GsonJsonParser.java @@ -26,6 +26,7 @@ import com.google.gson.GsonBuilder; * Thin wrapper to adapt {@link Gson} to a {@link JsonParser}. * * @author Dave Syer + * @author Jean de Klerk * @since 1.2.0 * @see JsonParserFactory */ @@ -34,17 +35,27 @@ public class GsonJsonParser implements JsonParser { private Gson gson = new GsonBuilder().create(); @Override + @SuppressWarnings("unchecked") public Map parseMap(String json) { - @SuppressWarnings("unchecked") - Map value = this.gson.fromJson(json, Map.class); - return value; + if (json != null) { + json = json.trim(); + if (json.startsWith("{")) { + return this.gson.fromJson(json, Map.class); + } + } + throw new IllegalArgumentException("Cannot parse JSON"); } @Override + @SuppressWarnings("unchecked") public List parseList(String json) { - @SuppressWarnings("unchecked") - List value = this.gson.fromJson(json, List.class); - return value; + if (json != null) { + json = json.trim(); + if (json.startsWith("[")) { + return this.gson.fromJson(json, List.class); + } + } + throw new IllegalArgumentException("Cannot parse JSON"); } } diff --git a/spring-boot/src/main/java/org/springframework/boot/json/JsonSimpleJsonParser.java b/spring-boot/src/main/java/org/springframework/boot/json/JsonSimpleJsonParser.java index 6a2ded4595..7e47e542cc 100644 --- a/spring-boot/src/main/java/org/springframework/boot/json/JsonSimpleJsonParser.java +++ b/spring-boot/src/main/java/org/springframework/boot/json/JsonSimpleJsonParser.java @@ -16,8 +16,6 @@ package org.springframework.boot.json; -import java.util.ArrayList; -import java.util.LinkedHashMap; import java.util.List; import java.util.Map; @@ -28,38 +26,32 @@ import org.json.simple.parser.ParseException; * Thin wrapper to adapt {@link org.json.simple.JSONObject} to a {@link JsonParser}. * * @author Dave Syer + * @author Jean de Klerk * @since 1.2.0 * @see JsonParserFactory */ public class JsonSimpleJsonParser implements JsonParser { @Override + @SuppressWarnings("unchecked") public Map parseMap(String json) { - Map map = new LinkedHashMap(); try { - @SuppressWarnings("unchecked") - Map value = (Map) new JSONParser() - .parse(json); - map.putAll(value); + return (Map) new JSONParser().parse(json); } catch (ParseException ex) { throw new IllegalArgumentException("Cannot parse JSON", ex); } - return map; } @Override + @SuppressWarnings("unchecked") public List parseList(String json) { - List nested = new ArrayList(); try { - @SuppressWarnings("unchecked") - List value = (List) new JSONParser().parse(json); - nested.addAll(value); + return (List) new JSONParser().parse(json); } catch (ParseException ex) { throw new IllegalArgumentException("Cannot parse JSON", ex); } - return nested; } } diff --git a/spring-boot/src/main/java/org/springframework/boot/json/YamlJsonParser.java b/spring-boot/src/main/java/org/springframework/boot/json/YamlJsonParser.java index 46f8885a4d..433d19b85f 100644 --- a/spring-boot/src/main/java/org/springframework/boot/json/YamlJsonParser.java +++ b/spring-boot/src/main/java/org/springframework/boot/json/YamlJsonParser.java @@ -25,6 +25,7 @@ import org.yaml.snakeyaml.Yaml; * Thin wrapper to adapt Snake {@link Yaml} to {@link JsonParser}. * * @author Dave Syer + * @author Jean de Klerk * @see JsonParserFactory */ public class YamlJsonParser implements JsonParser { @@ -32,13 +33,25 @@ public class YamlJsonParser implements JsonParser { @Override @SuppressWarnings("unchecked") public Map parseMap(String json) { - return new Yaml().loadAs(json, Map.class); + if (json != null) { + json = json.trim(); + if (json.startsWith("{")) { + return new Yaml().loadAs(json, Map.class); + } + } + throw new IllegalArgumentException("Cannot parse JSON"); } @Override @SuppressWarnings("unchecked") public List parseList(String json) { - return new Yaml().loadAs(json, List.class); + if (json != null) { + json = json.trim(); + if (json.startsWith("[")) { + return new Yaml().loadAs(json, List.class); + } + } + throw new IllegalArgumentException("Cannot parse JSON"); } } diff --git a/spring-boot/src/test/java/org/springframework/boot/json/AbstractJsonParserTests.java b/spring-boot/src/test/java/org/springframework/boot/json/AbstractJsonParserTests.java index 705c2ec3cc..d9d3494b05 100644 --- a/spring-boot/src/test/java/org/springframework/boot/json/AbstractJsonParserTests.java +++ b/spring-boot/src/test/java/org/springframework/boot/json/AbstractJsonParserTests.java @@ -19,7 +19,9 @@ package org.springframework.boot.json; import java.util.List; import java.util.Map; +import org.junit.Rule; import org.junit.Test; +import org.junit.rules.ExpectedException; import static org.junit.Assert.assertEquals; @@ -27,15 +29,19 @@ import static org.junit.Assert.assertEquals; * Base for {@link JsonParser} tests. * * @author Dave Syer + * @author Jean de Klerk */ public abstract class AbstractJsonParserTests { + @Rule + public ExpectedException thrown = ExpectedException.none(); + private final JsonParser parser = getParser(); protected abstract JsonParser getParser(); @Test - public void testSimpleMap() { + public void simpleMap() { Map map = this.parser.parseMap("{\"foo\":\"bar\",\"spam\":1}"); assertEquals(2, map.size()); assertEquals("bar", map.get("foo")); @@ -43,7 +49,7 @@ public abstract class AbstractJsonParserTests { } @Test - public void testDoubleValue() { + public void doubleValue() { Map map = this.parser.parseMap("{\"foo\":\"bar\",\"spam\":1.23}"); assertEquals(2, map.size()); assertEquals("bar", map.get("foo")); @@ -51,27 +57,27 @@ public abstract class AbstractJsonParserTests { } @Test - public void testEmptyMap() { + public void emptyMap() { Map map = this.parser.parseMap("{}"); assertEquals(0, map.size()); } @Test - public void testSimpleList() { + public void simpleList() { List list = this.parser.parseList("[\"foo\",\"bar\",1]"); assertEquals(3, list.size()); assertEquals("bar", list.get(1)); } @Test - public void testEmptyList() { + public void emptyList() { List list = this.parser.parseList("[]"); assertEquals(0, list.size()); } @SuppressWarnings("unchecked") @Test - public void testListOfMaps() { + public void listOfMaps() { List list = this.parser .parseList("[{\"foo\":\"bar\",\"spam\":1},{\"foo\":\"baz\",\"spam\":2}]"); assertEquals(2, list.size()); @@ -80,11 +86,73 @@ public abstract class AbstractJsonParserTests { @SuppressWarnings("unchecked") @Test - public void testMapOfLists() { + public void mapOfLists() { Map map = this.parser.parseMap( "{\"foo\":[{\"foo\":\"bar\",\"spam\":1},{\"foo\":\"baz\",\"spam\":2}]}"); assertEquals(1, map.size()); assertEquals(2, ((List) map.get("foo")).size()); } + @Test + public void mapWithNullThrowsARuntimeException() { + this.thrown.expect(RuntimeException.class); + this.parser.parseMap(null); + } + + @Test + public void listWithNullThrowsARuntimeException() { + this.thrown.expect(RuntimeException.class); + this.parser.parseList(null); + } + + @Test + public void mapWithEmptyStringThrowsARuntimeException() { + this.thrown.expect(RuntimeException.class); + this.parser.parseMap(""); + } + + @Test + public void listWithEmptyStringThrowsARuntimeException() { + this.thrown.expect(RuntimeException.class); + this.parser.parseList(""); + } + + @Test + public void mapWithListThrowsARuntimeException() { + this.thrown.expect(RuntimeException.class); + this.parser.parseMap("[]"); + } + + @Test + public void listWithMapThrowsARuntimeException() { + this.thrown.expect(RuntimeException.class); + this.parser.parseList("{}"); + } + + @Test + public void listWithLeadingWhitespace() { + List list = this.parser.parseList("\n\t[\"foo\"]"); + assertEquals(1, list.size()); + assertEquals("foo", list.get(0)); + } + + @Test + public void mapWithLeadingWhitespace() { + Map map = this.parser.parseMap("\n\t{\"foo\":\"bar\"}"); + assertEquals(1, map.size()); + assertEquals("bar", map.get("foo")); + } + + @Test + public void mapWithLeadingWhitespaceListThrowsARuntimeException() { + this.thrown.expect(RuntimeException.class); + this.parser.parseMap("\n\t[]"); + } + + @Test + public void listWithLeadingWhitespaceMapThrowsARuntimeException() { + this.thrown.expect(RuntimeException.class); + this.parser.parseList("\n\t{}"); + } + } diff --git a/spring-boot/src/test/java/org/springframework/boot/json/JsonJsonParserTests.java b/spring-boot/src/test/java/org/springframework/boot/json/JsonJsonParserTests.java index bc96f46787..b2a45e90bc 100644 --- a/spring-boot/src/test/java/org/springframework/boot/json/JsonJsonParserTests.java +++ b/spring-boot/src/test/java/org/springframework/boot/json/JsonJsonParserTests.java @@ -17,7 +17,7 @@ package org.springframework.boot.json; /** - * Tests for {@link JsonSimpleJsonParser}. + * Tests for {@link JsonJsonParser}. * * @author Dave Syer */ @@ -25,7 +25,7 @@ public class JsonJsonParserTests extends AbstractJsonParserTests { @Override protected JsonParser getParser() { - return new JsonSimpleJsonParser(); + return new JsonJsonParser(); } } diff --git a/spring-boot/src/test/java/org/springframework/boot/json/SimpleJsonJsonParserTests.java b/spring-boot/src/test/java/org/springframework/boot/json/JsonSimpleJsonParserTests.java similarity index 84% rename from spring-boot/src/test/java/org/springframework/boot/json/SimpleJsonJsonParserTests.java rename to spring-boot/src/test/java/org/springframework/boot/json/JsonSimpleJsonParserTests.java index 9a4a5cf907..a290f1c525 100644 --- a/spring-boot/src/test/java/org/springframework/boot/json/SimpleJsonJsonParserTests.java +++ b/spring-boot/src/test/java/org/springframework/boot/json/JsonSimpleJsonParserTests.java @@ -17,15 +17,15 @@ package org.springframework.boot.json; /** - * Tests for {@link JsonJsonParser}. + * Tests for {@link JsonSimpleJsonParser}. * * @author Dave Syer */ -public class SimpleJsonJsonParserTests extends AbstractJsonParserTests { +public class JsonSimpleJsonParserTests extends AbstractJsonParserTests { @Override protected JsonParser getParser() { - return new JsonJsonParser(); + return new JsonSimpleJsonParser(); } }