~bigbes/tarantool

tarantool-protobuf

96ed328e533f9a6eeca34e6691fc72bf967fe124 — Eugene Blikh 3 months ago 43f7b86
json: treat null fields as absent (and Value's null as a real value)

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.
M cmd/protoc-gen-tarantool/internal/gen/inline.go => cmd/protoc-gen-tarantool/internal/gen/inline.go +6 -0
@@ 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()

M examples/expected/full/conformance/conformance_pb.lua => examples/expected/full/conformance/conformance_pb.lua +1 -1
@@ 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

M examples/expected/full/hello/hello_pb.lua => examples/expected/full/hello/hello_pb.lua +12 -12
@@ 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

M examples/expected/full/protobuf_test_messages/proto3/test_messages_proto3_pb.lua => examples/expected/full/protobuf_test_messages/proto3/test_messages_proto3_pb.lua +20 -20
@@ 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

M runtime/pb/codec.lua => runtime/pb/codec.lua +5 -1
@@ 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))

M runtime/pb/json.lua => runtime/pb/json.lua +37 -14
@@ 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).

M test/conformance/known_failures.txt => test/conformance/known_failures.txt +0 -3
@@ 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

M test/conformance_test.lua => test/conformance_test.lua +55 -0
@@ 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,