Skip to content

Commit 2e035de

Browse files
michaelklishinmergify[bot]
michaelklishin
authored and
mergify[bot]
committed
HTTP API: limit default body size for binding creation
It does not need to use the "worst case scenario" default HTTP request body size limit that is primarily necessary because definition imports can be large (MiBs in size, for example). Since exchange, queue names and routing key have limits of 255 bytes and optional arguments can practically be expected to be short, we can lower the limit to < 10 KiB. (cherry picked from commit fb300d2)
1 parent 9408284 commit 2e035de

File tree

3 files changed

+74
-38
lines changed

3 files changed

+74
-38
lines changed

‎deps/rabbitmq_management/src/rabbit_mgmt_util.erl‎

Lines changed: 30 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@
3030
list_login_vhosts_names/2]).
3131
-export([filter_tracked_conn_list/3]).
3232
-export([with_decode/5, decode/1, decode/2, set_resp_header/3,
33-
args/1, read_complete_body/1]).
33+
args/1, read_complete_body/1, read_complete_body_with_limit/2]).
3434
-export([reply_list/3, reply_list/5, reply_list/4,
3535
sort_list/2, destination_type/1, reply_list_or_paginate/3
3636
]).
@@ -703,22 +703,49 @@ halt_response(Code, Type, Reason, ReqData, Context) ->
703703
id(Key, ReqData) ->
704704
rabbit_web_dispatch_access_control:id(Key, ReqData).
705705

706+
%% IMPORTANT:
707+
%% Prefer read_complete_body_with_limit/2 with an explicit limit to make it easier
708+
%% to reason about what limit will be used.
706709
read_complete_body(Req) ->
707710
read_complete_body(Req, <<"">>).
708711
read_complete_body(Req, Acc) ->
709712
BodySizeLimit = application:get_env(rabbitmq_management, max_http_body_size, ?MANAGEMENT_DEFAULT_HTTP_MAX_BODY_SIZE),
710713
read_complete_body(Req, Acc, BodySizeLimit).
711714
read_complete_body(Req0, Acc, BodySizeLimit) ->
712-
case bit_size(Acc) > BodySizeLimit of
715+
case byte_size(Acc) > BodySizeLimit of
713716
true ->
714-
{error, "Exceeded HTTP request body size limit"};
717+
{error, http_body_limit_exceeded};
715718
false ->
716719
case cowboy_req:read_body(Req0) of
717720
{ok, Data, Req} -> {ok, <<Acc/binary, Data/binary>>, Req};
718721
{more, Data, Req} -> read_complete_body(Req, <<Acc/binary, Data/binary>>)
719722
end
720723
end.
721724

725+
read_complete_body_with_limit(Req, BodySizeLimit) when is_integer(BodySizeLimit) ->
726+
case cowboy_req:body_length(Req) of
727+
N when is_integer(N) ->
728+
case N > BodySizeLimit of
729+
true ->
730+
{error, http_body_limit_exceeded, BodySizeLimit, N};
731+
false ->
732+
do_read_complete_body_with_limit(Req, <<"">>, BodySizeLimit)
733+
end;
734+
undefined ->
735+
do_read_complete_body_with_limit(Req, <<"">>, BodySizeLimit)
736+
end.
737+
738+
do_read_complete_body_with_limit(Req0, Acc, BodySizeLimit) ->
739+
case byte_size(Acc) > BodySizeLimit of
740+
true ->
741+
{error, http_body_limit_exceeded, BodySizeLimit, byte_size(Acc)};
742+
false ->
743+
case cowboy_req:read_body(Req0, #{length => BodySizeLimit, period => 30000}) of
744+
{ok, Data, Req} -> {ok, <<Acc/binary, Data/binary>>, Req};
745+
{more, Data, Req} -> do_read_complete_body_with_limit(Req, <<Acc/binary, Data/binary>>, BodySizeLimit)
746+
end
747+
end.
748+
722749
with_decode(Keys, ReqData, Context, Fun) ->
723750
case read_complete_body(ReqData) of
724751
{error, Reason} ->

‎deps/rabbitmq_management/src/rabbit_mgmt_wm_bindings.erl‎

Lines changed: 42 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,10 @@
1616
-include_lib("rabbitmq_management_agent/include/rabbit_mgmt_records.hrl").
1717
-include_lib("amqp_client/include/amqp_client.hrl").
1818

19+
%% Use a much lower limit for creating bindings over the HTTP API.
20+
%% The payload is not meant to be even 50 KiB in size.
21+
-define(HTTP_BODY_SIZE_LIMIT, 5000).
22+
1923
%%--------------------------------------------------------------------
2024

2125
init(Req, [Mode]) ->
@@ -64,39 +68,44 @@ to_json(ReqData, {Mode, Context}) ->
6468
ReqData, {Mode, Context}).
6569

6670
accept_content(ReqData0, {_Mode, Context}) ->
67-
{ok, Body, ReqData} = rabbit_mgmt_util:read_complete_body(ReqData0),
68-
Source = rabbit_mgmt_util:id(source, ReqData),
69-
Dest = rabbit_mgmt_util:id(destination, ReqData),
70-
DestType = rabbit_mgmt_util:id(dtype, ReqData),
71-
VHost = rabbit_mgmt_util:vhost(ReqData),
72-
{ok, Props} = rabbit_mgmt_util:decode(Body),
73-
MethodName = case rabbit_mgmt_util:destination_type(ReqData) of
74-
exchange -> 'exchange.bind';
75-
queue -> 'queue.bind'
76-
end,
77-
{Key, Args} = key_args(DestType, Props),
78-
case rabbit_mgmt_util:direct_request(
79-
MethodName,
80-
fun rabbit_mgmt_format:format_accept_content/1,
81-
[{queue, Dest},
82-
{exchange, Source},
83-
{destination, Dest},
84-
{source, Source},
85-
{routing_key, Key},
86-
{arguments, Args}],
87-
"Binding error: ~ts", ReqData, Context) of
88-
{stop, _, _} = Res ->
89-
Res;
90-
{true, ReqData, Context2} ->
91-
From = binary_to_list(cowboy_req:path(ReqData)),
92-
Prefix = rabbit_mgmt_util:get_path_prefix(),
93-
BindingProps = rabbit_mgmt_format:pack_binding_props(Key, Args),
94-
UrlWithBindings = rabbit_mgmt_format:url("/api/bindings/~ts/e/~ts/~ts/~ts/~ts",
95-
[VHost, Source, DestType,
96-
Dest, BindingProps]),
97-
To = Prefix ++ binary_to_list(UrlWithBindings),
98-
Loc = rabbit_web_dispatch_util:relativise(From, To),
99-
{{true, Loc}, ReqData, Context2}
71+
case rabbit_mgmt_util:read_complete_body_with_limit(ReqData0, ?HTTP_BODY_SIZE_LIMIT) of
72+
{ok, Body, ReqData} ->
73+
Source = rabbit_mgmt_util:id(source, ReqData),
74+
Dest = rabbit_mgmt_util:id(destination, ReqData),
75+
DestType = rabbit_mgmt_util:id(dtype, ReqData),
76+
VHost = rabbit_mgmt_util:vhost(ReqData),
77+
{ok, Props} = rabbit_mgmt_util:decode(Body),
78+
MethodName = case rabbit_mgmt_util:destination_type(ReqData) of
79+
exchange -> 'exchange.bind';
80+
queue -> 'queue.bind'
81+
end,
82+
{Key, Args} = key_args(DestType, Props),
83+
case rabbit_mgmt_util:direct_request(
84+
MethodName,
85+
fun rabbit_mgmt_format:format_accept_content/1,
86+
[{queue, Dest},
87+
{exchange, Source},
88+
{destination, Dest},
89+
{source, Source},
90+
{routing_key, Key},
91+
{arguments, Args}],
92+
"Binding error: ~ts", ReqData, Context) of
93+
{stop, _, _} = Res ->
94+
Res;
95+
{true, ReqData, Context2} ->
96+
From = binary_to_list(cowboy_req:path(ReqData)),
97+
Prefix = rabbit_mgmt_util:get_path_prefix(),
98+
BindingProps = rabbit_mgmt_format:pack_binding_props(Key, Args),
99+
UrlWithBindings = rabbit_mgmt_format:url("/api/bindings/~ts/e/~ts/~ts/~ts/~ts",
100+
[VHost, Source, DestType,
101+
Dest, BindingProps]),
102+
To = Prefix ++ binary_to_list(UrlWithBindings),
103+
Loc = rabbit_web_dispatch_util:relativise(From, To),
104+
{{true, Loc}, ReqData, Context2}
105+
end;
106+
{error, http_body_limit_exceeded, LimitApplied, BytesRead} ->
107+
rabbit_log:warning("HTTP API: binding creation request exceeded maximum allowed payload size (limit: ~tp bytes, payload size: ~tp bytes)", [LimitApplied, BytesRead]),
108+
rabbit_mgmt_util:bad_request("Payload size limit exceeded", ReqData0, Context)
100109
end.
101110

102111
is_authorized(ReqData, {Mode, Context}) ->

‎deps/rabbitmq_management/src/rabbit_mgmt_wm_definitions.erl‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -84,9 +84,9 @@ all_definitions(ReqData, Context) ->
8484
Context).
8585

8686
accept_json(ReqData0, Context) ->
87-
case rabbit_mgmt_util:read_complete_body(ReqData0) of
87+
BodySizeLimit = application:get_env(rabbitmq_management, max_http_body_size, ?MANAGEMENT_DEFAULT_HTTP_MAX_BODY_SIZE),
88+
case rabbit_mgmt_util:read_complete_body_with_limit(ReqData0, BodySizeLimit) of
8889
{error, Reason} ->
89-
BodySizeLimit = application:get_env(rabbitmq_management, max_http_body_size, ?MANAGEMENT_DEFAULT_HTTP_MAX_BODY_SIZE),
9090
_ = rabbit_log:warning("HTTP API: uploaded definition file exceeded the maximum request body limit of ~p bytes. "
9191
"Use the 'management.http.max_body_size' key in rabbitmq.conf to increase the limit if necessary", [BodySizeLimit]),
9292
rabbit_mgmt_util:bad_request(Reason, ReqData0, Context);

0 commit comments

Comments
 (0)