apache / apisix

The Cloud-Native API Gateway
https://apisix.apache.org/blog/
Apache License 2.0
14.52k stars 2.52k forks source link

bug: grpc-transcode plugin, in Protobuf, when there is a field of type int64, when 'a' is passed in, it is converted to 10. #11355

Closed Jamel-jun closed 4 months ago

Jamel-jun commented 5 months ago

Current Behavior

I only used the grpc-transcode plugin. I customized a protobuf, which contains an int64 type field. image When I make a call through HTTP, I given in.

{"currency":"MYR","value":"a"}

When parsed to the backend, the value equals 10.

Expected Behavior

Assigning the value as 10 is incorrect; it should only accept numerical types. I want it to throw an exception when a non-numerical type is inputted, for example, a 400 Bad Request error.

Error Logs

No response

Steps to Reproduce

This scene is inevitable

Environment

image

zhoujiexiong commented 5 months ago

Hi @Jamel-jun ,

Maybe there is a minor problem of lua-protobuf while converting integer from a series of string patterns. I have proposed a PR(https://github.com/starwing/lua-protobuf/pull/269) to fix it.


BTW & FYI

It seems that it's the ability of lua-protobuf to support INT string patterns like "#123666666", "#0x+123abc", "+123", "-#123666666", ...

As a contrast, https://pkg.go.dev/mod/google.golang.org/protobuf/encoding/protojson only support a few types, e.g. "123", "-123".

I suggest that we should

Some test snippets and outputs

Simulate The Flow of grpc-transcode Plugin

local pb = require "pb"
local protoc = require "protoc"
local cjson = require("cjson.safe")

pb.option "int64_as_string"
--pb.option "int64_as_hexstring"

assert(protoc:load [[
syntax = "proto3";
message Money {
  string currency = 1;
  repeated int64 values = 2;
}]])

local str_fmt = string.format
local function display_banner(msg)
    print(str_fmt([[

--------------------------------------------------------------------------------
%s
--------------------------------------------------------------------------------]], msg))
end

local req_body = [[
{
    "currency": "MYR",
    "values": [1, 2, -3,
"#123", "0xabF", "#-0x123abcdef", "-#0x123abcdef", "#0x123abc",
"922337203685480", "9.2233720368548e+14", 9.2233720368548e+14]
}]]

display_banner("Simulate The Flow of `grpc-transcode` Plugin")

display_banner("step 1: get http req. body of JSON format")

print(req_body)

display_banner("step 2: unmarshal/deserilize it to Lua object")

local req_body_tbl = cjson.decode(req_body)
if req_body_tbl then
    print(require "serpent".block(req_body_tbl))
end

display_banner("step 3: marshal/serilize the Lua object to PB wireformat then send to upstream")

local bytes = assert(pb.encode("Money", req_body_tbl))
--print(pb.tohex(bytes))
print(ngx.encode_base64(bytes))

display_banner("step 4: recv. PB wireformat from upstream then unmarshal/deserilize it to Lua object")

local data2 = assert(pb.decode("Money", bytes))
print(require "serpent".block(data2))

display_banner("step 5: marshal/serilize the Lua object to JSON wireformat then respond to client")

print(cjson.encode(data2))
OUTPUT
--------------------------------------------------------------------------------
Simulate The Flow of `grpc-transcode` Plugin
--------------------------------------------------------------------------------

--------------------------------------------------------------------------------
step 1: get http req. body of JSON format
--------------------------------------------------------------------------------
{
    "currency": "MYR",
    "values": [1, 2, -3,
"#123", "0xabF", "#-0x123abcdef", "-#0x123abcdef", "#0x123abc",
"922337203685480", "9.2233720368548e+14", 9.2233720368548e+14]
}

--------------------------------------------------------------------------------
step 2: unmarshal/deserilize it to Lua object
--------------------------------------------------------------------------------
{
  currency = "MYR",
  values = {
    1,
    2,
    -3,
    "#123",
    "0xabF",
    "#-0x123abcdef",
    "-#0x123abcdef",
    "#0x123abc",
    "922337203685480",
    "9.2233720368548e+14",
    922337203685480
  } --[[table: 0x08eda328]]
} --[[table: 0x08eda280]]

--------------------------------------------------------------------------------
step 3: marshal/serilize the Lua object to PB wireformat then send to upstream
--------------------------------------------------------------------------------
CgNNWVISPgEC/f//////////AXu/FZHk0OLt/////wGR5NDi7f////8BvPVI6JCO68Xb0QHokI7rxdvRAeiQjuvF29EB

--------------------------------------------------------------------------------
step 4: recv. PB wireformat from upstream then unmarshal/deserilize it to Lua object
--------------------------------------------------------------------------------
{
  currency = "MYR",
  values = {
    1,
    2,
    -3,
    123,
    2751,
    "#-4893429231",
    "#-4893429231",
    1194684,
    "#922337203685480",
    "#922337203685480",
    "#922337203685480"
  } --[[table: 0x08eebde0]]
} --[[table: 0x08eebd60]]

--------------------------------------------------------------------------------
step 5: marshal/serilize the Lua object to JSON wireformat then respond to client
--------------------------------------------------------------------------------
{"currency":"MYR","values":[1,2,-3,123,2751,"#-4893429231","#-4893429231",1194684,"#922337203685480","#922337203685480","#922337203685480"]}

Test/compare the ability of "google.golang.org/protobuf/encoding/protojson"

package foo

import (
    "encoding/base64"
    "testing"

    v1 "github.com/foo/bar/api/baz/service/v1"
    "github.com/stretchr/testify/require"
    "google.golang.org/protobuf/encoding/protojson"
    "google.golang.org/protobuf/proto"
)

// message MoneyWiredFromLuaProtobuf {
//   string currency = 1;
//   repeated int64 values = 2;
// }

// --------------------------------------------------------------------------------
// step 1: get http req. body of JSON format
// --------------------------------------------------------------------------------
// {
//  "currency": "MYR",
//  "values": [1, 2, -3,
// "#123", "0xabF", "#-0x123abcdef", "-#0x123abcdef", "#0x123abc",
// "922337203685480", "9.2233720368548e+14", 9.2233720368548e+14]
// }

func Test_GolangProtojsonUnmarshalWiredFromLuaProtobuf(t *testing.T) {
    money := &v1.MoneyWiredFromLuaProtobuf{}
    wiredFromLuaProtobufB64Encoded := "Ej4BAv3//////////wF7vxWR5NDi7f////8BkeTQ4u3/////Abz1SOiQjuvF29EB6JCO68Xb0QHokI7rxdvRAQoDTVlS"
    data, err := base64.StdEncoding.DecodeString(wiredFromLuaProtobufB64Encoded)
    require.Nil(t, err)
    err = proto.Unmarshal(data, money)
    require.Nil(t, err)
    var expected = int64(922337203685480)
    require.Equal(t, expected, money.Values[8])
    require.Equal(t, expected, money.Values[9])
    require.Equal(t, expected, money.Values[10])
    indented, err := protojson.MarshalOptions{Multiline: true, Indent: "\t"}.Marshal(money)
    require.Nil(t, err)
    t.Log("\n" + string(indented))
}

// message Money {
//   string currency = 1;
//   int64 value = 2;
// }

func Test_GolangProtojsonUnmarshal(t *testing.T) {
    money := &v1.Money{}
    cases := []struct {
        Name       string
        ReqBody    string
        RequireNil bool
    }{
        {
            "hexdecimal string without 0x[X] prefix",
            `{"currency": "MYR", "value": "abcdef"}`, false,
        },
        {
            "hexdecimal string with 0x[X] prefix",
            `{"currency": "MYR", "value": "0xabcef"}`, false,
        },
        {
            "hexdecimal string with 0x[X] prefix",
            `{"currency": "MYR", "value": "#123"}`, false,
        },
        {
            "decimal string",
            `{"currency": "MYR", "value": "123"}`, true,
        },
        {
            "decimal integer",
            `{"currency": "MYR", "value": 123}`, true,
        },
        {
            "decimal(negative) integer",
            `{"currency": "MYR", "value": -123}`, true,
        },
        {
            "decimal(negative) string",
            `{"currency": "MYR", "value": "-123"}`, true,
        },
        {
            "decimal(positive) integer",
            `{"currency": "MYR", "value": +123}`, false,
        },
        {
            "decimal(positive) string",
            `{"currency": "MYR", "value": "+123"}`, false,
        },
        {
            "scientific notation",
            `{"currency": "MYR", "value": 9.2233720368548e+14}`, true,
        },
        {
            "scientific notation string",
            `{"currency": "MYR", "value": "9.2233720368548e+14"}`, true,
        },
    }

    var err error
    for _, c := range cases {
        name := c.Name
        if !c.RequireNil {
            name = "[Unsupported] " + name
        }
        t.Run(name, func(t *testing.T) {
            err = protojson.Unmarshal([]byte(c.ReqBody), money)
            if c.RequireNil {
                require.Nil(t, err)
            } else {
                require.NotNil(t, err)
            }
        })
    }
}

// Local Variables:
// go-test-args: "-v -count=1"
// End:
OUTPUT
go test -v -count=1 -run='Test_GolangProtojsonUnmarshalWiredFromLuaProtobuf|Test_GolangProtojsonUnmarshal' .
=== RUN   Test_GolangProtojsonUnmarshalWiredFromLuaProtobuf
        {
            "currency": "MYR",
            "values": [
                "1",
                "2",
                "-3",
                "123",
                "2751",
                "-4893429231",
                "-4893429231",
                "1194684",
                "922337203685480",
                "922337203685480",
                "922337203685480"
            ]
        }
--- PASS: Test_GolangProtojsonUnmarshalWiredFromLuaProtobuf (0.00s)

=== RUN   Test_GolangProtojsonUnmarshal
=== RUN   Test_GolangProtojsonUnmarshal/[Unsupported]_hexdecimal_string_without_0x[X]_prefix
=== RUN   Test_GolangProtojsonUnmarshal/[Unsupported]_hexdecimal_string_with_0x[X]_prefix
=== RUN   Test_GolangProtojsonUnmarshal/[Unsupported]_hexdecimal_string_with_0x[X]_prefix#01
=== RUN   Test_GolangProtojsonUnmarshal/decimal_string
=== RUN   Test_GolangProtojsonUnmarshal/decimal_integer
=== RUN   Test_GolangProtojsonUnmarshal/decimal(negative)_integer
=== RUN   Test_GolangProtojsonUnmarshal/decimal(negative)_string
=== RUN   Test_GolangProtojsonUnmarshal/[Unsupported]_decimal(positive)_integer
=== RUN   Test_GolangProtojsonUnmarshal/[Unsupported]_decimal(positive)_string
=== RUN   Test_GolangProtojsonUnmarshal/scientific_notation
=== RUN   Test_GolangProtojsonUnmarshal/scientific_notation_string
--- PASS: Test_GolangProtojsonUnmarshal (0.00s)
    --- PASS: Test_GolangProtojsonUnmarshal/[Unsupported]_hexdecimal_string_without_0x[X]_prefix (0.00s)
    --- PASS: Test_GolangProtojsonUnmarshal/[Unsupported]_hexdecimal_string_with_0x[X]_prefix (0.00s)
    --- PASS: Test_GolangProtojsonUnmarshal/[Unsupported]_hexdecimal_string_with_0x[X]_prefix#01 (0.00s)
    --- PASS: Test_GolangProtojsonUnmarshal/decimal_string (0.00s)
    --- PASS: Test_GolangProtojsonUnmarshal/decimal_integer (0.00s)
    --- PASS: Test_GolangProtojsonUnmarshal/decimal(negative)_integer (0.00s)
    --- PASS: Test_GolangProtojsonUnmarshal/decimal(negative)_string (0.00s)
    --- PASS: Test_GolangProtojsonUnmarshal/[Unsupported]_decimal(positive)_integer (0.00s)
    --- PASS: Test_GolangProtojsonUnmarshal/[Unsupported]_decimal(positive)_string (0.00s)
    --- PASS: Test_GolangProtojsonUnmarshal/scientific_notation (0.00s)
    --- PASS: Test_GolangProtojsonUnmarshal/scientific_notation_string (0.00s)
PASS
ok      

Go-Test finished at Wed Jun 19 18:51:11