feat: own a SerdeException type and reject lenient scalar coercion#93
Merged
Conversation
…cion Two hardening changes to the serde layer. 1. Stable SPI failure type. The `serde` SPI declared no exception type, so the Jackson adapter let `com.fasterxml.jackson.*` exceptions escape across the zero-dependency `Serde` boundary. Callers coding against the SPI could not catch a stable type, and the abstraction leaked its backing library. Add an open `SerdeException : RuntimeException` to `sdk-core`'s serde package, plus `SerializationException` / `DeserializationException` for the write and read directions. The Jackson adapter now catches `JsonProcessingException` (the root of Jackson's parse/mapping failure hierarchy) at every SPI method and rethrows as the matching SDK type with the original chained as the cause. A genuine stream `IOException` still propagates unchanged, and the buffer overload's bounds checks remain `IndexOutOfBoundsException`. 2. Strict scalar coercion on the default mapper. Jackson's defaults silently reshape mismatched scalars: a wire string "5" coerced into a numeric field, numbers into strings, booleans across types. That masks malformed payloads. The SDK's default `ObjectMapper` now disables `MapperFeature.ALLOW_COERCION_OF_SCALARS` and sets per-type coercion to `Fail` for the cross-shape pairs (string -> int/float/boolean, boolean <-> int, and int/float/boolean -> string), built through `JsonMapper.builder().withCoercionConfig(...)`. Numeric widening (int -> floating-point) and correctly typed payloads are unaffected, and the auto-detect features Kotlin data-class binding relies on are left untouched. This applies only to the SDK default mapper, never to a caller-supplied one. This is a pre-1.0 behaviour change: payloads whose JSON shape does not match the target type now fail instead of binding to a quietly wrong value.
…Exception contract
The strict-coercion config rejected string/boolean into integer fields but had
no rule for a floating-point JSON value into an integer field, so {"n":1.5}
silently bound into an Int as 1. That truncation turns a contract-violating
payload into a quietly wrong value, the exact class of bug the lockdown exists
to prevent. Add CoercionInputShape.Float to the LogicalType.Integer fail list
and pin the rejection with a test.
Also document the failure contract on the SPI itself: the core Serializer /
Deserializer interfaces now carry @throws SerializationException /
DeserializationException notes, and the internal Jackson overrides mirror them,
so a reader of the SPI sees the expected stable failure type rather than
learning it only from the Jackson adapter.
…default mapper Extend the default ObjectMapper's coercion lockdown to two cases the initial rules left open: - boolean -> floating-point is now rejected, matching the existing boolean<->integer rejections, so `true` cannot become `1.0`. - an empty string into an integer/floating-point/boolean field is now rejected. Jackson otherwise coerces `""` to a null/zero scalar, which silently masks a malformed payload. An empty string into a textual field is still accepted — `""` is a legitimate string value. Tests cover each new rejection plus the empty-string-binds-to-string case.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Two serde-robustness changes.
SDK-owned
SerdeException(#22)Introduces
SerdeException(withSerializationException/DeserializationExceptionsubtypes) in theserdeSPI. The Jackson implementation now catches Jackson'sJsonProcessingExceptionhierarchy at the SPI boundary and rethrows as these SDK types with the original cause chained, so Jackson exception types no longer leak across theSerdeabstraction. A genuine streamIOException(not a Jackson type) still propagates unchanged, preserving the caller-owned-stream contract.Reject lenient scalar coercion in the default mapper (#24)
The SDK's default
ObjectMapperpreviously inherited Jackson's lenient scalar coercions, which silently turn malformed payloads into the wrong type. The default mapper now rejects the lenient cases — e.g."5"→int,"1.5"→double,"true"→boolean, and number/boolean→string — while leaving numeric widening and genuinely-typed payloads working. Caller-supplied mappers are untouched. (Configured at build time viaJsonMapper.builder()to avoid the deprecated runtime feature setters.)New tests cover the exception wrapping and each disabled coercion.
SerdeExceptionis added to the API snapshot.Closes #22
Closes #24