Skip to content

Commit

Permalink
feat(admin): reject invalid proto (#4750)
Browse files Browse the repository at this point in the history
  • Loading branch information
spacewander authored Aug 6, 2021
1 parent 183351c commit 6cbdaf3
Show file tree
Hide file tree
Showing 3 changed files with 102 additions and 50 deletions.
6 changes: 6 additions & 0 deletions apisix/admin/proto.lua
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ local core = require("apisix.core")
local utils = require("apisix.admin.utils")
local get_routes = require("apisix.router").http_routes
local get_services = require("apisix.http.service").services
local compile_proto = require("apisix.plugins.grpc-transcode.proto").compile_proto
local tostring = tostring


Expand Down Expand Up @@ -53,6 +54,11 @@ local function check_conf(id, conf, need_id)
return nil, {error_msg = "invalid configuration: " .. err}
end

local ok, err = compile_proto(conf.content)
if not ok then
return nil, {error_msg = "invalid content: " .. err}
end

return need_id and id or true
end

Expand Down
41 changes: 26 additions & 15 deletions apisix/plugins/grpc-transcode/proto.lua
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
local core = require("apisix.core")
local config_util = require("apisix.core.config_util")
local protoc = require("protoc")
local pcall = pcall
local protos


Expand All @@ -25,6 +26,26 @@ local lrucache_proto = core.lrucache.new({
})


local function compile_proto(content)
local _p = protoc.new()
-- the loaded proto won't appears in _p.loaded without a file name after lua-protobuf=0.3.2,
-- which means _p.loaded after _p:load(content) is always empty, so we can pass a fake file
-- name to keep the code below unchanged, or we can create our own load function with returning
-- the loaded DescriptorProto table additionally, see more details in
-- https://github.com/apache/apisix/pull/4368
local ok, res = pcall(_p.load, _p, content, "filename for loaded")
if not ok then
return nil, res
end

if not res or not _p.loaded then
return nil, "failed to load proto content"
end

return _p.loaded
end


local function create_proto_obj(proto_id)
if protos.values == nil then
return nil
Expand All @@ -42,24 +63,14 @@ local function create_proto_obj(proto_id)
return nil, "failed to find proto by id: " .. proto_id
end

local _p = protoc.new()
-- the loaded proto won't appears in _p.loaded without a file name after lua-protobuf=0.3.2,
-- which means _p.loaded after _p:load(content) is always empty, so we can pass a fake file
-- name to keep the code below unchanged, or we can create our own load function with returning
-- the loaded DescriptorProto table additionally, see more details in
-- https://github.com/apache/apisix/pull/4368
local res = _p:load(content, "filename for loaded")

if not res or not _p.loaded then
return nil, "failed to load proto content"
end


return _p.loaded
return compile_proto(content)
end


local _M = {version = 0.1}
local _M = {
version = 0.1,
compile_proto = compile_proto,
}


function _M.fetch(proto_id)
Expand Down
105 changes: 70 additions & 35 deletions t/admin/proto.t
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,18 @@ no_root_location();
no_shuffle();
log_level("info");

add_block_preprocessor(sub {
my ($block) = @_;

if (!$block->request) {
$block->set_value("request", "GET /t");
}

if (!$block->no_error_log && !$block->error_log) {
$block->set_value("no_error_log", "[error]\n[alert]");
}
});

run_tests;

__DATA__
Expand All @@ -36,20 +48,20 @@ __DATA__
ngx.HTTP_PUT,
[[{
"content": "syntax = \"proto3\";
package proto;
message HelloRequest{
package proto;
message HelloRequest{
string name = 1;
}
}
message HelloResponse{
message HelloResponse{
int32 code = 1;
string msg = 2;
}
// The greeting service definition.
service Hello {
// Sends a greeting
}
// The greeting service definition.
service Hello {
// Sends a greeting
rpc SayHi (HelloRequest) returns (HelloResponse){}
}"
}"
}]],
[[
{
Expand All @@ -67,12 +79,8 @@ __DATA__
ngx.say("[put proto] code: ", code, " message: ", message)
}
}
--- request
GET /t
--- response_body
[put proto] code: 200 message: passed
--- no_error_log
[error]
Expand All @@ -99,12 +107,8 @@ GET /t
ngx.say("[delete proto] code: ", code, " message: ", message)
}
}
--- request
GET /t
--- response_body
[delete proto] code: 200 message: passed
--- no_error_log
[error]
Expand All @@ -117,21 +121,21 @@ GET /t
local code, message = t('/apisix/admin/proto/2',
ngx.HTTP_PUT,
[[{
"content": "syntax = \"proto3\";
package proto;
message HelloRequest{
string name = 1;
}
"content": "syntax = \"proto3\";
package proto;
message HelloRequest{
string name = 1;
}
message HelloResponse{
int32 code = 1;
string msg = 2;
}
// The greeting service definition.
service Hello {
// Sends a greeting
rpc SayHi (HelloRequest) returns (HelloResponse){}
}"
message HelloResponse{
int32 code = 1;
string msg = 2;
}
// The greeting service definition.
service Hello {
// Sends a greeting
rpc SayHi (HelloRequest) returns (HelloResponse){}
}"
}]],
[[
{
Expand Down Expand Up @@ -194,11 +198,42 @@ GET /t
ngx.say("[delete proto] code: ", code)
}
}
--- request
GET /t
--- response_body
[put proto] code: 200 message: passed
[route refer proto] code: 200 message: passed
[delete proto] code: 400
--- no_error_log
[error]
=== TEST 4: reject invalid proto
--- config
location /t {
content_by_lua_block {
local t = require("lib.test_admin").test
local etcd = require("apisix.core.etcd")
local code, message = t('/apisix/admin/proto/1',
ngx.HTTP_PUT,
[[{
"content": "syntax = \"proto3\";
package proto;
message HelloRequest{
string name = 1;
}
message HelloResponse{
int32 code = 1;
string msg = 1;
}"
}]]
)
if code ~= 200 then
ngx.status = code
end
ngx.say(message)
}
}
--- error_code: 400
--- response_body eval
qr/invalid content:/

0 comments on commit 6cbdaf3

Please sign in to comment.