Skip to content

Commit 358ff79

Browse files
committed
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.
1 parent e4d20bb commit 358ff79

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
@@ -514,12 +514,16 @@ handle_cast({frame_body, FrameBody},
514514
noreply(State);
515515
{stop, _, _} = Stop ->
516516
Stop
517-
catch exit:#'v1_0.error'{} = Error ->
518-
log_error_and_close_session(Error, State0);
519-
exit:normal ->
517+
catch exit:normal ->
520518
{stop, normal, State0};
519+
exit:#'v1_0.error'{} = Error ->
520+
log_error_and_close_session(Error, State0);
521521
_:Reason:Stacktrace ->
522-
{stop, {Reason, Stacktrace}, State0}
522+
Description = unicode:characters_to_binary(
523+
lists:flatten(io_lib:format("~tp~n~tp", [Reason, Stacktrace]))),
524+
Err = #'v1_0.error'{condition = ?V_1_0_AMQP_ERROR_INTERNAL_ERROR,
525+
description = {utf8, Description}},
526+
log_error_and_close_session(Err, State0)
523527
end;
524528
handle_cast({queue_event, _, _} = QEvent, State0) ->
525529
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
@@ -155,7 +155,8 @@ groups() ->
155155
tcp_back_pressure_rabbitmq_internal_flow_classic_queue,
156156
tcp_back_pressure_rabbitmq_internal_flow_quorum_queue,
157157
session_max_per_connection,
158-
link_max_per_session
158+
link_max_per_session,
159+
reserved_annotation
159160
]},
160161

161162
{cluster_size_3, [shuffle],
@@ -5917,6 +5918,32 @@ link_max_per_session(Config) ->
59175918
flush(test_succeeded),
59185919
ok = rpc(Config, application, set_env, [App, Par, Default]).
59195920

5921+
reserved_annotation(Config) ->
5922+
OpnConf = connection_config(Config),
5923+
{ok, Connection} = amqp10_client:open_connection(OpnConf),
5924+
{ok, Session} = amqp10_client:begin_session(Connection),
5925+
TargetAddr = rabbitmq_amqp_address:exchange(<<"amq.fanout">>),
5926+
{ok, Sender} = amqp10_client:attach_sender_link_sync(
5927+
Session, <<"sender">>, TargetAddr, settled),
5928+
ok = wait_for_credit(Sender),
5929+
5930+
Msg = amqp10_msg:set_message_annotations(
5931+
#{<<"reserved-key">> => 1},
5932+
amqp10_msg:new(<<"tag">>, <<"payload">>, true)),
5933+
ok = amqp10_client:send_msg(Sender, Msg),
5934+
receive
5935+
{amqp10_event,
5936+
{session, Session,
5937+
{ended,
5938+
#'v1_0.error'{description = {utf8, Description}}}}} ->
5939+
?assertMatch(
5940+
<<"{reserved_annotation_key,{symbol,<<\"reserved-key\">>}}", _/binary>>,
5941+
Description)
5942+
after 5000 -> flush(missing_ended),
5943+
ct:fail({missing_event, ?LINE})
5944+
end,
5945+
ok = close_connection_sync(Connection).
5946+
59205947
%% internal
59215948
%%
59225949

0 commit comments

Comments
 (0)