cbor: check inline value classes if marked as @ByteString (fixes #2187) #2466
Conversation
the-eater
force-pushed
the
cbor/fix-inline-bytestring
branch
from
6838fd8
to
dfee01c
Compare
| @@ -636,6 +639,11 @@ private fun SerialDescriptor.isByteString(index: Int): Boolean { | |||
| return getElementAnnotations(index).find { it is ByteString } != null | |||
| } | |||
|
|
|||
| private fun SerialDescriptor.isInlineByteString(): Boolean { | |||
| // inline item classes should only have 1 item | |||
| return isInline && isByteString(0) | |||
There was a problem hiding this comment.
Yes, this is a correct way to do it
| @@ -278,6 +280,7 @@ internal open class CborReader(private val cbor: Cbor, protected val decoder: Cb | |||
| @Suppress("UNCHECKED_CAST") | |||
| decoder.nextByteString() as T | |||
| } else { | |||
| decodeByteArrayAsByteString = decodeByteArrayAsByteString || deserializer.descriptor.isInlineByteString() | |||
There was a problem hiding this comment.
This looks a bit suspicious. If the current descriptor is not a ByteArraySerializer().descriptor and not a value class over @ByteString ByteArray, why we should still try to decode it as ByteArray?
If this is to support cases when value class property is not annotated itself, like this:
@Serializable data class Outer(@ByteString val i: InnerValue)
@Serializable
@JvmInline value class InnerValue(val b: ByteArray)Then I don't see tests for it.
There was a problem hiding this comment.
oh good point, yes that was the intended outcome! I'm not sure if that is wanted, but I can see how it might be unwanted? I guess I could probably expose it as an option in the serializer?
I will add the tests anyhow :)
There was a problem hiding this comment.
I'm not sure if that is wanted
I'm also unsure because I didn't see any requests for that. At the first look, it sounds reasonable, but I do not use byte strings that often
the-eater
force-pushed
the
cbor/fix-inline-bytestring
branch
from
35e57ef
to
6292f2c
Compare
This fixed inline value classes not being processed as a ByteString, by checking inside
encodeSerializableValueanddecodeSerializableValueto see if the current descriptor is marked as inline and with@ByteStringthis is as far as I understand the most viable way to do it