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
The following commit(s) were added to refs/heads/release/0.25.0 by this push:
new 27e8a425f THRIFT-6367: Cap the message name in the Erlang binary and
compact protocols Client: erl
27e8a425f is described below
commit 27e8a425ffb498e190df3a12e239326bf5ba9ed6
Author: Jens Geyer <[email protected]>
AuthorDate: Thu Sep 24 00:58:25 2026 +0200
THRIFT-6367: Cap the message name in the Erlang binary and compact protocols
Client: erl
message_begin of thrift_binary_protocol and thrift_compact_protocol now
reads the declared length of the name first and refuses a name longer than
?MAX_MESSAGE_NAME_SIZE (4096 bytes) before reading it, with
{error, {message_name_exceeds_maximum, 4096}}. A name the processor can
dispatch is at most 255 characters for the function, plus a service name
and a ':' for a multiplexed service.
Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
---
lib/erl/include/thrift_constants.hrl | 6 +++---
lib/erl/src/thrift_binary_protocol.erl | 22 ++++++++++++++++++----
lib/erl/src/thrift_compact_protocol.erl | 18 +++++++++++++-----
lib/erl/test/test_thrift_max_message_size.erl | 19 +++++++++++++++++++
4 files changed, 53 insertions(+), 12 deletions(-)
diff --git a/lib/erl/include/thrift_constants.hrl
b/lib/erl/include/thrift_constants.hrl
index 5339131f5..d5a83c4dd 100644
--- a/lib/erl/include/thrift_constants.hrl
+++ b/lib/erl/include/thrift_constants.hrl
@@ -82,7 +82,7 @@
%% its max_message_size option.
-define(DEFAULT_MAX_MESSAGE_SIZE, (100 * 1024 * 1024)).
-%% The longest message name thrift_json_protocol will read. A name the
-%% processor can dispatch is a function name, an atom of at most 255
-%% characters, after a service name and a ':' for a multiplexed service.
+%% The longest message name the binary, compact and JSON protocols will read.
+%% A name the processor can dispatch is a function name, an atom of at most
+%% 255 characters, after a service name and a ':' for a multiplexed service.
-define(MAX_MESSAGE_NAME_SIZE, 4096).
diff --git a/lib/erl/src/thrift_binary_protocol.erl
b/lib/erl/src/thrift_binary_protocol.erl
index b1cf57f6b..37c1cd0cd 100644
--- a/lib/erl/src/thrift_binary_protocol.erl
+++ b/lib/erl/src/thrift_binary_protocol.erl
@@ -186,9 +186,14 @@ read(This0 = #binary_protocol{max_message_size = Max},
message_begin) ->
case Initial of
{ok, Sz} when Sz band ?VERSION_MASK =:= ?VERSION_1 ->
%% we're at version 1
- case read(This1, string) of
- {This2, {ok, Name}} ->
- read_message_begin(This2, Name, Sz band ?TYPE_MASK);
+ case read(This1, i32) of
+ {This2, {ok, NameSz}} ->
+ case read_name(This2, NameSz) of
+ {This3, {ok, Name}} ->
+ read_message_begin(This3, Name, Sz band
?TYPE_MASK);
+ {This3, Error} ->
+ {This3, Error}
+ end;
{This2, Error} ->
{This2, Error}
end;
@@ -200,7 +205,7 @@ read(This0 = #binary_protocol{max_message_size = Max},
message_begin) ->
{This1, {error, no_binary_protocol_version}};
{ok, Sz} when This1#binary_protocol.strict_read =:= false ->
%% strict_read is false, so just read the old way
- case read_data(This1, Sz) of
+ case read_name(This1, Sz) of
{This2, {ok, Name}} ->
case read(This2, byte) of
{This3, {ok, Type}} -> read_message_begin(This3, Name,
Type);
@@ -320,6 +325,15 @@ read(This0, string) ->
if Sz < 0 -> error({protocol_error, negative_size}); true -> ok end,
read_data(This1, Sz).
+%% The name of a message, refused before it is read if it is longer than
+%% ?MAX_MESSAGE_NAME_SIZE.
+read_name(_This, Sz) when Sz < 0 ->
+ error({protocol_error, negative_size});
+read_name(This, Sz) when Sz > ?MAX_MESSAGE_NAME_SIZE ->
+ {This, {error, {message_name_exceeds_maximum, ?MAX_MESSAGE_NAME_SIZE}}};
+read_name(This, Sz) ->
+ read_data(This, Sz).
+
%% The sequence id that ends a message header, once its name and type are read.
read_message_begin(This0, Name, Type) ->
case read(This0, i32) of
diff --git a/lib/erl/src/thrift_compact_protocol.erl
b/lib/erl/src/thrift_compact_protocol.erl
index 70464e1f6..2f3886e0d 100644
--- a/lib/erl/src/thrift_compact_protocol.erl
+++ b/lib/erl/src/thrift_compact_protocol.erl
@@ -271,15 +271,16 @@ read(This0 = #t_compact{max_message_size = Max},
message_begin) ->
{This2, {ok, VerAndType}} = read(This1, ubyte),
?VERSION_1 = VerAndType band ?VERSION_MASK,
{This3, {ok, SeqId}} = read(This2, ui32),
- case read(This3, string) of
- {This4, {ok, Name}} ->
- {This4, #protocol_message_begin{
+ {This4, {ok, NameSz}} = read(This3, ui32),
+ case read_name(This4, NameSz) of
+ {This5, {ok, Name}} ->
+ {This5, #protocol_message_begin{
name = binary_to_list(Name),
type = (VerAndType bsr ?TYPE_SHIFT_AMOUNT) band ?TYPE_BITS,
seqid = SeqId
}};
- {This4, Error} ->
- {This4, Error}
+ {This5, Error} ->
+ {This5, Error}
end;
read(This, message_end) ->
{This#t_compact{message_bytes_left = undefined}, ok};
@@ -401,6 +402,13 @@ read(This0, string) ->
{This1, {ok, Sz}} = read(This0, ui32),
read_data(This1, Sz).
+%% The name of a message, refused before it is read if it is longer than
+%% ?MAX_MESSAGE_NAME_SIZE.
+read_name(This, Sz) when Sz > ?MAX_MESSAGE_NAME_SIZE ->
+ {This, {error, {message_name_exceeds_maximum, ?MAX_MESSAGE_NAME_SIZE}}};
+read_name(This, Sz) ->
+ read_data(This, Sz).
+
-spec read_data(#t_compact{}, non_neg_integer()) ->
{#t_compact{}, {ok, binary()} | {error, _Reason}}.
read_data(This, 0) ->
diff --git a/lib/erl/test/test_thrift_max_message_size.erl
b/lib/erl/test/test_thrift_max_message_size.erl
index d9364ab02..176776716 100644
--- a/lib/erl/test/test_thrift_max_message_size.erl
+++ b/lib/erl/test/test_thrift_max_message_size.erl
@@ -139,6 +139,25 @@ name_test_() ->
|| {Name, Module, Base} <- protocols()
].
+%% A message name up to ?MAX_MESSAGE_NAME_SIZE bytes is read. A longer one is
+%% refused by message_begin before its bytes are taken from the transport.
+name_length_test_() ->
+ [
+ {Name,
+ ?_test(begin
+ AtMax = lists:duplicate(?MAX_MESSAGE_NAME_SIZE, $n),
+ Ok = message(Module, Base, AtMax, <<"x">>),
+ ?assertMatch({{ok, AtMax, _}, _, _}, read_call(new(Module, Ok,
Base), Ok)),
+ Long = message(Module, Base, [$n | AtMax], <<"x">>),
+ {Protocol, Result} = thrift_protocol:read(new(Module, Long,
Base), message_begin),
+ ?assertEqual(
+ {error, {message_name_exceeds_maximum,
?MAX_MESSAGE_NAME_SIZE}}, Result
+ ),
+ ?assert(byte_size(Long) - iolist_size(buffered(Protocol)) =< 8)
+ end)}
+ || {Name, Module, Base} <- protocols()
+ ].
+
%% Each message has the whole maximum to itself.
one_message_after_another_test_() ->
[
