Skip to content

Commit 8ea5c3d

Browse files
xaionaro@dx.centerxaionaro@dx.center
authored andcommitted
fix: type-aware null markers for opaque fields in JavaWireFormat codegen
java_extractor now emits specific write methods (bundle, typed_object, byte_array, blob, binder) instead of generic 'opaque'. The codegen uses these to write correct null markers: - bundle: int32(-1) (writeBundle null protocol) - typed_object: int32(0) (writeTypedObject null protocol) - others: int32(0) with length-based skip on unmarshal
1 parent 495867f commit 8ea5c3d

4 files changed

Lines changed: 50 additions & 26 deletions

File tree

tools/cmd/spec2go/main.go

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -320,14 +320,15 @@ func convertParcelableToAST(
320320
validName := isValidJavaWireFieldName(jwf.Name)
321321
resolvableCond := isResolvableJavaWireCondition(jwf.Condition)
322322
_, knownType := javaWireTypeToAIDL[jwf.WriteMethod]
323-
isOpaque := jwf.WriteMethod == "opaque"
323+
isOpaque := !knownType
324324

325325
// Treat fields with invalid names, unresolvable conditions,
326326
// or unknown types as opaque in the wire format.
327-
if !validName || !resolvableCond || (!knownType && !isOpaque) {
327+
if !validName || !resolvableCond || isOpaque {
328328
wireFields = append(wireFields, parser.JavaWireField{
329329
Name: jwf.Name,
330-
WriteMethod: "opaque",
330+
WriteMethod: jwf.WriteMethod,
331+
Condition: jwf.Condition,
331332
})
332333
continue
333334
}

tools/pkg/codegen/parcelable_gen.go

Lines changed: 38 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -808,9 +808,12 @@ func writeJavaWireMarshalParcel(
808808
for _, wf := range wireFields {
809809
info, ok := javaWireMethodMap[wf.WriteMethod]
810810
if !ok {
811-
// Opaque field (Bundle, Parcelable, etc.) — write null marker.
812-
// Android's writeBundle(null) writes int32(-1).
813-
f.P("\tp.WriteInt32(-1) // null %s", wf.Name)
811+
switch wf.WriteMethod {
812+
case "bundle":
813+
f.P("\tp.WriteInt32(-1) // null %s (Bundle)", wf.Name)
814+
default:
815+
f.P("\tp.WriteInt32(0) // null %s", wf.Name)
816+
}
814817
continue
815818
}
816819

@@ -860,18 +863,38 @@ func writeJavaWireUnmarshalParcel(
860863
info, ok := javaWireMethodMap[wf.WriteMethod]
861864

862865
if !ok {
863-
// Opaque field: skip by reading length-prefixed data.
864-
// Android's writeBundle/writeParcelable writes int32(length)
865-
// followed by <length> bytes.
866-
f.P("\t{")
867-
f.P("\t\t_opaqueLen, _opaqueErr := p.ReadInt32()")
868-
f.P("\t\tif _opaqueErr != nil {")
869-
f.P("\t\t\treturn _opaqueErr")
870-
f.P("\t\t}")
871-
f.P("\t\tif _opaqueLen > 0 {")
872-
f.P("\t\t\tp.SetPosition(p.Position() + int(_opaqueLen))")
873-
f.P("\t\t}")
874-
f.P("\t}")
866+
switch wf.WriteMethod {
867+
case "bundle":
868+
f.P("\t{")
869+
f.P("\t\t_opaqueLen, _opaqueErr := p.ReadInt32()")
870+
f.P("\t\tif _opaqueErr != nil {")
871+
f.P("\t\t\treturn _opaqueErr")
872+
f.P("\t\t}")
873+
f.P("\t\tif _opaqueLen > 0 {")
874+
f.P("\t\t\tp.SetPosition(p.Position() + int(_opaqueLen))")
875+
f.P("\t\t}")
876+
f.P("\t}")
877+
case "typed_object":
878+
f.P("\t{")
879+
f.P("\t\t_opaqueFlag, _opaqueErr := p.ReadInt32()")
880+
f.P("\t\tif _opaqueErr != nil {")
881+
f.P("\t\t\treturn _opaqueErr")
882+
f.P("\t\t}")
883+
f.P("\t\tif _opaqueFlag != 0 {")
884+
f.P("\t\t\treturn nil // non-null %s: cannot skip unknown-size typed object", wf.Name)
885+
f.P("\t\t}")
886+
f.P("\t}")
887+
default:
888+
f.P("\t{")
889+
f.P("\t\t_opaqueLen, _opaqueErr := p.ReadInt32()")
890+
f.P("\t\tif _opaqueErr != nil {")
891+
f.P("\t\t\treturn _opaqueErr")
892+
f.P("\t\t}")
893+
f.P("\t\tif _opaqueLen > 0 {")
894+
f.P("\t\t\tp.SetPosition(p.Position() + int(_opaqueLen))")
895+
f.P("\t\t}")
896+
f.P("\t}")
897+
}
875898
continue
876899
}
877900

tools/pkg/parcelspec/java_extractor.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -73,13 +73,13 @@ var javaWriteMethodToSpecType = map[string]string{
7373
"writeFloat": "float32",
7474
"writeDouble": "float64",
7575
"writeBoolean": "bool",
76-
"writeBundle": "opaque",
77-
"writeParcelable": "opaque",
78-
"writeTypedObject": "opaque",
76+
"writeBundle": "bundle",
77+
"writeParcelable": "typed_object",
78+
"writeTypedObject": "typed_object",
7979
"writeByte": "int32",
80-
"writeByteArray": "opaque",
81-
"writeBlob": "opaque",
82-
"writeStrongBinder": "opaque",
80+
"writeByteArray": "byte_array",
81+
"writeBlob": "blob",
82+
"writeStrongBinder": "binder",
8383
}
8484

8585
// deriveFieldName converts a Java field name to a spec field name

tools/pkg/parcelspec/java_extractor_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ public class AllTypes implements Parcelable {
7676
{"Latitude", "float64"},
7777
{"Label", "string8"},
7878
{"Description", "string16"},
79-
{"Extras", "opaque"},
79+
{"Extras", "bundle"},
8080
}
8181

8282
for i, exp := range expected {
@@ -246,7 +246,7 @@ func TestExtractSpec_Location(t *testing.T) {
246246
// The last field should be Extras (opaque, unconditional).
247247
lastField := spec.Fields[len(spec.Fields)-1]
248248
require.Equal(t, "Extras", lastField.Name)
249-
require.Equal(t, "opaque", lastField.Type)
249+
require.Equal(t, "bundle", lastField.Type)
250250
require.Empty(t, lastField.Condition)
251251
}
252252

0 commit comments

Comments
 (0)