fix: limit bytes param to signed 32bit int

We still need to check if chunk we're reading fits in memory

apps/emqx_management/src/emqx_mgmt_api_trace.erl

@@ -47,9 +47,12 @@
     get_trace_size/0
 ]).
 
+-define(MAX_SINT32, 2147483647).
+
 -define(TO_BIN(_B_), iolist_to_binary(_B_)).
 -define(NOT_FOUND(N), {404, #{code => 'NOT_FOUND', message => ?TO_BIN([N, " NOT FOUND"])}}).
 -define(BAD_REQUEST(C, M), {400, #{code => C, message => ?TO_BIN(M)}}).
+-define(SERVICE_UNAVAILABLE(C, M), {503, #{code => C, message => ?TO_BIN(M)}}).
 -define(TAGS, [<<"Trace">>]).
 
 namespace() -> "trace".
@@ -184,8 +187,15 @@ schema("/trace/:name/log") ->
                         {items, hoconsc:mk(binary(), #{example => "TEXT-LOG-ITEMS"})},
                         {meta, fields(bytes) ++ fields(position)}
                     ],
-                400 => emqx_dashboard_swagger:error_codes(['NODE_ERROR'], <<"Trace Log Failed">>),
-                404 => emqx_dashboard_swagger:error_codes(['NOT_FOUND'], <<"Trace Name Not Found">>)
+                400 => emqx_dashboard_swagger:error_codes(
+                    ['BAD_REQUEST', 'NODE_ERROR'], <<"Bad input parameter">>
+                ),
+                404 => emqx_dashboard_swagger:error_codes(
+                    ['NOT_FOUND'], <<"Trace Name Not Found">>
+                ),
+                503 => emqx_dashboard_swagger:error_codes(
+                    ['SERVICE_UNAVAILABLE'], <<"Requested chunk size too big">>
+                )
             }
         }
     }.
@@ -313,12 +323,16 @@ fields(bytes) ->
     [
         {bytes,
             hoconsc:mk(
-                integer(),
+                %% This seems to be the minimum max value we may encounter
+                %% across different OS
+                range(0, ?MAX_SINT32),
                 #{
                     desc => "Maximum number of bytes to send in response",
                     in => query,
                     required => false,
-                    default => 1000
+                    default => 1000,
+                    minimum => 0,
+                    maximum => ?MAX_SINT32
                 }
             )}
     ];
@@ -579,6 +593,14 @@ stream_log_file(get, #{bindings := #{name := Name}, query_string := Query}) ->
                     {200, #{meta => Meta, items => <<"">>}};
                 {error, not_found} ->
                     ?NOT_FOUND(Name);
+                {error, enomem} ->
+                    ?SLOG(warning, #{
+                        code => not_enough_mem,
+                        msg => "Requested chunk size too big",
+                        bytes => Bytes,
+                        name => Name
+                    }),
+                    ?SERVICE_UNAVAILABLE('SERVICE_UNAVAILABLE', <<"Requested chunk size too big">>);
                 {badrpc, nodedown} ->
                     ?BAD_REQUEST('NODE_ERROR', <<"Node not found">>)
             end;

apps/emqx_management/test/emqx_mgmt_api_trace_SUITE.erl

@@ -19,9 +19,7 @@
 -compile(export_all).
 -compile(nowarn_export_all).
 
--include_lib("common_test/include/ct.hrl").
 -include_lib("eunit/include/eunit.hrl").
--include_lib("emqx/include/emqx.hrl").
 -include_lib("kernel/include/file.hrl").
 -include_lib("stdlib/include/zip.hrl").
 -include_lib("snabbkaffe/include/snabbkaffe.hrl").
@@ -296,6 +294,15 @@ t_stream_log(_Config) ->
     #{<<"meta">> := Meta1, <<"items">> := Bin1} = json(Binary1),
     ?assertEqual(#{<<"position">> => 30, <<"bytes">> => 10}, Meta1),
     ?assertEqual(10, byte_size(Bin1)),
+    ct:pal("~p vs ~p", [Bin, Bin1]),
+    %% in theory they could be the same but we know they shouldn't
+    ?assertNotEqual(Bin, Bin1),
+    BadReqPath = api_path("trace/test_stream_log/log?&bytes=1000000000000"),
+    {error, {_, 400, _}} = request_api(get, BadReqPath),
+    meck:new(file, [passthrough, unstick]),
+    meck:expect(file, read, 2, {error, enomem}),
+    {error, {_, 503, _}} = request_api(get, Path),
+    meck:unload(file),
     {error, {_, 400, _}} =
         request_api(
             get,