From 5c759941d57c5b744c8ef98f20cda2c3909aa646 Mon Sep 17 00:00:00 2001 From: zhongwencool Date: Tue, 21 May 2024 09:18:07 +0800 Subject: [PATCH 1/3] feat: return 415 when UNSUPPORTED_MEDIA_TYPE --- apps/emqx/include/http_api.hrl | 3 +- apps/emqx_bridge/src/emqx_bridge_api.erl | 4 +- .../src/emqx_dashboard_swagger.erl | 79 ++++++++++++++----- .../src/emqx_dashboard_sso.app.src | 2 +- .../src/emqx_dashboard_sso_saml_api.erl | 7 +- .../src/emqx_license_http_api.erl | 4 +- .../test/emqx_mgmt_api_banned_SUITE.erl | 39 +++++++++ .../test/emqx_mgmt_api_test_util.erl | 6 +- .../src/emqx_rule_engine_api.erl | 4 +- .../test/emqx_rule_engine_api_2_SUITE.erl | 37 ++++++++- 10 files changed, 154 insertions(+), 31 deletions(-) diff --git a/apps/emqx/include/http_api.hrl b/apps/emqx/include/http_api.hrl index 256b1c689..275ffce4a 100644 --- a/apps/emqx/include/http_api.hrl +++ b/apps/emqx/include/http_api.hrl @@ -86,5 +86,6 @@ {'SOURCE_ERROR', <<"Source error">>}, {'UPDATE_FAILED', <<"Update failed">>}, {'REST_FAILED', <<"Reset source or config failed">>}, - {'CLIENT_NOT_RESPONSE', <<"Client not responding">>} + {'CLIENT_NOT_RESPONSE', <<"Client not responding">>}, + {'UNSUPPORTED_MEDIA_TYPE', <<"Unsupported media type">>} ]). diff --git a/apps/emqx_bridge/src/emqx_bridge_api.erl b/apps/emqx_bridge/src/emqx_bridge_api.erl index 5a862c492..2d2c1455c 100644 --- a/apps/emqx_bridge/src/emqx_bridge_api.erl +++ b/apps/emqx_bridge/src/emqx_bridge_api.erl @@ -82,7 +82,9 @@ namespace() -> "bridge". api_spec() -> - emqx_dashboard_swagger:spec(?MODULE, #{check_schema => false}). + emqx_dashboard_swagger:spec(?MODULE, #{ + check_schema => fun emqx_dashboard_swagger:is_content_type_json/2 + }). paths() -> [ diff --git a/apps/emqx_dashboard/src/emqx_dashboard_swagger.erl b/apps/emqx_dashboard/src/emqx_dashboard_swagger.erl index 4ada5994c..308ee3d40 100644 --- a/apps/emqx_dashboard/src/emqx_dashboard_swagger.erl +++ b/apps/emqx_dashboard/src/emqx_dashboard_swagger.erl @@ -30,6 +30,7 @@ -export([base_path/0]). -export([relative_uri/1, get_relative_uri/1]). -export([compose_filters/2]). +-export([is_content_type_json/2, validate_content_type/3]). -export([ filter_check_request/2, @@ -152,6 +153,30 @@ spec(Module, Options) -> ), {ApiSpec, components(lists:usort(AllRefs), Options)}. +is_content_type_json(Params, Meta) -> + validate_content_type(Params, Meta, <<"application/json">>). + +%% tip: Skip content-type check if body is empty. +validate_content_type(#{body := Body} = Params, #{method := Method}, Expect) when + (Method =:= put orelse + Method =:= post orelse + method =:= patch) andalso + Body =/= #{} +-> + ExpectSize = byte_size(Expect), + case find_content_type(Params) of + <> -> + {ok, Params}; + _ -> + {415, 'UNSUPPORTED_MEDIA_TYPE', <<"content-type:", Expect/binary, " Required">>} + end; +validate_content_type(Params, _Meta, _Expect) -> + {ok, Params}. + +find_content_type(#{headers := #{<<"content-type">> := Type}}) -> Type; +find_content_type(#{headers := #{<<"Content-Type">> := Type}}) -> Type; +find_content_type(_Headers) -> error. + -spec namespace() -> hocon_schema:name(). namespace() -> "public". @@ -341,8 +366,12 @@ translate_req(Request, #{module := Module, path := Path, method := Method}, Chec Params = maps:get(parameters, Spec, []), Body = maps:get('requestBody', Spec, []), {Bindings, QueryStr} = check_parameters(Request, Params, Module), - NewBody = check_request_body(Request, Body, Module, CheckFun, hoconsc:is_schema(Body)), - {ok, Request#{bindings => Bindings, query_string => QueryStr, body => NewBody}} + case check_request_body(Request, Body, Module, CheckFun, hoconsc:is_schema(Body)) of + {ok, NewBody} -> + {ok, Request#{bindings => Bindings, query_string => QueryStr, body => NewBody}}; + Error -> + Error + end catch throw:HoconError -> Msg = hocon_error_msg(HoconError), @@ -523,16 +552,23 @@ unwrap_array_conv([HVal | _], _Opts) -> HVal; unwrap_array_conv(SingleVal, _Opts) -> SingleVal. check_request_body(#{body := Body}, Schema, Module, CheckFun, true) -> - Type0 = hocon_schema:field_schema(Schema, type), - Type = - case Type0 of - ?REF(StructName) -> ?R_REF(Module, StructName); - _ -> Type0 - end, - NewSchema = ?INIT_SCHEMA#{roots => [{root, Type}]}, - Option = #{required => false}, - #{<<"root">> := NewBody} = CheckFun(NewSchema, #{<<"root">> => Body}, Option), - NewBody; + %% the body was already being decoded + %% if the content-type header specified application/json. + case is_binary(Body) of + false -> + Type0 = hocon_schema:field_schema(Schema, type), + Type = + case Type0 of + ?REF(StructName) -> ?R_REF(Module, StructName); + _ -> Type0 + end, + NewSchema = ?INIT_SCHEMA#{roots => [{root, Type}]}, + Option = #{required => false}, + #{<<"root">> := NewBody} = CheckFun(NewSchema, #{<<"root">> => Body}, Option), + {ok, NewBody}; + true -> + {415, 'UNSUPPORTED_MEDIA_TYPE', <<"content-type:application/json Required">>} + end; %% TODO not support nest object check yet, please use ref! %% 'requestBody' = [ {per_page, mk(integer(), #{}}, %% {nest_object, [ @@ -541,18 +577,19 @@ check_request_body(#{body := Body}, Schema, Module, CheckFun, true) -> %% ]} %% ] check_request_body(#{body := Body}, Spec, _Module, CheckFun, false) when is_list(Spec) -> - lists:foldl( - fun({Name, Type}, Acc) -> - Schema = ?INIT_SCHEMA#{roots => [{Name, Type}]}, - maps:merge(Acc, CheckFun(Schema, Body, #{})) - end, - #{}, - Spec - ); + {ok, + lists:foldl( + fun({Name, Type}, Acc) -> + Schema = ?INIT_SCHEMA#{roots => [{Name, Type}]}, + maps:merge(Acc, CheckFun(Schema, Body, #{})) + end, + #{}, + Spec + )}; %% requestBody => #{content => #{ 'application/octet-stream' => %% #{schema => #{ type => string, format => binary}}} check_request_body(#{body := Body}, Spec, _Module, _CheckFun, false) when is_map(Spec) -> - Body. + {ok, Body}. %% tags, description, summary, security, deprecated meta_to_spec(Meta, Module, Options) -> diff --git a/apps/emqx_dashboard_sso/src/emqx_dashboard_sso.app.src b/apps/emqx_dashboard_sso/src/emqx_dashboard_sso.app.src index 7f5e8f04a..0ed5d8025 100644 --- a/apps/emqx_dashboard_sso/src/emqx_dashboard_sso.app.src +++ b/apps/emqx_dashboard_sso/src/emqx_dashboard_sso.app.src @@ -1,6 +1,6 @@ {application, emqx_dashboard_sso, [ {description, "EMQX Dashboard Single Sign-On"}, - {vsn, "0.1.4"}, + {vsn, "0.1.5"}, {registered, [emqx_dashboard_sso_sup]}, {applications, [ kernel, diff --git a/apps/emqx_dashboard_sso/src/emqx_dashboard_sso_saml_api.erl b/apps/emqx_dashboard_sso/src/emqx_dashboard_sso_saml_api.erl index ec9396828..ce5d89930 100644 --- a/apps/emqx_dashboard_sso/src/emqx_dashboard_sso_saml_api.erl +++ b/apps/emqx_dashboard_sso/src/emqx_dashboard_sso_saml_api.erl @@ -38,7 +38,12 @@ namespace() -> "dashboard_sso". api_spec() -> - emqx_dashboard_swagger:spec(?MODULE, #{check_schema => false, translate_body => false}). + emqx_dashboard_swagger:spec(?MODULE, #{ + translate_body => false, + check_schema => fun(Params, Meta) -> + emqx_dashboard_swagger:validate_content_type(Params, Meta, <<"application/xml">>) + end + }). paths() -> [ diff --git a/apps/emqx_license/src/emqx_license_http_api.erl b/apps/emqx_license/src/emqx_license_http_api.erl index 31d185651..9265d65ec 100644 --- a/apps/emqx_license/src/emqx_license_http_api.erl +++ b/apps/emqx_license/src/emqx_license_http_api.erl @@ -28,7 +28,9 @@ namespace() -> "license_http_api". api_spec() -> - emqx_dashboard_swagger:spec(?MODULE, #{check_schema => false}). + emqx_dashboard_swagger:spec(?MODULE, #{ + check_schema => fun emqx_dashboard_swagger:is_content_type_json/2 + }). paths() -> [ diff --git a/apps/emqx_management/test/emqx_mgmt_api_banned_SUITE.erl b/apps/emqx_management/test/emqx_mgmt_api_banned_SUITE.erl index 8af23bb42..36c200258 100644 --- a/apps/emqx_management/test/emqx_mgmt_api_banned_SUITE.erl +++ b/apps/emqx_management/test/emqx_mgmt_api_banned_SUITE.erl @@ -216,6 +216,45 @@ t_create_failed(_Config) -> ?assertEqual(BadRequest, create_banned(Expired)), ok. +%% validate check_schema is true with bad content_type +t_create_with_bad_content_type(_Config) -> + Now = erlang:system_time(second), + At = emqx_banned:to_rfc3339(Now), + Until = emqx_banned:to_rfc3339(Now + 3), + Who = <<"TestClient-"/utf8>>, + By = <<"banned suite 中"/utf8>>, + Reason = <<"test测试"/utf8>>, + Banned = #{ + as => clientid, + who => Who, + by => By, + reason => Reason, + at => At, + until => Until + }, + AuthHeader = emqx_mgmt_api_test_util:auth_header_(), + Path = emqx_mgmt_api_test_util:api_path(["banned"]), + {error, { + {"HTTP/1.1", 415, "Unsupported Media Type"}, + _Headers, + MsgBin + }} = + emqx_mgmt_api_test_util:request_api( + post, + Path, + "", + AuthHeader, + Banned, + #{'content-type' => "application/xml", return_all => true} + ), + ?assertEqual( + #{ + <<"code">> => <<"UNSUPPORTED_MEDIA_TYPE">>, + <<"message">> => <<"content-type:application/json Required">> + }, + emqx_utils_json:decode(MsgBin) + ). + t_delete(_Config) -> Now = erlang:system_time(second), At = emqx_banned:to_rfc3339(Now), diff --git a/apps/emqx_management/test/emqx_mgmt_api_test_util.erl b/apps/emqx_management/test/emqx_mgmt_api_test_util.erl index 49f356316..99d1b6cb8 100644 --- a/apps/emqx_management/test/emqx_mgmt_api_test_util.erl +++ b/apps/emqx_management/test/emqx_mgmt_api_test_util.erl @@ -118,6 +118,7 @@ request_api(Method, Url, QueryParams, AuthOrHeaders, Body, Opts) when (Method =:= put) orelse (Method =:= delete) -> + ContentType = maps:get('content-type', Opts, "application/json"), NewUrl = case QueryParams of "" -> Url; @@ -125,9 +126,8 @@ request_api(Method, Url, QueryParams, AuthOrHeaders, Body, Opts) when end, do_request_api( Method, - {NewUrl, build_http_header(AuthOrHeaders), "application/json", - emqx_utils_json:encode(Body)}, - Opts + {NewUrl, build_http_header(AuthOrHeaders), ContentType, emqx_utils_json:encode(Body)}, + maps:remove('content-type', Opts) ). do_request_api(Method, Request, Opts) -> diff --git a/apps/emqx_rule_engine/src/emqx_rule_engine_api.erl b/apps/emqx_rule_engine/src/emqx_rule_engine_api.erl index b1308f008..16a012f50 100644 --- a/apps/emqx_rule_engine/src/emqx_rule_engine_api.erl +++ b/apps/emqx_rule_engine/src/emqx_rule_engine_api.erl @@ -137,7 +137,9 @@ end). namespace() -> "rule". api_spec() -> - emqx_dashboard_swagger:spec(?MODULE, #{check_schema => false}). + emqx_dashboard_swagger:spec(?MODULE, #{ + check_schema => fun emqx_dashboard_swagger:is_content_type_json/2 + }). paths() -> [ diff --git a/apps/emqx_rule_engine/test/emqx_rule_engine_api_2_SUITE.erl b/apps/emqx_rule_engine/test/emqx_rule_engine_api_2_SUITE.erl index 7ebb673a8..8e14af449 100644 --- a/apps/emqx_rule_engine/test/emqx_rule_engine_api_2_SUITE.erl +++ b/apps/emqx_rule_engine/test/emqx_rule_engine_api_2_SUITE.erl @@ -60,8 +60,11 @@ maybe_json_decode(X) -> end. request(Method, Path, Params) -> - AuthHeader = emqx_mgmt_api_test_util:auth_header_(), Opts = #{return_all => true}, + request(Method, Path, Params, Opts). + +request(Method, Path, Params, Opts) -> + AuthHeader = emqx_mgmt_api_test_util:auth_header_(), case emqx_mgmt_api_test_util:request_api(Method, Path, "", AuthHeader, Params, Opts) of {ok, {Status, Headers, Body0}} -> Body = maybe_json_decode(Body0), @@ -386,6 +389,38 @@ t_rule_test_smoke(_Config) -> ?assertEqual([], FailedCases), ok. +%% validate check_schema is function with bad content_type +t_rule_test_with_bad_content_type(_Config) -> + Params = + #{ + <<"context">> => + #{ + <<"clientid">> => <<"c_emqx">>, + <<"event_type">> => <<"message_publish">>, + <<"payload">> => <<"{\"msg\": \"hello\"}">>, + <<"qos">> => 1, + <<"topic">> => <<"t/a">>, + <<"username">> => <<"u_emqx">> + }, + <<"sql">> => <<"SELECT\n *\nFROM\n \"t/#\"">> + }, + Method = post, + Path = emqx_mgmt_api_test_util:api_path(["rule_test"]), + Opts = #{return_all => true, 'content-type' => "application/xml"}, + ?assertMatch( + {error, + { + {"HTTP/1.1", 415, "Unsupported Media Type"}, + _Headers, + #{ + <<"code">> := <<"UNSUPPORTED_MEDIA_TYPE">>, + <<"message">> := <<"content-type:application/json Required">> + } + }}, + request(Method, Path, Params, Opts) + ), + ok. + do_t_rule_test_smoke(#{input := Input, expected := #{code := ExpectedCode}} = Case) -> {_ErrOrOk, {{_, Code, _}, _, Body}} = sql_test_api(Input), case Code =:= ExpectedCode of From c013366b27be56088b497b6cf8cce321d9c3d0fb Mon Sep 17 00:00:00 2001 From: zhongwencool Date: Tue, 21 May 2024 16:05:47 +0800 Subject: [PATCH 2/3] chore: upgrade minirest to 1.4.1 to ignore 415 code check --- changes/ce/fix-13078.en.md | 1 + mix.exs | 2 +- rebar.config | 2 +- 3 files changed, 3 insertions(+), 2 deletions(-) create mode 100644 changes/ce/fix-13078.en.md diff --git a/changes/ce/fix-13078.en.md b/changes/ce/fix-13078.en.md new file mode 100644 index 000000000..8bb9aee24 --- /dev/null +++ b/changes/ce/fix-13078.en.md @@ -0,0 +1 @@ +Added validation and error handling to ensure requests with a JSON body include the required 'application/json' Content-Type header. If missing, the API now returns a 415 Unsupported Media Type status instead of a 400. diff --git a/mix.exs b/mix.exs index 552e2fd15..b4e7db9f8 100644 --- a/mix.exs +++ b/mix.exs @@ -58,7 +58,7 @@ defmodule EMQXUmbrella.MixProject do {:ekka, github: "emqx/ekka", tag: "0.19.3", override: true}, {:gen_rpc, github: "emqx/gen_rpc", tag: "3.3.1", override: true}, {:grpc, github: "emqx/grpc-erl", tag: "0.6.12", override: true}, - {:minirest, github: "emqx/minirest", tag: "1.4.0", override: true}, + {:minirest, github: "emqx/minirest", tag: "1.4.1", override: true}, {:ecpool, github: "emqx/ecpool", tag: "0.5.7", override: true}, {:replayq, github: "emqx/replayq", tag: "0.3.8", override: true}, {:pbkdf2, github: "emqx/erlang-pbkdf2", tag: "2.0.4", override: true}, diff --git a/rebar.config b/rebar.config index 98f8e2177..bb75bf3b8 100644 --- a/rebar.config +++ b/rebar.config @@ -86,7 +86,7 @@ {ekka, {git, "https://github.com/emqx/ekka", {tag, "0.19.3"}}}, {gen_rpc, {git, "https://github.com/emqx/gen_rpc", {tag, "3.3.1"}}}, {grpc, {git, "https://github.com/emqx/grpc-erl", {tag, "0.6.12"}}}, - {minirest, {git, "https://github.com/emqx/minirest", {tag, "1.4.0"}}}, + {minirest, {git, "https://github.com/emqx/minirest", {tag, "1.4.1"}}}, {ecpool, {git, "https://github.com/emqx/ecpool", {tag, "0.5.7"}}}, {replayq, {git, "https://github.com/emqx/replayq.git", {tag, "0.3.8"}}}, {pbkdf2, {git, "https://github.com/emqx/erlang-pbkdf2.git", {tag, "2.0.4"}}}, From 3aa1f86d9ec5a425dea3da47ef471be8dc963af7 Mon Sep 17 00:00:00 2001 From: zhongwencool Date: Wed, 22 May 2024 08:23:53 +0800 Subject: [PATCH 3/3] chore: is_content_type_json to validate_content_type_json --- apps/emqx_bridge/src/emqx_bridge_api.erl | 2 +- .../src/emqx_dashboard_swagger.erl | 16 ++++++++-------- apps/emqx_license/src/emqx_license_http_api.erl | 2 +- .../src/emqx_rule_engine_api.erl | 2 +- 4 files changed, 11 insertions(+), 11 deletions(-) diff --git a/apps/emqx_bridge/src/emqx_bridge_api.erl b/apps/emqx_bridge/src/emqx_bridge_api.erl index 2d2c1455c..003ec9a06 100644 --- a/apps/emqx_bridge/src/emqx_bridge_api.erl +++ b/apps/emqx_bridge/src/emqx_bridge_api.erl @@ -83,7 +83,7 @@ namespace() -> "bridge". api_spec() -> emqx_dashboard_swagger:spec(?MODULE, #{ - check_schema => fun emqx_dashboard_swagger:is_content_type_json/2 + check_schema => fun emqx_dashboard_swagger:validate_content_type_json/2 }). paths() -> diff --git a/apps/emqx_dashboard/src/emqx_dashboard_swagger.erl b/apps/emqx_dashboard/src/emqx_dashboard_swagger.erl index 308ee3d40..9112578dc 100644 --- a/apps/emqx_dashboard/src/emqx_dashboard_swagger.erl +++ b/apps/emqx_dashboard/src/emqx_dashboard_swagger.erl @@ -30,7 +30,7 @@ -export([base_path/0]). -export([relative_uri/1, get_relative_uri/1]). -export([compose_filters/2]). --export([is_content_type_json/2, validate_content_type/3]). +-export([validate_content_type_json/2, validate_content_type/3]). -export([ filter_check_request/2, @@ -153,18 +153,22 @@ spec(Module, Options) -> ), {ApiSpec, components(lists:usort(AllRefs), Options)}. -is_content_type_json(Params, Meta) -> +validate_content_type_json(Params, Meta) -> validate_content_type(Params, Meta, <<"application/json">>). %% tip: Skip content-type check if body is empty. -validate_content_type(#{body := Body} = Params, #{method := Method}, Expect) when +validate_content_type( + #{body := Body, headers := Headers} = Params, + #{method := Method}, + Expect +) when (Method =:= put orelse Method =:= post orelse method =:= patch) andalso Body =/= #{} -> ExpectSize = byte_size(Expect), - case find_content_type(Params) of + case maps:get(<<"content-type">>, Headers, undefined) of <> -> {ok, Params}; _ -> @@ -173,10 +177,6 @@ validate_content_type(#{body := Body} = Params, #{method := Method}, Expect) whe validate_content_type(Params, _Meta, _Expect) -> {ok, Params}. -find_content_type(#{headers := #{<<"content-type">> := Type}}) -> Type; -find_content_type(#{headers := #{<<"Content-Type">> := Type}}) -> Type; -find_content_type(_Headers) -> error. - -spec namespace() -> hocon_schema:name(). namespace() -> "public". diff --git a/apps/emqx_license/src/emqx_license_http_api.erl b/apps/emqx_license/src/emqx_license_http_api.erl index 9265d65ec..c3791e0ea 100644 --- a/apps/emqx_license/src/emqx_license_http_api.erl +++ b/apps/emqx_license/src/emqx_license_http_api.erl @@ -29,7 +29,7 @@ namespace() -> "license_http_api". api_spec() -> emqx_dashboard_swagger:spec(?MODULE, #{ - check_schema => fun emqx_dashboard_swagger:is_content_type_json/2 + check_schema => fun emqx_dashboard_swagger:validate_content_type_json/2 }). paths() -> diff --git a/apps/emqx_rule_engine/src/emqx_rule_engine_api.erl b/apps/emqx_rule_engine/src/emqx_rule_engine_api.erl index 16a012f50..0ac59b36c 100644 --- a/apps/emqx_rule_engine/src/emqx_rule_engine_api.erl +++ b/apps/emqx_rule_engine/src/emqx_rule_engine_api.erl @@ -138,7 +138,7 @@ namespace() -> "rule". api_spec() -> emqx_dashboard_swagger:spec(?MODULE, #{ - check_schema => fun emqx_dashboard_swagger:is_content_type_json/2 + check_schema => fun emqx_dashboard_swagger:validate_content_type_json/2 }). paths() ->