Skip to content

fix: reject nested objects inside list elements in the query format#147

Open
OmarAlJarrah wants to merge 1 commit into
mainfrom
c5-nesting-guard-and-nullable-tests
Open

fix: reject nested objects inside list elements in the query format#147
OmarAlJarrah wants to merge 1 commit into
mainfrom
c5-nesting-guard-and-nullable-tests

Conversation

@OmarAlJarrah

Copy link
Copy Markdown
Member

Summary

  • QueryListEncoder/QueryListDecoder (the delegates used while a List property is being encoded or decoded) never overrode beginStructure(), so a nested @Serializable object — or a nested list — inside a list element fell through to the AbstractEncoder/AbstractDecoder defaults instead of being rejected. A List<Contact>-shaped property silently flattened its fields into repeated values on encode instead of raising the documented SerializationException, and could throw an unrelated IndexOutOfBoundsException on decode. Both delegates now carry the same "structure begun here is rejected" guard the top-level encoder/decoder already have, so nesting is caught uniformly whether it happens at the top level or inside a list element.
  • The only existing test for a nullable non-collection property used a field with a declared default, so both the encoder's shouldEncodeElementDefault skip and the decoder's isElementOptional short-circuit meant decodeNotNullMark()/encodeNull() were never actually invoked by any test, and the "absent" vs. "present but empty" distinction for a scalar nullable property was unverified end to end. A required (no-default) nullable property now exercises both the absent (decodes to null) and present-but-empty (key= with no value, decodes to "") cases.

Test plan

  • ./gradlew :kuri-serde-kotlinx:jvmTest — confirmed the new nesting-rejection tests fail without the fix and pass with it (TDD)
  • ./gradlew :kuri-serde-kotlinx:ktlintCheck
  • ./gradlew :kuri-serde-kotlinx:detekt
  • ./gradlew :kuri-serde-kotlinx:apiCheck (no public API changes — the touched classes are internal)
  • ./gradlew :kuri-serde-kotlinx:jsNodeTest (commonMain was touched)
  • ./gradlew :kuri-serde-kotlinx:macosArm64Test (native target, since commonMain changed)

Closes #106
Closes #127

QueryListEncoder/QueryListDecoder are the delegates used while a list
property is being encoded or decoded, but unlike the top-level encoder
and decoder they never overrode beginStructure(), so a nested
@serializable object (or a nested list) inside a list element fell
through to the AbstractEncoder/AbstractDecoder defaults instead of
being rejected. A List<Contact>-shaped property silently flattened
its fields into repeated values on encode, and could throw an
unrelated IndexOutOfBoundsException on decode, rather than raising the
documented SerializationException.

Give both list delegates the same "structure begun here is rejected"
guard the top-level encoder/decoder already carry, so nesting is
caught uniformly whether it happens at the top level or inside a list
element.

Also add coverage for the nullable-vs-absent distinction on a
non-collection property: the existing nullable test used a defaulted
field, so both the encoder's shouldEncodeElementDefault skip and the
decoder's isElementOptional short-circuit meant decodeNotNullMark()
and encodeNull() were never actually invoked by any test. A required
(no-default) nullable property now exercises both the absent (decodes
to null) and present-but-empty ("key=" with no value, decodes to "")
cases end to end.

Closes #106, #127
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant