From 96ed328e533f9a6eeca34e6691fc72bf967fe124 Mon Sep 17 00:00:00 2001 From: Eugene Blikh Date: Sat, 16 May 2026 00:53:54 +0300 Subject: [PATCH] json: treat null fields as absent (and Value's null as a real value) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per the proto3 JSON spec, a null on any field means "use the field's default" — encoded as missing — with the lone exception of google.protobuf.Value, where JSON null is itself a Value carrying NullValue.NULL_VALUE. Three coupled bugs surfaced together: 1. decode_field_value used to fall through with v = box.NULL, leaving a useless box.NULL sitting in the result table for scalars. Now it returns nil for non-Value fields, PB_NULL for Value fields. 2. decode_message's repeated and map branches called `#jv` and `pairs(jv)` unconditionally; a JSON-null on either type crashed with "attempt to get length of 'void *'". Now both branches short-circuit when jv is box.NULL. 3. The nil-skip checks in the decode loop (`if dv ~= nil`) and in the codec / inline message encoders (`if v == nil then return end`) evaluated TRUE on box.NULL because Tarantool's cdata __eq aliases it to nil. Decode now uses rawequal(dv, nil); encode special-cases message kind by also accepting cdata, so the Value field's box.NULL sentinel survives all the way through to value_encode. Drops 3 entries from test/conformance/known_failures.txt (AllFieldAcceptNull, WrapperTypesWithNullValue, ValueAcceptNull). Adds 5 regression tests covering scalar / repeated / map / wrapper null treatment and the Value-NULL_VALUE exception. --- .../internal/gen/inline.go | 6 ++ .../full/conformance/conformance_pb.lua | 2 +- examples/expected/full/hello/hello_pb.lua | 24 ++++---- .../proto3/test_messages_proto3_pb.lua | 40 +++++++------- runtime/pb/codec.lua | 6 +- runtime/pb/json.lua | 51 ++++++++++++----- test/conformance/known_failures.txt | 3 - test/conformance_test.lua | 55 +++++++++++++++++++ 8 files changed, 136 insertions(+), 51 deletions(-) diff --git a/cmd/protoc-gen-tarantool/internal/gen/inline.go b/cmd/protoc-gen-tarantool/internal/gen/inline.go index e4d4815fd8a02580a107ed75ad37bfb6595d2afa..17408cb65a6a1c3cbb4590f9eaec2912c8ede496 100644 --- a/cmd/protoc-gen-tarantool/internal/gen/inline.go +++ b/cmd/protoc-gen-tarantool/internal/gen/inline.go @@ -95,9 +95,15 @@ func emitInlineEncodeField(w *writer, f *protogen.Field, file *protogen.File, se w.line(" v = t.%s", fname) oneof := fieldRealOneof(f) + // Default presence gate: a regular nil check. For message fields we + // also need to accept box.NULL (which == nil via Tarantool's cdata + // metamethod) because google.protobuf.Value uses it as the canonical + // null_value sentinel. gate := "v ~= nil" if oneof != "" { gate = fmt.Sprintf("%s == %q", oneofVar(oneof), fname) + } else if f.Message != nil { + gate = "v ~= nil or type(v) == 'cdata'" } // Explicit-optional fields: presence is meaningful, no default elision. hasPresence := oneof != "" || f.Desc.HasOptionalKeyword() diff --git a/examples/expected/full/conformance/conformance_pb.lua b/examples/expected/full/conformance/conformance_pb.lua index 82d01aa4f736e84be6bc9bfd37e9cc236d59e574..150c6b8d088b0475e2e569c044741d6543f4e870 100644 --- a/examples/expected/full/conformance/conformance_pb.lua +++ b/examples/expected/full/conformance/conformance_pb.lua @@ -345,7 +345,7 @@ function M.ConformanceRequest_encode(t) end -- field 6: jspb_encoding_options v = t.jspb_encoding_options - if v ~= nil then + if v ~= nil or type(v) == 'cdata' then n = n + 1; out[n] = "\x32" n = n + 1; out[n] = wire.encode_len(M.JspbEncodingConfig_encode(v)) end diff --git a/examples/expected/full/hello/hello_pb.lua b/examples/expected/full/hello/hello_pb.lua index de44968b8f65baf213d6453260b18593668e082d..0bfef1adae3efafd1a829d4f21bd5afe406671b6 100644 --- a/examples/expected/full/hello/hello_pb.lua +++ b/examples/expected/full/hello/hello_pb.lua @@ -385,67 +385,67 @@ function M.Event_encode(t) end -- field 2: created_at v = t.created_at - if v ~= nil then + if v ~= nil or type(v) == 'cdata' then n = n + 1; out[n] = "\x12" n = n + 1; out[n] = wire.encode_len(pb.wkt.Timestamp_encode(v)) end -- field 3: duration v = t.duration - if v ~= nil then + if v ~= nil or type(v) == 'cdata' then n = n + 1; out[n] = "\x1a" n = n + 1; out[n] = wire.encode_len(pb.wkt.Duration_encode(v)) end -- field 4: ack v = t.ack - if v ~= nil then + if v ~= nil or type(v) == 'cdata' then n = n + 1; out[n] = "\x22" n = n + 1; out[n] = wire.encode_len(pb.wkt.Empty_encode(v)) end -- field 5: retry_count v = t.retry_count - if v ~= nil then + if v ~= nil or type(v) == 'cdata' then n = n + 1; out[n] = "\x2a" n = n + 1; out[n] = wire.encode_len(pb.wkt.Int32Value_encode(v)) end -- field 6: note v = t.note - if v ~= nil then + if v ~= nil or type(v) == 'cdata' then n = n + 1; out[n] = "\x32" n = n + 1; out[n] = wire.encode_len(pb.wkt.StringValue_encode(v)) end -- field 7: is_admin v = t.is_admin - if v ~= nil then + if v ~= nil or type(v) == 'cdata' then n = n + 1; out[n] = "\x3a" n = n + 1; out[n] = wire.encode_len(pb.wkt.BoolValue_encode(v)) end -- field 8: payload v = t.payload - if v ~= nil then + if v ~= nil or type(v) == 'cdata' then n = n + 1; out[n] = "\x42" n = n + 1; out[n] = wire.encode_len(pb.wkt.Struct_encode(v)) end -- field 9: attribute v = t.attribute - if v ~= nil then + if v ~= nil or type(v) == 'cdata' then n = n + 1; out[n] = "\x4a" n = n + 1; out[n] = wire.encode_len(pb.wkt.Value_encode(v)) end -- field 10: tags v = t.tags - if v ~= nil then + if v ~= nil or type(v) == 'cdata' then n = n + 1; out[n] = "\x52" n = n + 1; out[n] = wire.encode_len(pb.wkt.ListValue_encode(v)) end -- field 11: extension v = t.extension - if v ~= nil then + if v ~= nil or type(v) == 'cdata' then n = n + 1; out[n] = "\x5a" n = n + 1; out[n] = wire.encode_len(pb.wkt.Any_encode(v)) end -- field 12: update_mask v = t.update_mask - if v ~= nil then + if v ~= nil or type(v) == 'cdata' then n = n + 1; out[n] = "\x62" n = n + 1; out[n] = wire.encode_len(pb.wkt.FieldMask_encode(v)) end @@ -674,7 +674,7 @@ function M.Person_encode(t) end -- field 5: address v = t.address - if v ~= nil then + if v ~= nil or type(v) == 'cdata' then n = n + 1; out[n] = "\x2a" n = n + 1; out[n] = wire.encode_len(M.Address_encode(v)) end diff --git a/examples/expected/full/protobuf_test_messages/proto3/test_messages_proto3_pb.lua b/examples/expected/full/protobuf_test_messages/proto3/test_messages_proto3_pb.lua index 36aea0fb7c9307638e5ca1503091af545a00eaaf..18c58a2d178b1c30849435b8805b7a6c83f37e91 100644 --- a/examples/expected/full/protobuf_test_messages/proto3/test_messages_proto3_pb.lua +++ b/examples/expected/full/protobuf_test_messages/proto3/test_messages_proto3_pb.lua @@ -523,13 +523,13 @@ function M.TestAllTypesProto3_encode(t) end -- field 18: optional_nested_message v = t.optional_nested_message - if v ~= nil then + if v ~= nil or type(v) == 'cdata' then n = n + 1; out[n] = "\x92\x01" n = n + 1; out[n] = wire.encode_len(M.TestAllTypesProto3_NestedMessage_encode(v)) end -- field 19: optional_foreign_message v = t.optional_foreign_message - if v ~= nil then + if v ~= nil or type(v) == 'cdata' then n = n + 1; out[n] = "\x9a\x01" n = n + 1; out[n] = wire.encode_len(M.ForeignMessage_encode(v)) end @@ -586,7 +586,7 @@ function M.TestAllTypesProto3_encode(t) end -- field 27: recursive_message v = t.recursive_message - if v ~= nil then + if v ~= nil or type(v) == 'cdata' then n = n + 1; out[n] = "\xda\x01" n = n + 1; out[n] = wire.encode_len(M.TestAllTypesProto3_encode(v)) end @@ -1509,55 +1509,55 @@ function M.TestAllTypesProto3_encode(t) end -- field 201: optional_bool_wrapper v = t.optional_bool_wrapper - if v ~= nil then + if v ~= nil or type(v) == 'cdata' then n = n + 1; out[n] = "\xca\x0c" n = n + 1; out[n] = wire.encode_len(pb.wkt.BoolValue_encode(v)) end -- field 202: optional_int32_wrapper v = t.optional_int32_wrapper - if v ~= nil then + if v ~= nil or type(v) == 'cdata' then n = n + 1; out[n] = "\xd2\x0c" n = n + 1; out[n] = wire.encode_len(pb.wkt.Int32Value_encode(v)) end -- field 203: optional_int64_wrapper v = t.optional_int64_wrapper - if v ~= nil then + if v ~= nil or type(v) == 'cdata' then n = n + 1; out[n] = "\xda\x0c" n = n + 1; out[n] = wire.encode_len(pb.wkt.Int64Value_encode(v)) end -- field 204: optional_uint32_wrapper v = t.optional_uint32_wrapper - if v ~= nil then + if v ~= nil or type(v) == 'cdata' then n = n + 1; out[n] = "\xe2\x0c" n = n + 1; out[n] = wire.encode_len(pb.wkt.UInt32Value_encode(v)) end -- field 205: optional_uint64_wrapper v = t.optional_uint64_wrapper - if v ~= nil then + if v ~= nil or type(v) == 'cdata' then n = n + 1; out[n] = "\xea\x0c" n = n + 1; out[n] = wire.encode_len(pb.wkt.UInt64Value_encode(v)) end -- field 206: optional_float_wrapper v = t.optional_float_wrapper - if v ~= nil then + if v ~= nil or type(v) == 'cdata' then n = n + 1; out[n] = "\xf2\x0c" n = n + 1; out[n] = wire.encode_len(pb.wkt.FloatValue_encode(v)) end -- field 207: optional_double_wrapper v = t.optional_double_wrapper - if v ~= nil then + if v ~= nil or type(v) == 'cdata' then n = n + 1; out[n] = "\xfa\x0c" n = n + 1; out[n] = wire.encode_len(pb.wkt.DoubleValue_encode(v)) end -- field 208: optional_string_wrapper v = t.optional_string_wrapper - if v ~= nil then + if v ~= nil or type(v) == 'cdata' then n = n + 1; out[n] = "\x82\x0d" n = n + 1; out[n] = wire.encode_len(pb.wkt.StringValue_encode(v)) end -- field 209: optional_bytes_wrapper v = t.optional_bytes_wrapper - if v ~= nil then + if v ~= nil or type(v) == 'cdata' then n = n + 1; out[n] = "\x8a\x0d" n = n + 1; out[n] = wire.encode_len(pb.wkt.BytesValue_encode(v)) end @@ -1644,37 +1644,37 @@ function M.TestAllTypesProto3_encode(t) end -- field 301: optional_duration v = t.optional_duration - if v ~= nil then + if v ~= nil or type(v) == 'cdata' then n = n + 1; out[n] = "\xea\x12" n = n + 1; out[n] = wire.encode_len(pb.wkt.Duration_encode(v)) end -- field 302: optional_timestamp v = t.optional_timestamp - if v ~= nil then + if v ~= nil or type(v) == 'cdata' then n = n + 1; out[n] = "\xf2\x12" n = n + 1; out[n] = wire.encode_len(pb.wkt.Timestamp_encode(v)) end -- field 303: optional_field_mask v = t.optional_field_mask - if v ~= nil then + if v ~= nil or type(v) == 'cdata' then n = n + 1; out[n] = "\xfa\x12" n = n + 1; out[n] = wire.encode_len(pb.wkt.FieldMask_encode(v)) end -- field 304: optional_struct v = t.optional_struct - if v ~= nil then + if v ~= nil or type(v) == 'cdata' then n = n + 1; out[n] = "\x82\x13" n = n + 1; out[n] = wire.encode_len(pb.wkt.Struct_encode(v)) end -- field 305: optional_any v = t.optional_any - if v ~= nil then + if v ~= nil or type(v) == 'cdata' then n = n + 1; out[n] = "\x8a\x13" n = n + 1; out[n] = wire.encode_len(pb.wkt.Any_encode(v)) end -- field 306: optional_value v = t.optional_value - if v ~= nil then + if v ~= nil or type(v) == 'cdata' then n = n + 1; out[n] = "\x92\x13" n = n + 1; out[n] = wire.encode_len(pb.wkt.Value_encode(v)) end @@ -1693,7 +1693,7 @@ function M.TestAllTypesProto3_encode(t) end -- field 308: optional_empty v = t.optional_empty - if v ~= nil then + if v ~= nil or type(v) == 'cdata' then n = n + 1; out[n] = "\xa2\x13" n = n + 1; out[n] = wire.encode_len(pb.wkt.Empty_encode(v)) end @@ -3568,7 +3568,7 @@ function M.TestAllTypesProto3_NestedMessage_encode(t) end -- field 2: corecursive v = t.corecursive - if v ~= nil then + if v ~= nil or type(v) == 'cdata' then n = n + 1; out[n] = "\x12" n = n + 1; out[n] = wire.encode_len(M.TestAllTypesProto3_encode(v)) end diff --git a/runtime/pb/codec.lua b/runtime/pb/codec.lua index 97749cc731c170fa9c456899fd47f994315cb939..d0d635477b885eae7d7b7e49d9088009008c6267 100644 --- a/runtime/pb/codec.lua +++ b/runtime/pb/codec.lua @@ -433,7 +433,11 @@ local function build_writer(f) local tag_bytes = wire.encode_tag(f.id, wire.WIRE_LEN) return function(data, out) local v = data[fname] - if v == nil then return end + -- box.NULL equals nil under Tarantool's cdata metamethod, but + -- google.protobuf.Value uses box.NULL as the canonical + -- null_value sentinel — its sub_desc.encode handles it. Only + -- treat the field as absent if it's actually nil (no cdata). + if v == nil and type(v) ~= 'cdata' then return end local n = #out out[n + 1] = tag_bytes out[n + 2] = wire.encode_len(encode_msg(sub_desc, v)) diff --git a/runtime/pb/json.lua b/runtime/pb/json.lua index ca0931b8167de8da2a18ed4946bd301dd20f668e..a23cf69a03a36b39a39525f332e6ed9ae9d33315 100644 --- a/runtime/pb/json.lua +++ b/runtime/pb/json.lua @@ -371,6 +371,18 @@ local function decode_enum(enum_desc, v) end local function decode_field_value(field, v) + if v == box.NULL then + -- proto3 JSON: a null value means "use the field's default" — i.e. + -- the field is treated as absent. The lone exception is + -- google.protobuf.Value, where null is itself a valid value + -- (NullValue.NULL_VALUE); pass through so json_to_value returns + -- PB_NULL. + if field.kind == 'message' and field.message + and field.message.name == 'google.protobuf.Value' then + return PB_NULL + end + return nil + end local kind = field.kind if kind == 'scalar' then return decode_scalar(field.proto_type, v) elseif kind == 'enum' then return decode_enum(field.enum, v) @@ -518,26 +530,37 @@ decode_message = function(desc, v) for k, jv in pairs(v) do local f = field_by_json_name[k] if f ~= nil then - if f.kind == 'map' then - local m = {} - for mk, mv in pairs(jv) do - local dv = decode_field_value(f.value, mv) - if dv ~= nil then - m[decode_map_key(f.key, mk)] = dv + if jv == box.NULL and (f.kind ~= 'message' + or f.message == nil + or f.message.name ~= 'google.protobuf.Value') then + -- proto3 JSON: null on any non-Value field means "use the + -- default" — i.e. leave the field unset. + elseif f.kind == 'map' then + if jv ~= box.NULL then + local m = {} + for mk, mv in pairs(jv) do + local dv = decode_field_value(f.value, mv) + if not rawequal(dv, nil) then + m[decode_map_key(f.key, mk)] = dv + end end + out[f.name] = m end - out[f.name] = m elseif f.repeated then - local arr = {} - local n = 0 - for i = 1, #jv do - local dv = decode_field_value(f, jv[i]) - if dv ~= nil then n = n + 1; arr[n] = dv end + if jv ~= box.NULL then + local arr = {} + local n = 0 + for i = 1, #jv do + local dv = decode_field_value(f, jv[i]) + if not rawequal(dv, nil) then + n = n + 1; arr[n] = dv + end + end + out[f.name] = arr end - out[f.name] = arr else local dv = decode_field_value(f, jv) - if dv ~= nil then out[f.name] = dv end + if not rawequal(dv, nil) then out[f.name] = dv end end end -- Unknown JSON keys are silently ignored (per spec). diff --git a/test/conformance/known_failures.txt b/test/conformance/known_failures.txt index 229c94eee22d78cfebe868a2e41bf058a72291e8..26abafd630c15ede928a5110d0c276fb5cba7390 100644 --- a/test/conformance/known_failures.txt +++ b/test/conformance/known_failures.txt @@ -21,7 +21,6 @@ Recommended.Proto3.JsonInput.FieldNameWithDoubleUnderscores.ProtobufOutput Recommended.Proto3.JsonInput.NullValueInOtherOneofOldFormat.Validator Required.Proto3.JsonInput.AllFieldAcceptNull.JsonOutput -Required.Proto3.JsonInput.AllFieldAcceptNull.ProtobufOutput Required.Proto3.JsonInput.AnyNested.JsonOutput Required.Proto3.JsonInput.AnyWithInt32ValueWrapper.JsonOutput Required.Proto3.JsonInput.AnyWithStruct.JsonOutput @@ -34,7 +33,5 @@ Required.Proto3.JsonInput.Int64FieldMinValueNotQuoted.JsonOutput Required.Proto3.JsonInput.Int64FieldMinValueNotQuoted.ProtobufOutput Required.Proto3.JsonInput.Uint64FieldMaxValueNotQuoted.JsonOutput Required.Proto3.JsonInput.Uint64FieldMaxValueNotQuoted.ProtobufOutput -Required.Proto3.JsonInput.ValueAcceptNull.ProtobufOutput Required.Proto3.JsonInput.WrapperTypesWithNullValue.JsonOutput -Required.Proto3.JsonInput.WrapperTypesWithNullValue.ProtobufOutput Required.Proto3.TimestampProtoNegativeNanos.JsonOutput diff --git a/test/conformance_test.lua b/test/conformance_test.lua index 4859cdd8676fd7b01ef5b1a5b75b9719b81e7283..73b60354c5b38cd23deeb8df5c05187cd25f33a5 100644 --- a/test/conformance_test.lua +++ b/test/conformance_test.lua @@ -634,6 +634,61 @@ core_g.test_bytes_field_accepts_arbitrary_bytes = function() t.assert_not(resp.parse_error, resp.parse_error) end +-- ========================================================================= +-- Fix 9: JSON null on a field is "use default" (drop the field), except +-- for google.protobuf.Value where null is itself a Value (NullValue). +-- Compounded by Tarantool's box.NULL aliasing to nil under __eq, so the +-- decode-side `if dv ~= nil` checks must use rawequal, and the encode- +-- side message field check must accept cdata box.NULL. +-- ========================================================================= + +core_g.test_null_scalar_fields_treated_as_default = function() + -- Every primitive type set to null in JSON should produce an empty + -- protobuf payload (default elision on encode). + local resp = json_to_pb([[{ + "optionalInt32": null, + "optionalInt64": null, + "optionalUint32": null, + "optionalBool": null, + "optionalString": null + }]]) + t.assert_not(resp.parse_error, resp.parse_error) + t.assert_equals(resp.protobuf_payload, '') +end + +core_g.test_null_repeated_field_treated_as_empty = function() + -- A repeated field set to null must not crash on #jv (it isn't an + -- array). The field stays empty. + local resp = json_to_pb([[{"repeatedInt32": null}]]) + t.assert_not(resp.parse_error, resp.parse_error) + t.assert_equals(resp.protobuf_payload, '') +end + +core_g.test_null_map_field_treated_as_empty = function() + -- A map field set to null must not crash on pairs(jv). Stays empty. + local resp = json_to_pb([[{"mapStringString": null}]]) + t.assert_not(resp.parse_error, resp.parse_error) + t.assert_equals(resp.protobuf_payload, '') +end + +core_g.test_null_wrapper_treated_as_absent = function() + -- proto3 JSON: null in a *Value wrapper field means the wrapper is + -- absent, NOT a wrapper containing 0/empty/false. Empty payload. + local resp = json_to_pb([[{"optionalInt32Wrapper": null}]]) + t.assert_not(resp.parse_error, resp.parse_error) + t.assert_equals(resp.protobuf_payload, '') +end + +core_g.test_null_value_field_emits_null_value_member = function() + -- google.protobuf.Value is the exception: JSON null IS a value + -- (NullValue.NULL_VALUE). The field must emit tag(value, LEN) + the + -- inner Value's bytes ('\x08\x00' = field 1 VARINT 0). + -- field 306 = optional_value. tag = (306<<3)|2 = 2450 → varint 0x92 0x13. + local resp = json_to_pb([[{"optionalValue": null}]]) + t.assert_not(resp.parse_error, resp.parse_error) + t.assert_equals(resp.protobuf_payload, '\x92\x13\x02\x08\x00') +end + core_g.test_oneof_merge_still_clears_sibling_branches = function() -- The post-fix merge code must still clear oneof siblings: setting -- oneof_uint32 first, then merging two oneof_nested_message entries,