This is an automated email from the ASF dual-hosted git repository. Jens-G pushed a commit to branch release/0.25.0 in repository https://gitbox.apache.org/repos/asf/thrift.git
commit 2c09024bde00de4e3cb0c77a371880fae3882171 Author: Jens Geyer <[email protected]> AuthorDate: Wed Sep 23 23:53:26 2026 +0200 Bound the size of a message thrift_json_protocol reads Client: erl read_all/1 now reads a message only up to the thrift application's max_message_size, the setting thrift_http_transport already uses (100 MB by default), or up to a max_message_size option given to thrift_json_protocol:new/2. A longer message is refused before more than the maximum has been read, with {error, {message_size_exceeds_maximum, Max}}, which thrift_processor answers by closing the connection. Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]> --- lib/erl/include/thrift_constants.hrl | 8 ++-- lib/erl/src/thrift_json_protocol.erl | 74 ++++++++++++++++++++-------- lib/erl/test/test_thrift_json_protocol.erl | 77 ++++++++++++++++++++++++++++-- 3 files changed, 132 insertions(+), 27 deletions(-) diff --git a/lib/erl/include/thrift_constants.hrl b/lib/erl/include/thrift_constants.hrl index 78c91a8a2..0e46cd727 100644 --- a/lib/erl/include/thrift_constants.hrl +++ b/lib/erl/include/thrift_constants.hrl @@ -75,8 +75,8 @@ %% bindings use. Override with {thrift, [{max_frame_size, Bytes}]}. -define(DEFAULT_MAX_FRAME_SIZE, 16384000). -%% The largest reply thrift_http_transport will read. Same value the other -%% bindings use for their maximum message size. Override with -%% {thrift, [{max_message_size, Bytes}]}, or for one transport with its -%% max_message_size option. +%% The largest reply thrift_http_transport will read, and the largest message +%% thrift_json_protocol will read. Same value the other bindings use for their +%% maximum message size. Override with {thrift, [{max_message_size, Bytes}]}, +%% or for one transport or protocol with its max_message_size option. -define(DEFAULT_MAX_MESSAGE_SIZE, (100 * 1024 * 1024)). diff --git a/lib/erl/src/thrift_json_protocol.erl b/lib/erl/src/thrift_json_protocol.erl index d8da197a3..d2602949c 100644 --- a/lib/erl/src/thrift_json_protocol.erl +++ b/lib/erl/src/thrift_json_protocol.erl @@ -50,7 +50,9 @@ -record(json_protocol, { transport :: term(), context_stack = [] :: [json_context()], - jsx :: undefined | jsx() + jsx :: undefined | jsx(), + % undefined: the thrift application's max_message_size + max_message_size :: pos_integer() | undefined }). -define(VERSION_1, 1). @@ -90,9 +92,26 @@ end_context(array) -> "]". new(Transport) -> new(Transport, _Options = []). +%%-------------------------------------------------------------------- +%% Options include: +%% {max_message_size, Bytes} = The longest message to read, in place of the +%% thrift application's max_message_size +%%-------------------------------------------------------------------- +new(Transport, Options) when is_list(Options) -> + State = lists:foldl(fun apply_option/2, #json_protocol{transport = Transport}, Options), + thrift_protocol:new(?MODULE, State); new(Transport, _Options) -> - State = #json_protocol{transport = Transport}, - thrift_protocol:new(?MODULE, State). + new(Transport, []). + +apply_option({max_message_size, Max}, State) when is_integer(Max), Max > 0 -> + State#json_protocol{max_message_size = Max}; +apply_option(_Other, State) -> + State. + +max_message_size(#json_protocol{max_message_size = undefined}) -> + application:get_env(thrift, max_message_size, ?DEFAULT_MAX_MESSAGE_SIZE); +max_message_size(#json_protocol{max_message_size = Max}) -> + Max. flush_transport(This = #json_protocol{transport = Transport}) -> {NewTransport, Result} = thrift_transport:flush(Transport), @@ -356,8 +375,11 @@ escape_byte(B) -> <<B>>. %% transport, and a buffered one over it, waits until it has all the bytes %% it was asked for, and the stream goes on with the next message; other %% transports, framed among them, return what they have and keep the rest. +%% +%% A message longer than max_message_size is refused before more than that +%% many bytes of it are read. read_all(#json_protocol{transport = Transport0} = State) -> - case read_message(Transport0, [], 0, false, false) of + case read_message(Transport0, max_message_size(State), [], 0, 0, false, false) of {Transport1, {ok, Bin}} -> P = thrift_json_parser:parser(), [First | Rest] = P(Bin), @@ -366,22 +388,34 @@ read_all(#json_protocol{transport = Transport0} = State) -> {State#json_protocol{transport = Transport1}, Error} end. -read_message(Transport0, Parts, Depth, InString, Escaped) -> - {Transport1, Result} = thrift_transport:read( - Transport0, least_left(Depth, InString, Escaped) - ), - case Result of - {ok, <<>>} -> - {Transport1, {error, eof}}; - {ok, Data} -> - case scan(Data, Depth, InString, Escaped) of - done -> - {Transport1, {ok, iolist_to_binary(lists:reverse([Data | Parts]))}}; - {Depth1, InString1, Escaped1} -> - read_message(Transport1, [Data | Parts], Depth1, InString1, Escaped1) - end; - {error, _} = Error -> - {Transport1, Error} +read_message(Transport0, Max, Parts, Size, Depth, InString, Escaped) -> + Least = least_left(Depth, InString, Escaped), + case Size + Least > Max of + true -> + {Transport0, {error, {message_size_exceeds_maximum, Max}}}; + false -> + {Transport1, Result} = thrift_transport:read(Transport0, Least), + case Result of + {ok, <<>>} -> + {Transport1, {error, eof}}; + {ok, Data} -> + case scan(Data, Depth, InString, Escaped) of + done -> + {Transport1, {ok, iolist_to_binary(lists:reverse([Data | Parts]))}}; + {Depth1, InString1, Escaped1} -> + read_message( + Transport1, + Max, + [Data | Parts], + Size + byte_size(Data), + Depth1, + InString1, + Escaped1 + ) + end; + {error, _} = Error -> + {Transport1, Error} + end end. %% The fewest bytes that can still complete the message: a closing bracket diff --git a/lib/erl/test/test_thrift_json_protocol.erl b/lib/erl/test/test_thrift_json_protocol.erl index 7d01dceab..eb060f83e 100644 --- a/lib/erl/test/test_thrift_json_protocol.erl +++ b/lib/erl/test/test_thrift_json_protocol.erl @@ -88,9 +88,12 @@ message(Name, Type, Value) -> {Protocol2, ok} = thrift_protocol:write(Protocol1, {Type, Value}), {Protocol3, ok} = thrift_protocol:write(Protocol2, message_end), {Protocol4, ok} = thrift_protocol:flush_transport(Protocol3), - {protocol, thrift_json_protocol, {json_protocol, Transport, _, _}} = Protocol4, - {t_transport, thrift_membuffer_transport, {t_membuffer, Bytes}} = Transport, - iolist_to_binary(Bytes). + iolist_to_binary(buffered(Protocol4)). + +%% What is left in the memory buffer under Protocol. +buffered({protocol, thrift_json_protocol, State}) -> + {t_transport, thrift_membuffer_transport, {t_membuffer, Bytes}} = element(2, State), + Bytes. %% Reads a message of Type from Protocol: {Protocol, {Name, Value}}. read_message(Protocol0, Type) -> @@ -217,6 +220,74 @@ read_calls_test_() -> ?assert(Calls < Size div 3) end)}. +%%%% Message size %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%% + +%% A message of exactly Size bytes, a string field making up the rest. +message_of_size(Size) -> + Type = {struct, [{1, string}]}, + Empty = byte_size(message("m", Type, {args, ""})), + message("m", Type, {args, binary:copy(<<"a">>, Size - Empty)}). + +%% Reads message_begin from Bytes: what it returned, and how many bytes it +%% took from the transport. +read_begin(Bytes, Options) -> + {ok, Transport} = thrift_membuffer_transport:new(Bytes), + {ok, Protocol0} = thrift_json_protocol:new(Transport, Options), + {Protocol1, Result} = thrift_protocol:read(Protocol0, message_begin), + {Result, byte_size(Bytes) - iolist_size(buffered(Protocol1))}. + +%% A message up to the max_message_size option is read. One a byte longer is +%% refused before more than the maximum is taken from the transport. +max_message_size_option_test() -> + Max = 4096, + ?assertEqual(Max, byte_size(message_of_size(Max))), + ?assertMatch( + {#protocol_message_begin{name = "m"}, Max}, + read_begin(message_of_size(Max), [{max_message_size, Max}]) + ), + {Result, Taken} = read_begin(message_of_size(Max + 1), [{max_message_size, Max}]), + ?assertEqual({error, {message_size_exceeds_maximum, Max}}, Result), + ?assert(Taken =< Max). + +%% Without the option, the thrift application's max_message_size applies. +max_message_size_env_test_() -> + {setup, + fun() -> + ok = load_thrift_application(), + Saved = application:get_env(thrift, max_message_size), + ok = application:set_env(thrift, max_message_size, 4096), + Saved + end, + fun + (undefined) -> application:unset_env(thrift, max_message_size); + ({ok, Value}) -> application:set_env(thrift, max_message_size, Value) + end, + ?_test(begin + ?assertMatch( + {#protocol_message_begin{name = "m"}, 4096}, read_begin(message_of_size(4096), []) + ), + {Result, Taken} = read_begin(message_of_size(4097), []), + ?assertEqual({error, {message_size_exceeds_maximum, 4096}}, Result), + ?assert(Taken =< 4096), + %% The option takes the place of the application's setting. + ?assertMatch( + {#protocol_message_begin{name = "m"}, 8192}, + read_begin(message_of_size(8192), [{max_message_size, 8192}]) + ) + end)}. + +%% A long message reads back the same, including the characters that decide +%% where it ends, and the message after it is still there to be read. +long_message_test() -> + Type = {struct, [{1, string}, {2, i32}]}, + Text = binary:copy(list_to_binary(?AWKWARD), 20000), + First = message("first", Type, {args, Text, 1}), + Second = message("second", Type, {args, "]}\"[{", 2}), + {Protocol1, Read1} = read_message(new_protocol(<<First/binary, Second/binary>>), Type), + ?assertEqual({"first", {Text, 1}}, Read1), + {_, Read2} = read_message(Protocol1, Type), + ?assertEqual({"second", {<<"]}\"[{">>, 2}}, Read2). + %%%% Over a socket %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%% handle_function(_Function, Args) -> {reply, element(1, Args)}.
