Skip to content

pxf: (pxf.default) on a repeated field throws ClassCastException out of the decoder #52

Description

@trendvidia

(pxf.default) on a repeated field throws a raw
java.lang.ClassCastException out of the decoder instead of a
PxfException. Same failure class as trendvidia/protowire-go#66.

Repro

message Config {
  repeated string tags = 6 [(pxf.default) = "ignored"];
}

Decode a document that leaves tags absent — FastDecoder.postDecode
reaches applyDefault only on the absent-field path — and the call
unwinds with:

java.lang.ClassCastException: class java.lang.String cannot be cast to
class java.util.List (java.lang.String and java.util.List are in module
java.base of loader 'bootstrap')

Mechanism

FastDecoder.applyDefault switches on fd.getJavaType():

switch (fd.getJavaType()) {
    case STRING  -> b.setField(fd, def);
    …
}

getJavaType() reports the element type and says nothing about
cardinality, so repeated string is STRING and a bare String reaches
Builder.setField. protobuf-java's setField requires a List for a
repeated field and casts unconditionally.

Verified with a probe on DynamicMessage.Builder over
org.protowire.pxf.testproto.AllTypes:

repeated_string javaType=STRING isRepeated=true
setField(repeated_string, "ignored") -> ClassCastException: String cannot be cast to List

Every scalar arm of the switch has the same shape, so the bug is not
string-specific — repeated int32, repeated bool and the rest all
reach setField with a boxed scalar.

(The probe file was removed; the tree is unmodified apart from the
pre-existing gradlew.bat modification, which I did not touch.)

Map fields are fine, but the message is confusing

map<string, string> has getJavaType() == MESSAGE, so it lands in
applyMessageDefault, misses every well-known-type branch, and throws a
proper PxfException — but one naming the synthetic entry type:

default values not supported for message type test.v1.AllTypes.StringMapEntry (field "string_map")

which describes protobuf's map representation rather than the mistake the
schema author made. protowire-go PR #67 replaces the equivalent message
with one naming the placement.

Suggested fix

Guard on cardinality before the getJavaType() dispatch, matching
protowire-go PR #67 and the message protowire-typescript already produces:

if (fd.isMapField()) {
    throw new PxfException(Position.UNKNOWN,
            "default values not supported for map field \"" + fd.getName() + "\"");
}
if (fd.isRepeated()) {
    throw new PxfException(Position.UNKNOWN,
            "default values not supported for repeated field \"" + fd.getName() + "\"");
}

Note isRepeated() is true for map fields too, so the map check must come
first.

Context

The spec says nothing about where (pxf.default) may be placed — that gap
is trendvidia/protowire#223, and whether the check should move to
bind time is trendvidia/protowire-go#68. This issue is narrower and needs
neither settled: whatever the placement rule turns out to be, a decoder
should not leak a ClassCastException from protobuf-java's reflection
layer to its caller.

Port repeated + (pxf.default)
protowire-go (main) panic in protoreflect.Message.Set
protowire-go (PR #67, open) error: default values not supported for repeated field "tags"
protowire-rust silently sets ["ignored"]; pb output changes
protowire-java ClassCastException out of the decoder
protowire-typescript error: default values not supported for list field "tags"

Related: trendvidia/protowire#223, trendvidia/protowire-go#66,
trendvidia/protowire-go#68, trendvidia/chameleon#131.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions