From 5c759941d57c5b744c8ef98f20cda2c3909aa646 Mon Sep 17 00:00:00 2001 From: zhongwencool Date: Tue, 21 May 2024 09:18:07 +0800 Subject: [PATCH] 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