Merge pull request #13078 from zhongwencool/http-415

feat: return 415 when UNSUPPORTED_MEDIA_TYPE

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">>}
 ]).

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:validate_content_type_json/2
+    }).
 
 paths() ->
     [

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([validate_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)}.
 
+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, headers := Headers} = Params,
+    #{method := Method},
+    Expect
+) when
+    (Method =:= put orelse
+        Method =:= post orelse
+        method =:= patch) andalso
+        Body =/= #{}
+->
+    ExpectSize = byte_size(Expect),
+    case maps:get(<<"content-type">>, Headers, undefined) of
+        <<Expect:ExpectSize/binary, _/binary>> ->
+            {ok, Params};
+        _ ->
+            {415, 'UNSUPPORTED_MEDIA_TYPE', <<"content-type:", Expect/binary, " Required">>}
+    end;
+validate_content_type(Params, _Meta, _Expect) ->
+    {ok, Params}.
+
 -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)),
+        case check_request_body(Request, Body, Module, CheckFun, hoconsc:is_schema(Body)) of
-        {ok, Request#{bindings => Bindings, query_string => QueryStr, body => NewBody}}
+            {ok, NewBody} ->
+                {ok, Request#{bindings => Bindings, query_string => QueryStr, body => NewBody}};
+            Error ->
+                Error
+        end
     catch
         throw:HoconError ->
             Msg = hocon_error_msg(HoconError),
@@ -523,6 +552,10 @@ unwrap_array_conv([HVal | _], _Opts) -> HVal;
 unwrap_array_conv(SingleVal, _Opts) -> SingleVal.
 
 check_request_body(#{body := Body}, Schema, Module, CheckFun, true) ->
+    %% 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
@@ -532,7 +565,10 @@ check_request_body(#{body := Body}, Schema, Module, CheckFun, true) ->
             NewSchema = ?INIT_SCHEMA#{roots => [{root, Type}]},
             Option = #{required => false},
             #{<<"root">> := NewBody} = CheckFun(NewSchema, #{<<"root">> => Body}, Option),
-    NewBody;
+            {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,6 +577,7 @@ check_request_body(#{body := Body}, Schema, Module, CheckFun, true) ->
 %%                ]}
 %% ]
 check_request_body(#{body := Body}, Spec, _Module, CheckFun, false) when is_list(Spec) ->
+    {ok,
         lists:foldl(
             fun({Name, Type}, Acc) ->
                 Schema = ?INIT_SCHEMA#{roots => [{Name, Type}]},
@@ -548,11 +585,11 @@ check_request_body(#{body := Body}, Spec, _Module, CheckFun, false) when is_list
             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) ->

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,

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() ->
     [

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:validate_content_type_json/2
+    }).
 
 paths() ->
     [

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),

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",
+        {NewUrl, build_http_header(AuthOrHeaders), ContentType, emqx_utils_json:encode(Body)},
-            emqx_utils_json:encode(Body)},
+        maps:remove('content-type', Opts)
-        Opts
     ).
 
 do_request_api(Method, Request, Opts) ->

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:validate_content_type_json/2
+    }).
 
 paths() ->
     [

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

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.

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},

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"}}},