Skip to content

Commit 61e82d8

Browse files
michaelklishinmichaelklishin
committed
DQT: be extra defensive with a few more possible JSON null value representations
References #11541, #12109, #12821, #13837.
1 parent 20ed2a6 commit 61e82d8

File tree

4 files changed

+175
-21
lines changed

4 files changed

+175
-21
lines changed

‎deps/rabbit/src/rabbit_queue_type.erl‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -824,10 +824,11 @@ inject_dqt(VHost) when is_list(VHost) ->
824824
inject_dqt(rabbit_data_coercion:to_map(VHost));
825825
inject_dqt(M) when is_map(M) ->
826826
RawDQT = maps:get(default_queue_type, M, undefined),
827-
%% Handle undefined, <<"undefined">> (rabbitmq-server#10469),
828-
%% "undefined", or a missing key by falling back to the node default
827+
%% See rabbitmq/rabbitmq-server#10469
829828
DQT = case RawDQT of
830829
undefined -> default();
830+
null -> default();
831+
nil -> default();
831832
<<"undefined">> -> default();
832833
"undefined" -> default();
833834
Valid -> Valid

‎deps/rabbit/src/rabbit_vhost.erl‎

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -309,7 +309,18 @@ update_metadata(Name, Metadata0, _ActingUser) when is_map(Metadata0) andalso map
309309
end;
310310
update_metadata(Name, Metadata0, ActingUser) ->
311311
KnownKeys = [description, tags, default_queue_type, protected_from_deletion],
312-
Metadata = maps:with(KnownKeys, Metadata0),
312+
Metadata1 = maps:with(KnownKeys, Metadata0),
313+
%% See rabbitmq/rabbitmq-server#10469
314+
Metadata = case Metadata1 of
315+
#{default_queue_type := <<"undefined">>} ->
316+
maps:remove(default_queue_type, Metadata1);
317+
#{default_queue_type := null} ->
318+
maps:remove(default_queue_type, Metadata1);
319+
#{default_queue_type := nil} ->
320+
maps:remove(default_queue_type, Metadata1);
321+
_ ->
322+
Metadata1
323+
end,
313324

314325
case rabbit_db_vhost:merge_metadata(Name, Metadata) of
315326
{ok, VHost} ->
@@ -715,6 +726,10 @@ i(metadata, VHost) ->
715726
M#{default_queue_type => DQT};
716727
M = #{default_queue_type := <<"undefined">>} ->
717728
M#{default_queue_type => DQT};
729+
M = #{default_queue_type := null} ->
730+
M#{default_queue_type => DQT};
731+
M = #{default_queue_type := nil} ->
732+
M#{default_queue_type => DQT};
718733
M = #{default_queue_type := QT} ->
719734
M#{default_queue_type => rabbit_queue_type:short_alias_of(QT)};
720735
M when is_map(M) ->

‎deps/rabbit/src/vhost.erl‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -217,6 +217,15 @@ new_metadata(Description, Tags, undefined) ->
217217
#{description => Description,
218218
default_queue_type => rabbit_queue_type:default_alias(),
219219
tags => Tags};
220+
new_metadata(Description, Tags, <<"undefined">>) ->
221+
%% See rabbitmq/rabbitmq-server#10469
222+
new_metadata(Description, Tags, undefined);
223+
new_metadata(Description, Tags, null) ->
224+
%% JSON null (thoas), see rabbitmq/rabbitmq-server#10469
225+
new_metadata(Description, Tags, undefined);
226+
new_metadata(Description, Tags, nil) ->
227+
%% JSON null (Elixir JSON), see rabbitmq/rabbitmq-server#10469
228+
new_metadata(Description, Tags, undefined);
220229
new_metadata(Description, Tags, DefaultQueueType) ->
221230
#{description => Description,
222231
tags => Tags,

‎deps/rabbit/test/unit_default_queue_type_SUITE.erl‎

Lines changed: 147 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,18 @@
1717
all() ->
1818
[
1919
inject_dqt_undefined_binary,
20-
inject_dqt_preserves_existing_metadata
20+
inject_dqt_null,
21+
inject_dqt_nil,
22+
inject_dqt_preserves_existing_metadata,
23+
new_metadata_sanitizes_undefined_binary,
24+
new_metadata_sanitizes_null,
25+
new_metadata_sanitizes_nil,
26+
vhost_import_with_undefined_dqt,
27+
vhost_import_with_null_dqt,
28+
vhost_import_with_nil_dqt,
29+
update_metadata_sanitizes_undefined_binary,
30+
update_metadata_sanitizes_null,
31+
update_metadata_sanitizes_nil
2132
].
2233

2334
%% -------------------------------------------------------------------
@@ -31,20 +42,6 @@ init_per_suite(Config) ->
3142
end_per_suite(Config) ->
3243
rabbit_ct_helpers:run_teardown_steps(Config).
3344

34-
init_per_group(Group, Config) ->
35-
Config1 = rabbit_ct_helpers:set_config(Config, [
36-
{rmq_nodename_suffix, Group},
37-
{rmq_nodes_count, 1}
38-
]),
39-
rabbit_ct_helpers:run_steps(Config1,
40-
rabbit_ct_broker_helpers:setup_steps() ++
41-
rabbit_ct_client_helpers:setup_steps()).
42-
43-
end_per_group(_Group, Config) ->
44-
rabbit_ct_helpers:run_steps(Config,
45-
rabbit_ct_client_helpers:teardown_steps() ++
46-
rabbit_ct_broker_helpers:teardown_steps()).
47-
4845
init_per_testcase(Testcase, Config) ->
4946
Config1 = rabbit_ct_helpers:set_config(Config, [
5047
{rmq_nodename_suffix, Testcase},
@@ -65,8 +62,6 @@ end_per_testcase(Testcase, Config) ->
6562
%% Test Cases
6663
%% -------------------------------------------------------------------
6764

68-
%% When default_queue_type is the binary <<"undefined">> (exported from an older versions),
69-
%% inject the default. See rabbitmq/rabbitmq-server#10469
7065
inject_dqt_undefined_binary(Config) ->
7166
Expected = rpc(Config, 0, rabbit_queue_type, default_alias, []),
7267
Input = #{default_queue_type => <<"undefined">>, name => <<"/">>},
@@ -75,7 +70,22 @@ inject_dqt_undefined_binary(Config) ->
7570
rpc(Config, 0, rabbit_queue_type, inject_dqt, [Input]),
7671
ok.
7772

78-
%% Existing metadata keys should be preserved when injecting DQT
73+
inject_dqt_null(Config) ->
74+
Expected = rpc(Config, 0, rabbit_queue_type, default_alias, []),
75+
Input = #{default_queue_type => null, name => <<"/">>},
76+
#{default_queue_type := Expected,
77+
metadata := #{default_queue_type := Expected}} =
78+
rpc(Config, 0, rabbit_queue_type, inject_dqt, [Input]),
79+
ok.
80+
81+
inject_dqt_nil(Config) ->
82+
Expected = rpc(Config, 0, rabbit_queue_type, default_alias, []),
83+
Input = #{default_queue_type => nil, name => <<"/">>},
84+
#{default_queue_type := Expected,
85+
metadata := #{default_queue_type := Expected}} =
86+
rpc(Config, 0, rabbit_queue_type, inject_dqt, [Input]),
87+
ok.
88+
7989
inject_dqt_preserves_existing_metadata(Config) ->
8090
Expected = rpc(Config, 0, rabbit_queue_type, default_alias, []),
8191
Input = #{
@@ -91,3 +101,122 @@ inject_dqt_preserves_existing_metadata(Config) ->
91101
tags := [replicate]}} =
92102
rpc(Config, 0, rabbit_queue_type, inject_dqt, [Input]),
93103
ok.
104+
105+
new_metadata_sanitizes_undefined_binary(Config) ->
106+
Expected = rpc(Config, 0, rabbit_queue_type, default_alias, []),
107+
#{default_queue_type := Expected} =
108+
rpc(Config, 0, vhost, new_metadata, [<<"description">>, [], <<"undefined">>]),
109+
#{default_queue_type := Expected} =
110+
rpc(Config, 0, vhost, new_metadata, [<<"description">>, [], undefined]),
111+
#{default_queue_type := <<"quorum">>} =
112+
rpc(Config, 0, vhost, new_metadata, [<<"description">>, [], <<"quorum">>]),
113+
ok.
114+
115+
new_metadata_sanitizes_null(Config) ->
116+
Expected = rpc(Config, 0, rabbit_queue_type, default_alias, []),
117+
#{default_queue_type := Expected} =
118+
rpc(Config, 0, vhost, new_metadata, [<<"description">>, [], null]),
119+
ok.
120+
121+
new_metadata_sanitizes_nil(Config) ->
122+
Expected = rpc(Config, 0, rabbit_queue_type, default_alias, []),
123+
#{default_queue_type := Expected} =
124+
rpc(Config, 0, vhost, new_metadata, [<<"description">>, [], nil]),
125+
ok.
126+
127+
vhost_import_with_undefined_dqt(Config) ->
128+
VHost = <<"import-undefined-dqt-test">>,
129+
Expected = rpc(Config, 0, rabbit_queue_type, default_alias, []),
130+
try
131+
ok = rpc(Config, 0, rabbit_vhost, put_vhost,
132+
[VHost, <<"test description">>, [], <<"undefined">>, false, <<"test-user">>]),
133+
V = rpc(Config, 0, rabbit_vhost, lookup, [VHost]),
134+
Metadata = rpc(Config, 0, vhost, get_metadata, [V]),
135+
#{default_queue_type := StoredDQT} = Metadata,
136+
Expected = StoredDQT
137+
after
138+
rpc(Config, 0, rabbit_vhost, delete, [VHost, <<"test-user">>])
139+
end,
140+
ok.
141+
142+
vhost_import_with_null_dqt(Config) ->
143+
VHost = <<"import-null-dqt-test">>,
144+
Expected = rpc(Config, 0, rabbit_queue_type, default_alias, []),
145+
try
146+
ok = rpc(Config, 0, rabbit_vhost, put_vhost,
147+
[VHost, <<"test description">>, [], null, false, <<"test-user">>]),
148+
V = rpc(Config, 0, rabbit_vhost, lookup, [VHost]),
149+
Metadata = rpc(Config, 0, vhost, get_metadata, [V]),
150+
#{default_queue_type := StoredDQT} = Metadata,
151+
Expected = StoredDQT
152+
after
153+
rpc(Config, 0, rabbit_vhost, delete, [VHost, <<"test-user">>])
154+
end,
155+
ok.
156+
157+
vhost_import_with_nil_dqt(Config) ->
158+
VHost = <<"import-nil-dqt-test">>,
159+
Expected = rpc(Config, 0, rabbit_queue_type, default_alias, []),
160+
try
161+
ok = rpc(Config, 0, rabbit_vhost, put_vhost,
162+
[VHost, <<"test description">>, [], nil, false, <<"test-user">>]),
163+
V = rpc(Config, 0, rabbit_vhost, lookup, [VHost]),
164+
Metadata = rpc(Config, 0, vhost, get_metadata, [V]),
165+
#{default_queue_type := StoredDQT} = Metadata,
166+
Expected = StoredDQT
167+
after
168+
rpc(Config, 0, rabbit_vhost, delete, [VHost, <<"test-user">>])
169+
end,
170+
ok.
171+
172+
update_metadata_sanitizes_undefined_binary(Config) ->
173+
VHost = <<"update-metadata-undefined-dqt-test">>,
174+
try
175+
ok = rpc(Config, 0, rabbit_vhost, put_vhost,
176+
[VHost, <<"test">>, [], <<"classic">>, false, <<"test-user">>]),
177+
V1 = rpc(Config, 0, rabbit_vhost, lookup, [VHost]),
178+
#{default_queue_type := <<"classic">>} = rpc(Config, 0, vhost, get_metadata, [V1]),
179+
%% update_metadata should not overwrite with <<"undefined">>
180+
ok = rpc(Config, 0, rabbit_vhost, update_metadata,
181+
[VHost, #{default_queue_type => <<"undefined">>}, <<"test-user">>]),
182+
V2 = rpc(Config, 0, rabbit_vhost, lookup, [VHost]),
183+
#{default_queue_type := StoredDQT} = rpc(Config, 0, vhost, get_metadata, [V2]),
184+
<<"classic">> = StoredDQT
185+
after
186+
rpc(Config, 0, rabbit_vhost, delete, [VHost, <<"test-user">>])
187+
end,
188+
ok.
189+
190+
update_metadata_sanitizes_null(Config) ->
191+
VHost = <<"update-metadata-null-dqt-test">>,
192+
try
193+
ok = rpc(Config, 0, rabbit_vhost, put_vhost,
194+
[VHost, <<"test">>, [], <<"classic">>, false, <<"test-user">>]),
195+
V1 = rpc(Config, 0, rabbit_vhost, lookup, [VHost]),
196+
#{default_queue_type := <<"classic">>} = rpc(Config, 0, vhost, get_metadata, [V1]),
197+
ok = rpc(Config, 0, rabbit_vhost, update_metadata,
198+
[VHost, #{default_queue_type => null}, <<"test-user">>]),
199+
V2 = rpc(Config, 0, rabbit_vhost, lookup, [VHost]),
200+
#{default_queue_type := StoredDQT} = rpc(Config, 0, vhost, get_metadata, [V2]),
201+
<<"classic">> = StoredDQT
202+
after
203+
rpc(Config, 0, rabbit_vhost, delete, [VHost, <<"test-user">>])
204+
end,
205+
ok.
206+
207+
update_metadata_sanitizes_nil(Config) ->
208+
VHost = <<"update-metadata-nil-dqt-test">>,
209+
try
210+
ok = rpc(Config, 0, rabbit_vhost, put_vhost,
211+
[VHost, <<"test">>, [], <<"classic">>, false, <<"test-user">>]),
212+
V1 = rpc(Config, 0, rabbit_vhost, lookup, [VHost]),
213+
#{default_queue_type := <<"classic">>} = rpc(Config, 0, vhost, get_metadata, [V1]),
214+
ok = rpc(Config, 0, rabbit_vhost, update_metadata,
215+
[VHost, #{default_queue_type => nil}, <<"test-user">>]),
216+
V2 = rpc(Config, 0, rabbit_vhost, lookup, [VHost]),
217+
#{default_queue_type := StoredDQT} = rpc(Config, 0, vhost, get_metadata, [V2]),
218+
<<"classic">> = StoredDQT
219+
after
220+
rpc(Config, 0, rabbit_vhost, delete, [VHost, <<"test-user">>])
221+
end,
222+
ok.

0 commit comments

Comments
 (0)