Skip to content

Commit

Permalink
fix(plugins) gRPC gateway uri para bool handling (#9180)
Browse files Browse the repository at this point in the history
gRPC gateway should handle the "false" argument as false for boolean parameters.

This change is to be consistent with [grpc-gateway](https://github.com/grpc-ecosystem/grpc-gateway), where by `http://localhost:8090/v1/example/echo?stringv=false&boolean=false&number=1` you get:
```json
{
	"stringv": "false",
	"boolean": false,
	"number": 1
}
```
as echo.

Before this fix, any value passed to a boolean parameter is treated as a string and thus encoded as `true`.

Fix FTI-3335
  • Loading branch information
StarlightIbuki authored Aug 3, 2022
1 parent 090f046 commit 6cd2676
Show file tree
Hide file tree
Showing 6 changed files with 168 additions and 77 deletions.
28 changes: 24 additions & 4 deletions kong/plugins/grpc-gateway/deco.lua
Original file line number Diff line number Diff line change
Expand Up @@ -169,21 +169,42 @@ function deco.new(method, path, protofile)
}, deco)
end

local function get_field_type(typ, field)
local _, _, field_typ = pb.field(typ, field)
return field_typ
end

local function encode_fix(v, typ)
if typ == "bool" then
-- special case for URI parameters
return v and v ~= "0" and v ~= "false"
end

return v
end

--[[
// Set value `v` at `path` in table `t`
// Path contains value address in dot-syntax. For example:
// `path="a.b.c"` would lead to `t[a][b][c] = v`.
]]
local function add_to_table( t, path, v )
local function add_to_table( t, path, v, typ )
local tab = t -- set up pointer to table root
local msg_typ = typ;
for m in re_gmatch( path , "([^.]+)(\\.)?") do
local key, dot = m[1], m[2]
msg_typ = get_field_type(msg_typ, key)

-- not argument that we concern with
if not msg_typ then
return
end

if dot then
tab[key] = tab[key] or {} -- create empty nested table if key does not exist
tab = tab[key]
else
tab[key] = v
tab[key] = encode_fix(v, msg_typ)
end
end

Expand Down Expand Up @@ -241,12 +262,11 @@ function deco:upstream(body)
// For example: `GET /v1/messages/123456?revision=2&sub.subfield=foo`
// translates into `payload = { sub = { subfield = "foo" }}`
]]--
add_to_table( payload, k, v )
add_to_table( payload, k, v, self.endpoint.input_type)
end
end
end
body = grpc_frame(0x0, pb.encode(self.endpoint.input_type, payload))

return body
end

Expand Down
55 changes: 51 additions & 4 deletions spec/03-plugins/28-grpc-gateway/01-proxy_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ for _, strategy in helpers.each_strategy() do
local body = res:read_body()
local data = cjson.decode(body)

assert.same({reply = "hello john_doe"}, data)
assert.same({reply = "hello john_doe", boolean_test = false}, data)
end)

test("additional binding", function()
Expand All @@ -77,19 +77,66 @@ for _, strategy in helpers.each_strategy() do

local data = cjson.decode((res:read_body()))

assert.same({reply = "hello john_doe"}, data)
assert.same({reply = "hello john_doe", boolean_test = false}, data)
end)

test("removes unbound query args", function()
local res, err = proxy_client:get("/v1/messages/john_doe?arg1=1&arg2=2")
local res, err = proxy_client:get("/v1/messages/john_doe?arg1=1&arg2.test=2")

assert.equal(200, res.status)
assert.is_nil(err)

local body = res:read_body()
local data = cjson.decode(body)

assert.same({reply = "hello john_doe"}, data)
assert.same({reply = "hello john_doe", boolean_test = false}, data)
end)

describe("boolean behavior", function ()
test("true", function()
local res, err = proxy_client:get("/v1/messages/legacy/john_doe?boolean_test=true")
assert.equal(200, res.status)
assert.is_nil(err)

local body = res:read_body()
local data = cjson.decode(body)
assert.same({reply = "hello john_doe", boolean_test = true}, data)
end)

test("false", function()
local res, err = proxy_client:get("/v1/messages/legacy/john_doe?boolean_test=false")

assert.equal(200, res.status)
assert.is_nil(err)

local body = res:read_body()
local data = cjson.decode(body)

assert.same({reply = "hello john_doe", boolean_test = false}, data)
end)

test("zero", function()
local res, err = proxy_client:get("/v1/messages/legacy/john_doe?boolean_test=0")

assert.equal(200, res.status)
assert.is_nil(err)

local body = res:read_body()
local data = cjson.decode(body)

assert.same({reply = "hello john_doe", boolean_test = false}, data)
end)

test("non-zero", function()
local res, err = proxy_client:get("/v1/messages/legacy/john_doe?boolean_test=1")
assert.equal(200, res.status)
assert.is_nil(err)

local body = res:read_body()
local data = cjson.decode(body)

assert.same({reply = "hello john_doe", boolean_test = true}, data)
end)
end)

test("unknown path", function()
Expand Down
6 changes: 4 additions & 2 deletions spec/fixtures/grpc/target/grpc-target.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,10 @@ import (
"net"
"time"

pb "target/targetservice"

"google.golang.org/grpc"
"google.golang.org/protobuf/types/known/timestamppb"
pb "target/targetservice"
)

const (
Expand All @@ -22,7 +23,8 @@ type server struct {

func (s *server) SayHello(ctx context.Context, in *pb.HelloRequest) (*pb.HelloResponse, error) {
return &pb.HelloResponse{
Reply: fmt.Sprintf("hello %s", in.GetGreeting()),
Reply: fmt.Sprintf("hello %s", in.GetGreeting()),
BooleanTest: in.GetBooleanTest(),
}, nil
}

Expand Down
150 changes: 83 additions & 67 deletions spec/fixtures/grpc/target/targetservice/targetservice.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions spec/fixtures/grpc/targetservice.proto
Original file line number Diff line number Diff line change
Expand Up @@ -49,10 +49,12 @@ service Bouncer {

message HelloRequest {
string greeting = 1;
bool boolean_test = 2;
}

message HelloResponse {
string reply = 1;
bool boolean_test = 2;
}


Expand Down

0 comments on commit 6cd2676

Please sign in to comment.