Skip to content

pxf: (pxf.default) on a oneof member destroys the arm the document chose #53

Description

@trendvidia

A (pxf.default) on a oneof member is applied over the arm the document
chose, destroying the value the document wrote. Silent — the decode
succeeds.

Found while fixing the same bug in protowire-go
(trendvidia/protowire-go#72). Measured in this port directly, not
inferred: a probe drove one protoc-built FileDescriptorSet through
this port's unmarshalFull equivalent. Probe files were removed
afterward; the tree is unmodified.

Repro

message OneDefault {
  oneof choice {
    string a = 1;
    string b = 2 [(pxf.default) = "bbb"];
  }
}
Document Observed Expected
a = "written" b="bbb", case ba is gone a="written", case a
(empty) b="bbb", case b same — correct

One annotated member is enough; this is not about how many defaults a
oneof carries.

Cause

Setting any member of a oneof clears the others, so the per-field reading
of "absent" the post-decode pass uses does not hold inside one: a member
is absent precisely when a sibling was chosen. Applying its default then
clears the sibling.

pxf/src/main/java/org/protowire/pxf/FastDecoder.java, in postDecode: if (def != null) applyDefault(b, fd, def); inside the !isPresent branch

The same root cause breaks (pxf.required)

oneof choice { string a = 1 [(pxf.required) = true]; string b = 2; }

b = "written" — a perfectly valid arm — is rejected with
required field "a" is absent. Read per field, the annotation demands
one specific arm always be chosen, which makes every other arm of the
oneof undecodable. Same presence test, different symptom.

Not a divergence

protowire-go, protowire-java, protowire-rust and protowire-typescript all
behave identically here — byte for byte, both bugs. Unlike
trendvidia/protowire#223 this is one design bug inherited uniformly
rather than several answers to an unasked question, so every port needs
the same change and none has to migrate off a local dialect.

The rule

Now specified: draft -01 §annotation-extensions, "Oneof Members"
(trendvidia/protowire#226, PR trendvidia/protowire#227).

  • pxf.default on a oneof member applies only when no member of the
    oneof is present
    . A member bound to null counts as present,
    consistent with the existing null-suppresses-default rule.
  • At most one member of a oneof may carry pxf.default — a
    bind-time rejection, because with two, declaration order would decide
    which wins.
  • pxf.required MUST NOT be set on a oneof member — a bind-time
    rejection. Its only coherent reading is a property of the oneof, and
    the spec defines no annotation at that scope.

Suggested fix

The runtime half is the part that stops data loss and is worth doing on
its own, independent of when this port adopts the bind-time checks:
consult fd.getContainingOneof(), skipping when any other member of that oneof is present in the recorded presence set.

Read presence from the document, not from the decoded message's
current oneof case — a default applied earlier in the same pass would
itself set the case and let the first annotated member suppress every
later one. protowire-go's fix is trendvidia/protowire-go#73 if a
reference is useful.

Note a proto3 optional field sits in a synthetic single-member
oneof; it must be excluded, since nothing can clear it and its default
must keep applying.

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