Strictly validate annotations

This commit is contained in:
David Ansari 2024-09-18 10:03:32 +02:00
parent cd600bef8b
commit b1eb354385
9 changed files with 45 additions and 35 deletions

View File

@ -348,8 +348,8 @@ roundtrip(OpenConf, Body) ->
Msg0 = amqp10_msg:new(<<"my-tag">>, Body, true), Msg0 = amqp10_msg:new(<<"my-tag">>, Body, true),
Msg1 = amqp10_msg:set_application_properties(#{"a_key" => "a_value"}, Msg0), Msg1 = amqp10_msg:set_application_properties(#{"a_key" => "a_value"}, Msg0),
Msg2 = amqp10_msg:set_properties(Props, Msg1), Msg2 = amqp10_msg:set_properties(Props, Msg1),
Msg = amqp10_msg:set_message_annotations(#{<<"x-key">> => "x-value", Msg = amqp10_msg:set_message_annotations(#{<<"x-key 1">> => "value 1",
<<"x_key">> => "x_value"}, Msg2), <<"x-key 2">> => "value 2"}, Msg2),
ok = amqp10_client:send_msg(Sender, Msg), ok = amqp10_client:send_msg(Sender, Msg),
ok = amqp10_client:detach_link(Sender), ok = amqp10_client:detach_link(Sender),
await_link(Sender, {detached, normal}, link_detach_timeout), await_link(Sender, {detached, normal}, link_detach_timeout),
@ -364,8 +364,8 @@ roundtrip(OpenConf, Body) ->
% ct:pal(?LOW_IMPORTANCE, "roundtrip message Out: ~tp~nIn: ~tp~n", [OutMsg, Msg]), % ct:pal(?LOW_IMPORTANCE, "roundtrip message Out: ~tp~nIn: ~tp~n", [OutMsg, Msg]),
?assertMatch(Props, amqp10_msg:properties(OutMsg)), ?assertMatch(Props, amqp10_msg:properties(OutMsg)),
?assertEqual(#{<<"a_key">> => <<"a_value">>}, amqp10_msg:application_properties(OutMsg)), ?assertEqual(#{<<"a_key">> => <<"a_value">>}, amqp10_msg:application_properties(OutMsg)),
?assertMatch(#{<<"x-key">> := <<"x-value">>, ?assertMatch(#{<<"x-key 1">> := <<"value 1">>,
<<"x_key">> := <<"x_value">>}, amqp10_msg:message_annotations(OutMsg)), <<"x-key 2">> := <<"value 2">>}, amqp10_msg:message_annotations(OutMsg)),
?assertEqual([Body], amqp10_msg:body(OutMsg)), ?assertEqual([Body], amqp10_msg:body(OutMsg)),
ok. ok.

View File

@ -122,11 +122,11 @@ decode({described, Descriptor, {map, Fields} = Type}) ->
#'v1_0.application_properties'{} -> #'v1_0.application_properties'{} ->
#'v1_0.application_properties'{content = decode_map(Fields)}; #'v1_0.application_properties'{content = decode_map(Fields)};
#'v1_0.delivery_annotations'{} -> #'v1_0.delivery_annotations'{} ->
#'v1_0.delivery_annotations'{content = decode_map(Fields)}; #'v1_0.delivery_annotations'{content = decode_annotations(Fields)};
#'v1_0.message_annotations'{} -> #'v1_0.message_annotations'{} ->
#'v1_0.message_annotations'{content = decode_map(Fields)}; #'v1_0.message_annotations'{content = decode_annotations(Fields)};
#'v1_0.footer'{} -> #'v1_0.footer'{} ->
#'v1_0.footer'{content = decode_map(Fields)}; #'v1_0.footer'{content = decode_annotations(Fields)};
#'v1_0.amqp_value'{} -> #'v1_0.amqp_value'{} ->
#'v1_0.amqp_value'{content = Type}; #'v1_0.amqp_value'{content = Type};
Else -> Else ->
@ -149,6 +149,16 @@ decode(Other) ->
decode_map(Fields) -> decode_map(Fields) ->
[{decode(K), decode(V)} || {K, V} <- Fields]. [{decode(K), decode(V)} || {K, V} <- Fields].
%% "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." [3.2.10]
%% Since we already parse annotations here and neither the client nor server uses
%% reserved keys, we perform strict validation and crash if any reserved keys are used.
decode_annotations(Fields) ->
lists:map(fun({{symbol, <<"x-", _/binary>>} = K, V}) ->
{K, decode(V)}
end, Fields).
-spec encode_described(list | map | binary | annotations | '*', -spec encode_described(list | map | binary | annotations | '*',
non_neg_integer(), non_neg_integer(),
amqp10_frame()) -> amqp10_frame()) ->
@ -216,7 +226,7 @@ pprint(Other) -> Other.
-include_lib("eunit/include/eunit.hrl"). -include_lib("eunit/include/eunit.hrl").
encode_decode_test_() -> encode_decode_test_() ->
Data = [{{utf8, <<"k">>}, {binary, <<"v">>}}], Data = [{{symbol, <<"x-my key">>}, {binary, <<"my value">>}}],
Test = fun(M) -> [M] = decode_bin(iolist_to_binary(encode_bin(M))) end, Test = fun(M) -> [M] = decode_bin(iolist_to_binary(encode_bin(M))) end,
[ [
fun() -> Test(#'v1_0.application_properties'{content = Data}) end, fun() -> Test(#'v1_0.application_properties'{content = Data}) end,

View File

@ -412,14 +412,21 @@ footer_section() ->
annotations() -> annotations() ->
?LET(KvList, ?LET(KvList,
list({oneof([amqp_symbol(), list({non_reserved_annotation_key(),
amqp_ulong()]),
prefer_simple_type()}), prefer_simple_type()}),
begin begin
KvList1 = lists:uniq(fun({K, _V}) -> K end, KvList), KvList1 = lists:uniq(fun({K, _V}) -> K end, KvList),
lists:filter(fun({_K, V}) -> V =/= null end, KvList1) lists:filter(fun({_K, V}) -> V =/= null end, KvList1)
end). end).
non_reserved_annotation_key() ->
{symbol, ?LET(L,
?SIZED(Size, resize(Size * 10, list(ascii_char()))),
begin
Bin = list_to_binary(L) ,
<<"x-", Bin/binary>>
end)}.
sequence_no() -> sequence_no() ->
amqp_uint(). amqp_uint().

View File

@ -355,7 +355,7 @@ reliable_send_receive_with_outcomes(QType, Config) ->
Outcomes = [ Outcomes = [
accepted, accepted,
modified, modified,
{modified, true, false, #{<<"fruit">> => <<"banana">>}}, {modified, true, false, #{<<"x-fruit">> => <<"banana">>}},
{modified, false, true, #{}}, {modified, false, true, #{}},
rejected, rejected,
released released
@ -1124,7 +1124,7 @@ amqp_amqpl(QType, Config) ->
#{"my int" => -2}, #{"my int" => -2},
amqp10_msg:new(<<>>, Body1, true)))), amqp10_msg:new(<<>>, Body1, true)))),
%% Send with footer %% Send with footer
Footer = #'v1_0.footer'{content = [{{symbol, <<"my footer">>}, {ubyte, 255}}]}, Footer = #'v1_0.footer'{content = [{{symbol, <<"x-my footer">>}, {ubyte, 255}}]},
ok = amqp10_client:send_msg( ok = amqp10_client:send_msg(
Sender, Sender,
amqp10_msg:from_amqp_records( amqp10_msg:from_amqp_records(
@ -5155,7 +5155,7 @@ footer_checksum(FooterOpt, Config) ->
priority => 7, priority => 7,
ttl => 100_000}, ttl => 100_000},
amqp10_msg:set_delivery_annotations( amqp10_msg:set_delivery_annotations(
#{"a" => "b"}, #{"x-a" => "b"},
amqp10_msg:set_message_annotations( amqp10_msg:set_message_annotations(
#{"x-string" => "string-value", #{"x-string" => "string-value",
"x-int" => 3, "x-int" => 3,

View File

@ -298,10 +298,10 @@ module Test =
use c = connectAnon uri use c = connectAnon uri
let sender, receiver = senderReceiver c "test" "/queues/message_annotations" let sender, receiver = senderReceiver c "test" "/queues/message_annotations"
let ann = MessageAnnotations() let ann = MessageAnnotations()
let k1 = Symbol "key1" let k1 = Symbol "x-key1"
let k2 = Symbol "key2" let k2 = Symbol "x-key2"
ann.[Symbol "key1"] <- "value1" ann.[Symbol "x-key1"] <- "value1"
ann.[Symbol "key2"] <- "value2" ann.[Symbol "x-key2"] <- "value2"
let m = new Message("testing annotations", MessageAnnotations = ann) let m = new Message("testing annotations", MessageAnnotations = ann)
sender.Send m sender.Send m
let m' = receive receiver let m' = receive receiver

View File

@ -524,8 +524,6 @@ amqp_amqpl(_Config) ->
durable = true}, durable = true},
MAC = [ MAC = [
{{symbol, <<"x-stream-filter">>}, {utf8, <<"apple">>}}, {{symbol, <<"x-stream-filter">>}, {utf8, <<"apple">>}},
thead2(list, [utf8(<<"l">>)]),
thead2(map, [{utf8(<<"k">>), utf8(<<"v">>)}]),
thead2('x-list', list, [utf8(<<"l">>)]), thead2('x-list', list, [utf8(<<"l">>)]),
thead2('x-map', map, [{utf8(<<"k">>), utf8(<<"v">>)}]) thead2('x-map', map, [{utf8(<<"k">>), utf8(<<"v">>)}])
], ],
@ -591,9 +589,6 @@ amqp_amqpl(_Config) ->
?assertMatch(#'P_basic'{expiration = <<"20000">>}, Props), ?assertMatch(#'P_basic'{expiration = <<"20000">>}, Props),
?assertMatch({_, longstr, <<"apple">>}, header(<<"x-stream-filter">>, HL)), ?assertMatch({_, longstr, <<"apple">>}, header(<<"x-stream-filter">>, HL)),
%% these are not coverted as not x- headers
?assertEqual(undefined, header(<<"list">>, HL)),
?assertEqual(undefined, header(<<"map">>, HL)),
?assertMatch({_ ,array, [{longstr,<<"l">>}]}, header(<<"x-list">>, HL)), ?assertMatch({_ ,array, [{longstr,<<"l">>}]}, header(<<"x-list">>, HL)),
?assertMatch({_, table, [{<<"k">>,longstr,<<"v">>}]}, header(<<"x-map">>, HL)), ?assertMatch({_, table, [{<<"k">>,longstr,<<"v">>}]}, header(<<"x-map">>, HL)),

View File

@ -244,21 +244,21 @@ amqp_dead_letter(Config) ->
Msg1 = case Seq rem 2 of Msg1 = case Seq rem 2 of
0 -> 0 ->
amqp10_msg:set_message_annotations( amqp10_msg:set_message_annotations(
#{<<"k1">> => Seq}, Msg0); #{<<"x-k1">> => Seq}, Msg0);
1 -> 1 ->
Msg0 Msg0
end, end,
Msg2 = case Seq rem 3 of Msg2 = case Seq rem 3 of
0 -> 0 ->
amqp10_msg:set_application_properties( amqp10_msg:set_application_properties(
#{<<"k2">> => Seq}, Msg1); #{<<"x-k2">> => Seq}, Msg1);
_ -> _ ->
Msg1 Msg1
end, end,
Msg = case Seq rem 4 of Msg = case Seq rem 4 of
0 -> 0 ->
amqp10_msg:set_delivery_annotations( amqp10_msg:set_delivery_annotations(
#{<<"k3">> => Seq}, Msg2); #{<<"x-k3">> => Seq}, Msg2);
_ -> _ ->
Msg2 Msg2
end, end,

View File

@ -265,7 +265,7 @@ amqp_to_mqtt_reply_to(_Config) ->
amqp_to_mqtt_footer(_Config) -> amqp_to_mqtt_footer(_Config) ->
Body = <<"hey">>, Body = <<"hey">>,
Footer = #'v1_0.footer'{content = [{{symbol, <<"key">>}, {utf8, <<"value">>}}]}, Footer = #'v1_0.footer'{content = [{{symbol, <<"x-key">>}, {utf8, <<"value">>}}]},
%% We can translate, but lose the footer. %% We can translate, but lose the footer.
#mqtt_msg{payload = Payload} = amqp_to_mqtt([#'v1_0.data'{content = Body}, Footer]), #mqtt_msg{payload = Payload} = amqp_to_mqtt([#'v1_0.data'{content = Body}, Footer]),
?assertEqual(<<"hey">>, iolist_to_binary(Payload)). ?assertEqual(<<"hey">>, iolist_to_binary(Payload)).
@ -404,8 +404,6 @@ amqp_mqtt(_Config) ->
durable = true}, durable = true},
MAC = [ MAC = [
{{symbol, <<"x-stream-filter">>}, {utf8, <<"apple">>}}, {{symbol, <<"x-stream-filter">>}, {utf8, <<"apple">>}},
thead2(list, [utf8(<<"l">>)]),
thead2(map, [{utf8(<<"k">>), utf8(<<"v">>)}]),
thead2('x-list', list, [utf8(<<"l">>)]), thead2('x-list', list, [utf8(<<"l">>)]),
thead2('x-map', map, [{utf8(<<"k">>), utf8(<<"v">>)}]) thead2('x-map', map, [{utf8(<<"k">>), utf8(<<"v">>)}])
], ],

View File

@ -121,12 +121,12 @@ test_amqp10_destination(Config, Src, Dest, Sess, Protocol, ProtocolSrc) ->
end}, end},
{<<"dest-message-annotations">>, {<<"dest-message-annotations">>,
case MapConfig of case MapConfig of
true -> true ->
#{<<"message-ann-key">> => #{<<"x-message-ann-key">> =>
<<"message-ann-value">>}; <<"message-ann-value">>};
_ -> _ ->
[{<<"message-ann-key">>, [{<<"x-message-ann-key">>,
<<"message-ann-value">>}] <<"message-ann-value">>}]
end}]), end}]),
Msg = publish_expect(Sess, Src, Dest, <<"tag1">>, <<"hello">>), Msg = publish_expect(Sess, Src, Dest, <<"tag1">>, <<"hello">>),
AppProps = amqp10_msg:application_properties(Msg), AppProps = amqp10_msg:application_properties(Msg),
@ -138,7 +138,7 @@ test_amqp10_destination(Config, Src, Dest, Sess, Protocol, ProtocolSrc) ->
<<"app-prop-key">> := <<"app-prop-value">>}), <<"app-prop-key">> := <<"app-prop-value">>}),
(AppProps)), (AppProps)),
?assertEqual(undefined, maps:get(<<"delivery_mode">>, AppProps, undefined)), ?assertEqual(undefined, maps:get(<<"delivery_mode">>, AppProps, undefined)),
?assertMatch((#{<<"message-ann-key">> := <<"message-ann-value">>}), ?assertMatch((#{<<"x-message-ann-key">> := <<"message-ann-value">>}),
(amqp10_msg:message_annotations(Msg))). (amqp10_msg:message_annotations(Msg))).
simple_amqp10_src(Config) -> simple_amqp10_src(Config) ->