Skip to content

Commit 843a06d

Browse files
ansdmergify[bot]
authored andcommitted
Provide clear error message for reserved annotation keys
As described in https://docs.oasis-open.org/amqp/core/v1.0/os/amqp-core-messaging-v1.0-os.html#type-annotations > The annotations type is a map where the keys are restricted to be of type symbol or of type ulong. > All ulong keys, and all symbolic keys except those beginning with "x-" are reserved. Prior to this commit, if an AMQP client used a reserved annotation key, the entire AMQP connection terminated with a function_clause error message that might be difficult to understand for client libs: ``` <<"Session error: function_clause\n[{amqp10_framing,'-decode_annotations/1-fun-0-',\n [{{symbol,<<\"aa\">>},{utf8,<<\"bbb\">>}}],\n [{file,\"amqp10_framing.erl\"},{line,158}]},\n {lists,map,2,[{file,\"lists.erl\"},{line,1559}]},\n {amqp10_framing,decode,1,[{file,\"amqp10_framing.erl\"},{line,127}]},\n {lists,map_1,2,[{file,\"lists.erl\"},{line,1564}]},\n {lists,map,2,[{file,\"lists.erl\"},{line,1559}]},\n {mc_amqp,init,1,[{file,\"mc_amqp.erl\"},{line,102}]},\n {mc,init,4,[{file,\"mc.erl\"},{line,150}]},\n {rabbit_amqp_session,incoming_link_transfer,4,\n [{file,\"rabbit_amqp_session.erl\"},{line,2341}]}]">> ``` This commit ends only the session and provides a clearer error message. (cherry picked from commit 358ff79)
1 parent 62c34d3 commit 843a06d

File tree

3 files changed

+40
-7
lines changed

3 files changed

+40
-7
lines changed

deps/amqp10_common/src/amqp10_framing.erl

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -153,10 +153,12 @@ decode_map(Fields) ->
153153
%% or of type ulong. All ulong keys, and all symbolic keys except those beginning
154154
%% with "x-" are reserved." [3.2.10]
155155
%% Since we already parse annotations here and neither the client nor server uses
156-
%% reserved keys, we perform strict validation and crash if any reserved keys are used.
156+
%% reserved keys, we perform strict validation and throw if any reserved keys are used.
157157
decode_annotations(Fields) ->
158158
lists:map(fun({{symbol, <<"x-", _/binary>>} = K, V}) ->
159-
{K, decode(V)}
159+
{K, decode(V)};
160+
({ReservedKey, _V}) ->
161+
throw({reserved_annotation_key, ReservedKey})
160162
end, Fields).
161163

162164
-spec encode_described(list | map | binary | annotations | '*',

deps/rabbit/src/rabbit_amqp_session.erl

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -508,12 +508,16 @@ handle_cast({frame_body, FrameBody},
508508
noreply(State);
509509
{stop, _, _} = Stop ->
510510
Stop
511-
catch exit:#'v1_0.error'{} = Error ->
512-
log_error_and_close_session(Error, State0);
513-
exit:normal ->
511+
catch exit:normal ->
514512
{stop, normal, State0};
513+
exit:#'v1_0.error'{} = Error ->
514+
log_error_and_close_session(Error, State0);
515515
_:Reason:Stacktrace ->
516-
{stop, {Reason, Stacktrace}, State0}
516+
Description = unicode:characters_to_binary(
517+
lists:flatten(io_lib:format("~tp~n~tp", [Reason, Stacktrace]))),
518+
Err = #'v1_0.error'{condition = ?V_1_0_AMQP_ERROR_INTERNAL_ERROR,
519+
description = {utf8, Description}},
520+
log_error_and_close_session(Err, State0)
517521
end;
518522
handle_cast({queue_event, _, _} = QEvent, State0) ->
519523
try handle_queue_event(QEvent, State0) of

deps/rabbit/test/amqp_client_SUITE.erl

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -144,7 +144,8 @@ groups() ->
144144
tcp_back_pressure_rabbitmq_internal_flow_classic_queue,
145145
tcp_back_pressure_rabbitmq_internal_flow_quorum_queue,
146146
session_max_per_connection,
147-
link_max_per_session
147+
link_max_per_session,
148+
reserved_annotation
148149
]},
149150

150151
{cluster_size_3, [shuffle],
@@ -5894,6 +5895,32 @@ link_max_per_session(Config) ->
58945895
flush(test_succeeded),
58955896
ok = rpc(Config, application, set_env, [App, Par, Default]).
58965897

5898+
reserved_annotation(Config) ->
5899+
OpnConf = connection_config(Config),
5900+
{ok, Connection} = amqp10_client:open_connection(OpnConf),
5901+
{ok, Session} = amqp10_client:begin_session(Connection),
5902+
TargetAddr = rabbitmq_amqp_address:exchange(<<"amq.fanout">>),
5903+
{ok, Sender} = amqp10_client:attach_sender_link_sync(
5904+
Session, <<"sender">>, TargetAddr, settled),
5905+
ok = wait_for_credit(Sender),
5906+
5907+
Msg = amqp10_msg:set_message_annotations(
5908+
#{<<"reserved-key">> => 1},
5909+
amqp10_msg:new(<<"tag">>, <<"payload">>, true)),
5910+
ok = amqp10_client:send_msg(Sender, Msg),
5911+
receive
5912+
{amqp10_event,
5913+
{session, Session,
5914+
{ended,
5915+
#'v1_0.error'{description = {utf8, Description}}}}} ->
5916+
?assertMatch(
5917+
<<"{reserved_annotation_key,{symbol,<<\"reserved-key\">>}}", _/binary>>,
5918+
Description)
5919+
after 5000 -> flush(missing_ended),
5920+
ct:fail({missing_event, ?LINE})
5921+
end,
5922+
ok = close_connection_sync(Connection).
5923+
58975924
%% internal
58985925
%%
58995926

0 commit comments

Comments
 (0)