From b980770f27cc151aac8ace1898738bfef99dfcc2 Mon Sep 17 00:00:00 2001 From: xxvw Date: Mon, 27 Jul 2026 11:37:02 +0900 Subject: [PATCH 1/2] fix: make nested response values immutable --- CHANGELOG.md | 5 + .../02-models-and-errors.md | 5 +- .../05-json-and-mapping.md | 5 +- .../07-testing-requirements.md | 1 + .../cloudflare/d1/D1MappingException.java | 4 +- .../io/github/xxvw/cloudflare/d1/D1Meta.java | 6 +- .../xxvw/cloudflare/d1/D1RawResult.java | 5 +- .../xxvw/cloudflare/d1/D1ResponseInfo.java | 7 +- .../xxvw/cloudflare/d1/D1ResponseSource.java | 7 +- .../github/xxvw/cloudflare/d1/D1Result.java | 4 +- .../github/xxvw/cloudflare/d1/D1Timings.java | 7 +- .../d1/internal/D1ImmutableValues.java | 91 +++++++++ .../d1/D1NestedImmutabilityTest.java | 176 ++++++++++++++++++ 13 files changed, 293 insertions(+), 30 deletions(-) create mode 100644 src/main/java/io/github/xxvw/cloudflare/d1/internal/D1ImmutableValues.java create mode 100644 src/test/java/io/github/xxvw/cloudflare/d1/D1NestedImmutabilityTest.java diff --git a/CHANGELOG.md b/CHANGELOG.md index 2661563..9738dc2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,11 @@ This project follows Semantic Versioning. ## [Unreleased] +### Fixed + +- Made nested JSON maps and lists in public response and mapping-error models recursively immutable + and defensively copied. + ## [0.2.0] - 2026-07-11 See the [migration guide](docs/guides/migration-0.2.md) for upgrade steps from 0.1.x. diff --git a/docs/implementation-requirements/02-models-and-errors.md b/docs/implementation-requirements/02-models-and-errors.md index 1c51a17..311301a 100644 --- a/docs/implementation-requirements/02-models-and-errors.md +++ b/docs/implementation-requirements/02-models-and-errors.md @@ -44,6 +44,7 @@ Rules: - Missing `results` means empty rows. - `rows()` must return an immutable list. - Each row map must be immutable. +- Nested JSON maps and lists in rows must be recursively immutable and defensively copied. - `firstRow()` must return an immutable map if present. - `messages()` and `errors()` must return immutable lists. - `rawBody()` must contain the raw HTTP response body. @@ -99,6 +100,8 @@ timings = Optional.empty() ``` Unknown meta fields must be preserved in `additionalProperties()`. +Nested JSON maps and lists in `additionalProperties()` must be recursively immutable and +defensively copied. ## D1Timings @@ -228,7 +231,7 @@ public Map row(); Rules: -- `row()` must be immutable. +- `row()` and its nested JSON maps and lists must be immutable and defensively copied. - Default exception message must not include row contents. ## Error Mapping diff --git a/docs/implementation-requirements/05-json-and-mapping.md b/docs/implementation-requirements/05-json-and-mapping.md index 262d93b..4727cfe 100644 --- a/docs/implementation-requirements/05-json-and-mapping.md +++ b/docs/implementation-requirements/05-json-and-mapping.md @@ -104,9 +104,8 @@ JSON array -> List Rows may contain nested `Map` or `List` values. -The outer rows list and each row map must be immutable. - -Deep immutability for nested values is not required in v0.1.0. +The outer rows list, each row map, and nested JSON maps and lists must be immutable and +defensively copied. ## Typed Mapping diff --git a/docs/implementation-requirements/07-testing-requirements.md b/docs/implementation-requirements/07-testing-requirements.md index 20d5ba7..b370ab7 100644 --- a/docs/implementation-requirements/07-testing-requirements.md +++ b/docs/implementation-requirements/07-testing-requirements.md @@ -102,6 +102,7 @@ Required cases: - Rows list is immutable - Row map is immutable +- Nested JSON maps and lists are immutable and defensively copied - Messages list is immutable - Errors list is immutable - additionalProperties map is immutable diff --git a/src/main/java/io/github/xxvw/cloudflare/d1/D1MappingException.java b/src/main/java/io/github/xxvw/cloudflare/d1/D1MappingException.java index 2aa69da..3f61858 100644 --- a/src/main/java/io/github/xxvw/cloudflare/d1/D1MappingException.java +++ b/src/main/java/io/github/xxvw/cloudflare/d1/D1MappingException.java @@ -1,7 +1,7 @@ package io.github.xxvw.cloudflare.d1; +import io.github.xxvw.cloudflare.d1.internal.D1ImmutableValues; import java.util.Collections; -import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.Objects; @@ -30,7 +30,7 @@ public D1MappingException(Class targetType, int rowIndex, Map Collections.emptyList(), Collections.emptyList()); this.targetType = Objects.requireNonNull(targetType, "targetType must not be null"); this.rowIndex = rowIndex; - this.row = row == null ? Collections.emptyMap() : Collections.unmodifiableMap(new LinkedHashMap<>(row)); + this.row = D1ImmutableValues.immutableMap(row); } /** diff --git a/src/main/java/io/github/xxvw/cloudflare/d1/D1Meta.java b/src/main/java/io/github/xxvw/cloudflare/d1/D1Meta.java index 68be6d6..0e2c43e 100644 --- a/src/main/java/io/github/xxvw/cloudflare/d1/D1Meta.java +++ b/src/main/java/io/github/xxvw/cloudflare/d1/D1Meta.java @@ -1,7 +1,7 @@ package io.github.xxvw.cloudflare.d1; +import io.github.xxvw.cloudflare.d1.internal.D1ImmutableValues; import java.util.Collections; -import java.util.LinkedHashMap; import java.util.Map; import java.util.Optional; import java.util.OptionalLong; @@ -66,9 +66,7 @@ public D1Meta( this.servedByRegion = servedByRegion; this.servedByPrimary = servedByPrimary; this.timings = timings; - this.additionalProperties = additionalProperties == null - ? Collections.emptyMap() - : Collections.unmodifiableMap(new LinkedHashMap<>(additionalProperties)); + this.additionalProperties = D1ImmutableValues.immutableMap(additionalProperties); } /** diff --git a/src/main/java/io/github/xxvw/cloudflare/d1/D1RawResult.java b/src/main/java/io/github/xxvw/cloudflare/d1/D1RawResult.java index c8c08d1..3856a5e 100644 --- a/src/main/java/io/github/xxvw/cloudflare/d1/D1RawResult.java +++ b/src/main/java/io/github/xxvw/cloudflare/d1/D1RawResult.java @@ -1,5 +1,6 @@ package io.github.xxvw.cloudflare.d1; +import io.github.xxvw.cloudflare.d1.internal.D1ImmutableValues; import java.util.ArrayList; import java.util.Collections; import java.util.List; @@ -139,9 +140,7 @@ private static List> immutableRows(List> rows) { } List> copy = new ArrayList<>(rows.size()); for (List row : rows) { - copy.add(row == null - ? Collections.emptyList() - : Collections.unmodifiableList(new ArrayList<>(row))); + copy.add(D1ImmutableValues.immutableList(row)); } return Collections.unmodifiableList(copy); } diff --git a/src/main/java/io/github/xxvw/cloudflare/d1/D1ResponseInfo.java b/src/main/java/io/github/xxvw/cloudflare/d1/D1ResponseInfo.java index 4782ac0..9d74966 100644 --- a/src/main/java/io/github/xxvw/cloudflare/d1/D1ResponseInfo.java +++ b/src/main/java/io/github/xxvw/cloudflare/d1/D1ResponseInfo.java @@ -1,7 +1,6 @@ package io.github.xxvw.cloudflare.d1; -import java.util.Collections; -import java.util.LinkedHashMap; +import io.github.xxvw.cloudflare.d1.internal.D1ImmutableValues; import java.util.Map; import java.util.Optional; @@ -34,9 +33,7 @@ public D1ResponseInfo( this.message = message; this.documentationUrl = documentationUrl; this.source = source; - this.additionalProperties = additionalProperties == null - ? Collections.emptyMap() - : Collections.unmodifiableMap(new LinkedHashMap<>(additionalProperties)); + this.additionalProperties = D1ImmutableValues.immutableMap(additionalProperties); } /** diff --git a/src/main/java/io/github/xxvw/cloudflare/d1/D1ResponseSource.java b/src/main/java/io/github/xxvw/cloudflare/d1/D1ResponseSource.java index 4d4c3b1..389b564 100644 --- a/src/main/java/io/github/xxvw/cloudflare/d1/D1ResponseSource.java +++ b/src/main/java/io/github/xxvw/cloudflare/d1/D1ResponseSource.java @@ -1,7 +1,6 @@ package io.github.xxvw.cloudflare.d1; -import java.util.Collections; -import java.util.LinkedHashMap; +import io.github.xxvw.cloudflare.d1.internal.D1ImmutableValues; import java.util.Map; import java.util.Optional; @@ -20,9 +19,7 @@ public final class D1ResponseSource { */ public D1ResponseSource(String pointer, Map additionalProperties) { this.pointer = pointer; - this.additionalProperties = additionalProperties == null - ? Collections.emptyMap() - : Collections.unmodifiableMap(new LinkedHashMap<>(additionalProperties)); + this.additionalProperties = D1ImmutableValues.immutableMap(additionalProperties); } /** diff --git a/src/main/java/io/github/xxvw/cloudflare/d1/D1Result.java b/src/main/java/io/github/xxvw/cloudflare/d1/D1Result.java index d18ce3a..d0dd916 100644 --- a/src/main/java/io/github/xxvw/cloudflare/d1/D1Result.java +++ b/src/main/java/io/github/xxvw/cloudflare/d1/D1Result.java @@ -1,8 +1,8 @@ package io.github.xxvw.cloudflare.d1; +import io.github.xxvw.cloudflare.d1.internal.D1ImmutableValues; import java.util.ArrayList; import java.util.Collections; -import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.Optional; @@ -134,7 +134,7 @@ private static List> immutableRows(List> } List> copy = new ArrayList<>(rows.size()); for (Map row : rows) { - copy.add(Collections.unmodifiableMap(new LinkedHashMap<>(row))); + copy.add(D1ImmutableValues.immutableMap(row)); } return Collections.unmodifiableList(copy); } diff --git a/src/main/java/io/github/xxvw/cloudflare/d1/D1Timings.java b/src/main/java/io/github/xxvw/cloudflare/d1/D1Timings.java index b30cdee..8642317 100644 --- a/src/main/java/io/github/xxvw/cloudflare/d1/D1Timings.java +++ b/src/main/java/io/github/xxvw/cloudflare/d1/D1Timings.java @@ -1,7 +1,6 @@ package io.github.xxvw.cloudflare.d1; -import java.util.Collections; -import java.util.LinkedHashMap; +import io.github.xxvw.cloudflare.d1.internal.D1ImmutableValues; import java.util.Map; /** @@ -19,9 +18,7 @@ public final class D1Timings { */ public D1Timings(double sqlDurationMs, Map additionalProperties) { this.sqlDurationMs = sqlDurationMs; - this.additionalProperties = additionalProperties == null - ? Collections.emptyMap() - : Collections.unmodifiableMap(new LinkedHashMap<>(additionalProperties)); + this.additionalProperties = D1ImmutableValues.immutableMap(additionalProperties); } /** diff --git a/src/main/java/io/github/xxvw/cloudflare/d1/internal/D1ImmutableValues.java b/src/main/java/io/github/xxvw/cloudflare/d1/internal/D1ImmutableValues.java new file mode 100644 index 0000000..ec3b925 --- /dev/null +++ b/src/main/java/io/github/xxvw/cloudflare/d1/internal/D1ImmutableValues.java @@ -0,0 +1,91 @@ +package io.github.xxvw.cloudflare.d1.internal; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.IdentityHashMap; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; + +/** + * Creates immutable defensive copies of JSON-shaped maps and lists. + */ +public final class D1ImmutableValues { + private static final String CYCLIC_VALUES_MESSAGE = + "cyclic map/list values are not supported"; + + private D1ImmutableValues() {} + + public static Map immutableMap(Map values) { + if (values == null || values.isEmpty()) { + return Collections.emptyMap(); + } + return immutableStringMap(values, new IdentityHashMap()); + } + + public static List immutableList(List values) { + if (values == null || values.isEmpty()) { + return Collections.emptyList(); + } + return immutableObjectList(values, new IdentityHashMap()); + } + + private static Map immutableStringMap( + Map values, IdentityHashMap visiting) { + enter(values, visiting); + try { + Map copy = new LinkedHashMap<>(); + for (Map.Entry entry : values.entrySet()) { + copy.put(entry.getKey(), immutableValue(entry.getValue(), visiting)); + } + return Collections.unmodifiableMap(copy); + } finally { + visiting.remove(values); + } + } + + private static Map immutableObjectMap( + Map values, IdentityHashMap visiting) { + enter(values, visiting); + try { + Map copy = new LinkedHashMap<>(); + for (Map.Entry entry : values.entrySet()) { + copy.put(entry.getKey(), immutableValue(entry.getValue(), visiting)); + } + return Collections.unmodifiableMap(copy); + } finally { + visiting.remove(values); + } + } + + private static List immutableObjectList( + List values, IdentityHashMap visiting) { + enter(values, visiting); + try { + List copy = new ArrayList<>(values.size()); + for (Object value : values) { + copy.add(immutableValue(value, visiting)); + } + return Collections.unmodifiableList(copy); + } finally { + visiting.remove(values); + } + } + + private static Object immutableValue( + Object value, IdentityHashMap visiting) { + if (value instanceof Map) { + return immutableObjectMap((Map) value, visiting); + } + if (value instanceof List) { + return immutableObjectList((List) value, visiting); + } + return value; + } + + private static void enter(Object value, IdentityHashMap visiting) { + if (visiting.put(value, Boolean.TRUE) != null) { + throw new IllegalArgumentException(CYCLIC_VALUES_MESSAGE); + } + } +} diff --git a/src/test/java/io/github/xxvw/cloudflare/d1/D1NestedImmutabilityTest.java b/src/test/java/io/github/xxvw/cloudflare/d1/D1NestedImmutabilityTest.java new file mode 100644 index 0000000..09592ed --- /dev/null +++ b/src/test/java/io/github/xxvw/cloudflare/d1/D1NestedImmutabilityTest.java @@ -0,0 +1,176 @@ +package io.github.xxvw.cloudflare.d1; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; +import org.junit.jupiter.api.Test; + +class D1NestedImmutabilityTest { + + @Test + void resultRowsAreRecursivelyImmutableAndDefensivelyCopied() { + NestedValue input = nestedValue(); + Map row = new LinkedHashMap<>(); + row.put("profile", input.map); + D1Result result = new D1Result( + true, + Collections.singletonList(row), + D1Meta.empty(), + Collections.emptyList(), + Collections.emptyList(), + "{}"); + + input.list.add("changed"); + input.map.put("changed", true); + row.put("changed", true); + + Map exposedRow = result.rows().get(0); + Map exposedProfile = nestedMap(exposedRow.get("profile")); + List exposedTags = nestedList(exposedProfile.get("tags")); + + assertThat(exposedRow).doesNotContainKey("changed"); + assertThat(exposedProfile).doesNotContainKey("changed"); + assertThat(exposedTags).containsExactly("one"); + assertThatThrownBy(() -> exposedProfile.put("added", true)) + .isInstanceOf(UnsupportedOperationException.class); + assertThatThrownBy(() -> exposedTags.add("two")) + .isInstanceOf(UnsupportedOperationException.class); + } + + @Test + void rawRowsAreRecursivelyImmutableAndDefensivelyCopied() { + NestedValue input = nestedValue(); + List row = new ArrayList<>(); + row.add(input.map); + D1RawResult result = new D1RawResult( + true, + Collections.singletonList("profile"), + Collections.singletonList(row), + D1Meta.empty(), + Collections.emptyList(), + Collections.emptyList(), + "{}"); + + input.list.add("changed"); + input.map.put("changed", true); + row.add("changed"); + + List exposedRow = result.rows().get(0); + Map exposedProfile = nestedMap(exposedRow.get(0)); + List exposedTags = nestedList(exposedProfile.get("tags")); + + assertThat(exposedRow).hasSize(1); + assertThat(exposedProfile).doesNotContainKey("changed"); + assertThat(exposedTags).containsExactly("one"); + assertThatThrownBy(() -> exposedTags.add("two")) + .isInstanceOf(UnsupportedOperationException.class); + } + + @Test + void additionalPropertiesAreRecursivelyImmutableAndDefensivelyCopied() { + NestedValue input = nestedValue(); + D1Meta meta = new D1Meta( + false, 0, null, 0, 0, 0.0, null, null, null, null, null, input.map); + D1Timings timings = new D1Timings(1.0, input.map); + D1ResponseInfo info = new D1ResponseInfo(1, "message", null, null, input.map); + D1ResponseSource source = new D1ResponseSource("/result", input.map); + + input.list.add("changed"); + input.map.put("changed", true); + + for (Map properties : Arrays.asList( + meta.additionalProperties(), + timings.additionalProperties(), + info.additionalProperties(), + source.additionalProperties())) { + assertThat(properties).doesNotContainKey("changed"); + assertThat(nestedList(properties.get("tags"))).containsExactly("one"); + assertThatThrownBy(() -> nestedList(properties.get("tags")).add("two")) + .isInstanceOf(UnsupportedOperationException.class); + } + } + + @Test + void mappingExceptionRowIsRecursivelyImmutableAndDefensivelyCopied() { + NestedValue input = nestedValue(); + Map row = new LinkedHashMap<>(); + row.put("profile", input.map); + D1MappingException exception = + new D1MappingException(String.class, 2, row, new IllegalArgumentException("mapping")); + + input.list.add("changed"); + input.map.put("changed", true); + row.put("changed", true); + + Map exposedRow = exception.row(); + Map exposedProfile = nestedMap(exposedRow.get("profile")); + + assertThat(exposedRow).doesNotContainKey("changed"); + assertThat(exposedProfile).doesNotContainKey("changed"); + assertThat(nestedList(exposedProfile.get("tags"))).containsExactly("one"); + assertThatThrownBy(() -> nestedList(exposedProfile.get("tags")).add("two")) + .isInstanceOf(UnsupportedOperationException.class); + } + + @Test + void cyclicMapAndListValuesAreRejected() { + Map cyclicMap = new LinkedHashMap<>(); + cyclicMap.put("self", cyclicMap); + List cyclicList = new ArrayList<>(); + cyclicList.add(cyclicList); + + assertThatThrownBy(() -> new D1Result( + true, + Collections.singletonList(cyclicMap), + D1Meta.empty(), + Collections.emptyList(), + Collections.emptyList(), + "{}")) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("cyclic map/list values are not supported"); + assertThatThrownBy(() -> new D1RawResult( + true, + Collections.singletonList("value"), + Collections.singletonList(cyclicList), + D1Meta.empty(), + Collections.emptyList(), + Collections.emptyList(), + "{}")) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("cyclic map/list values are not supported"); + } + + private static NestedValue nestedValue() { + List list = new ArrayList<>(); + list.add("one"); + Map map = new LinkedHashMap<>(); + map.put("tags", list); + return new NestedValue(map, list); + } + + @SuppressWarnings("unchecked") + private static Map nestedMap(Object value) { + return (Map) value; + } + + @SuppressWarnings("unchecked") + private static List nestedList(Object value) { + return (List) value; + } + + private static final class NestedValue { + private final Map map; + private final List list; + + private NestedValue(Map map, List list) { + this.map = map; + this.list = list; + } + } +} From f6e88bbf19c5269a3f519d26e8a5e7e5b09d177d Mon Sep 17 00:00:00 2001 From: xxvw Date: Mon, 27 Jul 2026 11:52:30 +0900 Subject: [PATCH 2/2] docs: centralize unreleased changelog entries --- CHANGELOG.md | 5 ----- 1 file changed, 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9738dc2..2661563 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,11 +6,6 @@ This project follows Semantic Versioning. ## [Unreleased] -### Fixed - -- Made nested JSON maps and lists in public response and mapping-error models recursively immutable - and defensively copied. - ## [0.2.0] - 2026-07-11 See the [migration guide](docs/guides/migration-0.2.md) for upgrade steps from 0.1.x.