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

Jamel-jun · 2024-06-17T02:17:26Z

Current Behavior

I only used the grpc-transcode plugin.
I customized a protobuf, which contains an int64 type field.

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

APISIX version (run apisix version):
Operating system (run uname -a):
OpenResty / Nginx version (run openresty -V or nginx -V):
etcd version, if relevant (run curl http://127.0.0.1:9090/v1/server_info):
APISIX Dashboard version, if relevant:
Plugin runner version, for issues related to plugin runners:
LuaRocks version, for installation issues (run luarocks --version):

The text was updated successfully, but these errors were encountered:

zhoujiexiong · 2024-06-19T11:00:47Z

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(starwing/lua-protobuf#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

use the subset of the supported patterns while composing the HTTP request body with field[s] of type INT64.
And for the HTTP response body with field[s] of type INT64, using the option int64_as_string can avoid loss of accuracy, but the client site should notice the '#' prefix.
See also:
https://github.com/zhoujiexiong/lua-protobuf?tab=readme-ov-file#options

Note: The string returned by int64_as_string or int64_as_hexstring will prefix a '#' character. Because Lua may convert between string with number, prefix a '#' makes Lua return the string as-is.

all routines in all module accepts '#' prefix string/hex string as arguments regardless of the option setting.

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

github-project-automation bot added this to Apache APISIX backlog Jun 17, 2024

github-project-automation bot moved this to 📋 Backlog in Apache APISIX backlog Jun 17, 2024

Jamel-jun changed the title ~~bug: grpc-transcode protobuf 当有 int64 类型的字段时，传入 a 时，转换成了 10~~ bug: grpc-transcode plugin, in Protobuf, when there is a field of type int64, when 'a' is passed in, it is converted to 10. Jun 17, 2024

zhoujiexiong mentioned this issue Jun 21, 2024

fix(grpc-transcode): filter out illegal INT(string) formats #11367

Merged

5 tasks

moonming closed this as completed in #11367 Jul 9, 2024

github-project-automation bot moved this from 📋 Backlog to ✅ Done in Apache APISIX backlog Jul 9, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

Jamel-jun commented Jun 17, 2024 •

edited

Loading

zhoujiexiong commented Jun 19, 2024

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

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

Comments

Jamel-jun commented Jun 17, 2024 • edited Loading

Current Behavior

Expected Behavior

Error Logs

Steps to Reproduce

Environment

zhoujiexiong commented Jun 19, 2024

BTW & FYI

Some test snippets and outputs

Simulate The Flow of grpc-transcode Plugin

OUTPUT

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

OUTPUT

Jamel-jun commented Jun 17, 2024 •

edited

Loading

Simulate The Flow of `grpc-transcode` Plugin