diff --git a/src/main/java/dev/toonformat/jtoon/DecodeOptions.java b/src/main/java/dev/toonformat/jtoon/DecodeOptions.java index 452676e..05c19b4 100644 --- a/src/main/java/dev/toonformat/jtoon/DecodeOptions.java +++ b/src/main/java/dev/toonformat/jtoon/DecodeOptions.java @@ -1,5 +1,7 @@ package dev.toonformat.jtoon; +import java.util.Objects; + /** * Configuration options for decoding TOON format to Java objects. * @@ -22,6 +24,11 @@ public record DecodeOptions( */ public static final DecodeOptions DEFAULT = new DecodeOptions(2, Delimiter.COMMA, true, PathExpansion.OFF); + /** + * Maximum allowed indent to prevent memory exhaustion attacks. + */ + public static final int MAX_ALLOWED_INDENT = 100; + /** * Creates DecodeOptions with default values. */ @@ -29,6 +36,19 @@ public DecodeOptions() { this(2, Delimiter.COMMA, true, PathExpansion.OFF); } + /** + * Compact constructor with validation. + */ + public DecodeOptions { + if (indent < 0) { + throw new IllegalArgumentException("indent must be non-negative, got: " + indent); + } + if (indent > MAX_ALLOWED_INDENT) { + throw new IllegalArgumentException("indent must be <= " + MAX_ALLOWED_INDENT + ", got: " + indent); + } + delimiter = Objects.requireNonNull(delimiter, "delimiter cannot be null"); + } + /** * Creates DecodeOptions with custom indent, using default delimiter and strict * mode. diff --git a/src/main/java/dev/toonformat/jtoon/EncodeOptions.java b/src/main/java/dev/toonformat/jtoon/EncodeOptions.java index c522826..3ce1ff1 100644 --- a/src/main/java/dev/toonformat/jtoon/EncodeOptions.java +++ b/src/main/java/dev/toonformat/jtoon/EncodeOptions.java @@ -1,5 +1,7 @@ package dev.toonformat.jtoon; +import java.util.Objects; + /** * Configuration options for encoding data to JToon format. * @@ -26,6 +28,11 @@ public record EncodeOptions( public static final EncodeOptions DEFAULT = new EncodeOptions( 2, Delimiter.COMMA, false, KeyFolding.OFF, Integer.MAX_VALUE); + /** + * Maximum allowed indent to prevent memory exhaustion attacks. + */ + public static final int MAX_ALLOWED_INDENT = 100; + /** * Creates EncodeOptions with default values. */ @@ -33,6 +40,22 @@ public EncodeOptions() { this(2, Delimiter.COMMA, false, KeyFolding.OFF, Integer.MAX_VALUE); } + /** + * Compact constructor with validation. + */ + public EncodeOptions { + if (indent < 0) { + throw new IllegalArgumentException("indent must be non-negative, got: " + indent); + } + if (indent > MAX_ALLOWED_INDENT) { + throw new IllegalArgumentException("indent must be <= " + MAX_ALLOWED_INDENT + ", got: " + indent); + } + delimiter = Objects.requireNonNull(delimiter, "delimiter cannot be null"); + if (flattenDepth < 0) { + throw new IllegalArgumentException("flattenDepth must be non-negative, got: " + flattenDepth); + } + } + /** * Creates EncodeOptions with custom indent, using default delimiter and length * marker. diff --git a/src/main/java/dev/toonformat/jtoon/normalizer/JsonNormalizer.java b/src/main/java/dev/toonformat/jtoon/normalizer/JsonNormalizer.java index c1cf3d6..e6345ef 100644 --- a/src/main/java/dev/toonformat/jtoon/normalizer/JsonNormalizer.java +++ b/src/main/java/dev/toonformat/jtoon/normalizer/JsonNormalizer.java @@ -27,6 +27,7 @@ import java.util.Calendar; import java.util.Collection; import java.util.Date; +import java.util.IdentityHashMap; import java.util.List; import java.util.Map; import java.util.Objects; @@ -45,6 +46,15 @@ public final class JsonNormalizer { */ public static final ObjectMapper MAPPER = ObjectMapperSingleton.getInstance(); + /** + * maximal allowed nesting depth of list. + */ + public static final int MAX_ALLOWED_NESTING_DEPTH = 512; + + private static final ThreadLocal DEPTH_COUNTER = ThreadLocal.withInitial(() -> 0); + private static final ThreadLocal> VISITED = + ThreadLocal.withInitial(IdentityHashMap::new); + private static final List> NORMALIZERS = List.of( JsonNormalizer::tryNormalizePrimitive, JsonNormalizer::tryNormalizeBigNumber, @@ -88,20 +98,45 @@ public static JsonNode parse(final String json) { * * @param value The value to normalize * @return The normalized JsonNode + * @throws IllegalArgumentException if nesting depth exceeds MAX_DEPTH or circular reference detected */ public static JsonNode normalize(final Object value) { + final int currentDepth = DEPTH_COUNTER.get(); + if (currentDepth > MAX_ALLOWED_NESTING_DEPTH) { + DEPTH_COUNTER.remove(); + throw new IllegalArgumentException("Maximum nesting depth exceeded: " + MAX_ALLOWED_NESTING_DEPTH); + } + DEPTH_COUNTER.set(currentDepth + 1); + try { + return normalizeInternal(value); + } finally { + DEPTH_COUNTER.set(currentDepth); + } + } + + private static JsonNode normalizeInternal(final Object value) { if (value == null) { return NullNode.getInstance(); } else if (value instanceof JsonNode jsonNode) { return jsonNode; - } else if (value instanceof Optional) { - return normalize(((Optional) value).orElse(null)); - } else if (value instanceof Stream) { - return normalize(((Stream) value).toList()); - } else if (value.getClass().isArray()) { - return normalizeArray(value); - } else { - return normalizeWithStrategy(value); + } + final Map visited = VISITED.get(); + if (visited.containsKey(value)) { + throw new IllegalArgumentException("Circular reference detected"); + } + visited.put(value, Boolean.TRUE); + try { + if (value instanceof Optional) { + return normalize(((Optional) value).orElse(null)); + } else if (value instanceof Stream) { + return normalize(((Stream) value).toList()); + } else if (value.getClass().isArray()) { + return normalizeArray(value); + } else { + return normalizeWithStrategy(value); + } + } finally { + visited.remove(value); } } @@ -296,60 +331,58 @@ private static JsonNode tryNormalizePojo(final Object value) { * Uses direct array population to avoid IntFunction lambda allocations. */ private static JsonNode normalizeArray(final Object array) { - if (array instanceof int[] intArr) { + if (array instanceof int[] intArray) { final ArrayNode node = MAPPER.createArrayNode(); - for (int i = 0; i < intArr.length; i++) { - node.add(IntNode.valueOf(intArr[i])); + for (int i : intArray) { + node.add(IntNode.valueOf(i)); } return node; - } else if (array instanceof long[] longArr) { + } else if (array instanceof long[] longArray) { final ArrayNode node = MAPPER.createArrayNode(); - for (int i = 0; i < longArr.length; i++) { - node.add(LongNode.valueOf(longArr[i])); + for (long l : longArray) { + node.add(LongNode.valueOf(l)); } return node; - } else if (array instanceof double[] doubleArr) { + } else if (array instanceof double[] doubleArray) { final ArrayNode node = MAPPER.createArrayNode(); - for (int i = 0; i < doubleArr.length; i++) { - final double val = doubleArr[i]; - node.add(Double.isFinite(val) ? DoubleNode.valueOf(val) : NullNode.getInstance()); + for (final double d : doubleArray) { + node.add(Double.isFinite(d) ? DoubleNode.valueOf(d) : NullNode.getInstance()); } return node; - } else if (array instanceof float[] floatArr) { + } else if (array instanceof float[] floatArray) { final ArrayNode node = MAPPER.createArrayNode(); - for (int i = 0; i < floatArr.length; i++) { - final float val = floatArr[i]; - node.add(Float.isFinite(val) ? FloatNode.valueOf(val) : NullNode.getInstance()); + for (final float f : floatArray) { + node.add(Float.isFinite(f) ? FloatNode.valueOf(f) : NullNode.getInstance()); } return node; - } else if (array instanceof boolean[] boolArr) { + } else if (array instanceof boolean[] boolArray) { final ArrayNode node = MAPPER.createArrayNode(); - for (int i = 0; i < boolArr.length; i++) { - node.add(BooleanNode.valueOf(boolArr[i])); + for (boolean b : boolArray) { + node.add(BooleanNode.valueOf(b)); } return node; - } else if (array instanceof byte[] byteArr) { + } else if (array instanceof byte[] byteArray) { final ArrayNode node = MAPPER.createArrayNode(); - for (int i = 0; i < byteArr.length; i++) { - node.add(IntNode.valueOf(byteArr[i])); + for (byte by : byteArray) { + node.add(IntNode.valueOf(by)); } return node; - } else if (array instanceof short[] shortArr) { + } else if (array instanceof short[] shortArray) { final ArrayNode node = MAPPER.createArrayNode(); - for (int i = 0; i < shortArr.length; i++) { - node.add(ShortNode.valueOf(shortArr[i])); + for (short s : shortArray) { + node.add(ShortNode.valueOf(s)); } return node; - } else if (array instanceof char[] charArr) { + } else if (array instanceof char[] charArray) { final ArrayNode node = MAPPER.createArrayNode(); - for (int i = 0; i < charArr.length; i++) { - node.add(StringNode.valueOf(String.valueOf(charArr[i]))); + for (char c : charArray) { + node.add(StringNode.valueOf(String.valueOf(c))); } return node; - } else if (array instanceof Object[] objArr) { + } else if (array instanceof Object[] objArray) { final ArrayNode node = MAPPER.createArrayNode(); - for (int i = 0; i < objArr.length; i++) { - node.add(normalize(objArr[i])); + for (Object o : objArray) { + node.add(normalize(o)); } return node; } else { diff --git a/src/main/java/dev/toonformat/jtoon/util/StringEscaper.java b/src/main/java/dev/toonformat/jtoon/util/StringEscaper.java index 20abf4d..b31c96a 100644 --- a/src/main/java/dev/toonformat/jtoon/util/StringEscaper.java +++ b/src/main/java/dev/toonformat/jtoon/util/StringEscaper.java @@ -126,6 +126,7 @@ public static String unescape(final String value) { * * @param c The character following a backslash * @return The unescaped character + * @throws IllegalArgumentException if the escape sequence is invalid */ private static char unescapeChar(final char c) { return switch (c) { @@ -134,7 +135,7 @@ private static char unescapeChar(final char c) { case 't' -> '\t'; case '"' -> '"'; case '\\' -> '\\'; - default -> c; + default -> throw new IllegalArgumentException("Invalid escape sequence: \\" + c); }; } } diff --git a/src/test/java/dev/toonformat/jtoon/SecurityValidationTest.java b/src/test/java/dev/toonformat/jtoon/SecurityValidationTest.java new file mode 100644 index 0000000..11119bb --- /dev/null +++ b/src/test/java/dev/toonformat/jtoon/SecurityValidationTest.java @@ -0,0 +1,87 @@ +package dev.toonformat.jtoon; + +import dev.toonformat.jtoon.normalizer.JsonNormalizer; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Nested; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.*; + +class SecurityValidationTest { + + @Nested + @DisplayName("EncodeOptions validation") + class EncodeOptionsValidation { + @Test + @DisplayName("should reject negative indent") + void testNegativeIndent() { + assertThrows(IllegalArgumentException.class, + () -> new EncodeOptions(-1, Delimiter.COMMA, false, KeyFolding.OFF, 10)); + } + + @Test + @DisplayName("should reject indent exceeding MAX_INDENT") + void testExcessiveIndent() { + assertThrows(IllegalArgumentException.class, + () -> new EncodeOptions(EncodeOptions.MAX_ALLOWED_INDENT + 1, Delimiter.COMMA, false, KeyFolding.OFF, 10)); + } + + @Test + @DisplayName("should reject null delimiter") + void testNullDelimiter() { + assertThrows(NullPointerException.class, + () -> new EncodeOptions(2, null, false, KeyFolding.OFF, 10)); + } + + @Test + @DisplayName("should reject negative flattenDepth") + void testNegativeFlattenDepth() { + assertThrows(IllegalArgumentException.class, + () -> new EncodeOptions(2, Delimiter.COMMA, false, KeyFolding.SAFE, -1)); + } + + @Test + @DisplayName("should accept valid options") + void testValidOptions() { + EncodeOptions opts = new EncodeOptions(4, Delimiter.PIPE, true, KeyFolding.SAFE, 5); + assertEquals(4, opts.indent()); + assertEquals(Delimiter.PIPE, opts.delimiter()); + assertEquals(5, opts.flattenDepth()); + } + } + + @Nested + @DisplayName("DecodeOptions validation") + class DecodeOptionsValidation { + @Test + @DisplayName("should reject negative indent") + void testNegativeIndent() { + assertThrows(IllegalArgumentException.class, + () -> new DecodeOptions(-1, Delimiter.COMMA, true, PathExpansion.OFF)); + } + + @Test + @DisplayName("should reject indent exceeding MAX_INDENT") + void testExcessiveIndent() { + assertThrows(IllegalArgumentException.class, + () -> new DecodeOptions(DecodeOptions.MAX_ALLOWED_INDENT + 1, Delimiter.COMMA, true, PathExpansion.OFF)); + } + + @Test + @DisplayName("should reject null delimiter") + void testNullDelimiter() { + assertThrows(NullPointerException.class, + () -> new DecodeOptions(2, null, true, PathExpansion.OFF)); + } + } + + @Nested + @DisplayName("JsonNormalizer depth limits") + class JsonNormalizerDepthLimits { + @Test + @DisplayName("should have MAX_DEPTH constant") + void testMaxDepthConstant() { + assertEquals(512, JsonNormalizer.MAX_ALLOWED_NESTING_DEPTH); + } + } +} diff --git a/src/test/java/dev/toonformat/jtoon/normalizer/JsonNormalizerTest.java b/src/test/java/dev/toonformat/jtoon/normalizer/JsonNormalizerTest.java index a13c328..81b11f0 100644 --- a/src/test/java/dev/toonformat/jtoon/normalizer/JsonNormalizerTest.java +++ b/src/test/java/dev/toonformat/jtoon/normalizer/JsonNormalizerTest.java @@ -1939,5 +1939,145 @@ void parseEmptyString() { } } + + @Nested + @DisplayName("Security - Depth Limits") + class SecurityDepthLimits { + + @Test + @DisplayName("MAX_ALLOWED_NESTING_DEPTH constant should be 512") + void maxDepthConstantIs512() { + assertEquals(512, JsonNormalizer.MAX_ALLOWED_NESTING_DEPTH); + } + + @Test + @DisplayName("Should throw when nesting depth exceeds MAX_DEPTH") + void throwsWhenDepthExceedsMax() { + // Given - create deeply nested structure that exceeds MAX_DEPTH + // We'll use reflection to test this by creating a custom scenario + // For practical testing, we verify the constant exists and logic works + Map deepMap = new HashMap<>(); + Map current = deepMap; + for (int i = 0; i < 600; i++) { + Map next = new HashMap<>(); + next.put("value", "test"); + current.put("nested", next); + current = next; + } + + // When/Then - should throw due to depth limit + assertThrows(IllegalArgumentException.class, () -> JsonNormalizer.normalize(deepMap)); + } + + @Test + @DisplayName("Should include MAX_DEPTH in exception message") + void exceptionMessageIncludesMaxDepth() { + Map deepMap = new HashMap<>(); + Map current = deepMap; + for (int i = 0; i < 600; i++) { + Map next = new HashMap<>(); + current.put("nested", next); + current = next; + } + + IllegalArgumentException thrown = assertThrows( + IllegalArgumentException.class, + () -> JsonNormalizer.normalize(deepMap) + ); + + assertTrue(thrown.getMessage().contains("512")); + assertTrue(thrown.getMessage().contains("nesting depth")); + } + } + + @Nested + @DisplayName("Security - Circular Reference Detection") + class SecurityCircularReference { + + @Test + @DisplayName("Should detect circular reference in Map") + void detectsCircularMapReference() { + // Given - create circular reference in Map + Map map1 = new HashMap<>(); + Map map2 = new HashMap<>(); + map1.put("key", map2); + map2.put("key", map1); // circular! + + // When/Then + IllegalArgumentException thrown = assertThrows( + IllegalArgumentException.class, + () -> JsonNormalizer.normalize(map1) + ); + + assertTrue(thrown.getMessage().contains("Circular reference")); + } + + @Test + @DisplayName("Should detect circular reference in List") + void detectsCircularListReference() { + // Given - create circular reference in List + List list1 = new java.util.ArrayList<>(); + List list2 = new java.util.ArrayList<>(); + list1.add(list2); + list2.add(list1); // circular! + + // When/Then + IllegalArgumentException thrown = assertThrows( + IllegalArgumentException.class, + () -> JsonNormalizer.normalize(list1) + ); + + assertTrue(thrown.getMessage().contains("Circular reference")); + } + + @Test + @DisplayName("Should detect self-referential object") + void detectsSelfReference() { + // Given - self-referential object + Map map = new HashMap<>(); + map.put("self", map); + + // When/Then + IllegalArgumentException thrown = assertThrows( + IllegalArgumentException.class, + () -> JsonNormalizer.normalize(map) + ); + + assertTrue(thrown.getMessage().contains("Circular reference")); + } + } + + @Nested + @DisplayName("Security - Stream Handling") + class SecurityStreamHandling { + + @Test + @DisplayName("Stream should be materialized to List") + void streamMaterializedToList() { + // Given + Stream stream = Stream.of("a", "b", "c"); + + // When + JsonNode result = JsonNormalizer.normalize(stream); + + // Then + assertInstanceOf(ArrayNode.class, result); + assertEquals(3, result.size()); + } + + @Test + @DisplayName("Empty stream should return empty array") + void emptyStreamReturnsEmptyArray() { + // Given + Stream stream = Stream.empty(); + + // When + JsonNode result = JsonNormalizer.normalize(stream); + + // Then + assertInstanceOf(ArrayNode.class, result); + assertEquals(0, result.size()); + } + } } diff --git a/src/test/java/dev/toonformat/jtoon/util/StringEscaperTest.java b/src/test/java/dev/toonformat/jtoon/util/StringEscaperTest.java index 68a7b1b..3582954 100644 --- a/src/test/java/dev/toonformat/jtoon/util/StringEscaperTest.java +++ b/src/test/java/dev/toonformat/jtoon/util/StringEscaperTest.java @@ -241,10 +241,9 @@ void testNoEscapeSequences() { } @Test - @DisplayName("should handle unknown escape sequences as literals") + @DisplayName("should reject invalid escape sequences") void testUnknownEscapeSequences() { - // Then - assertEquals("ax", StringEscaper.unescape("\\ax")); + assertThrows(IllegalArgumentException.class, () -> StringEscaper.unescape("\\ax")); } @Test