Skip to content

Commit 990b14d

Browse files
lukebakkenmergify[bot]
authored andcommitted
Fix error popup text display
When validation fails for a policy parameter, the resulting popup can't be read due to one extra binary encoding as well as code that escapes HTML entites. Since the EJS template uses `<%= >` for the popup, it will display the text as-is, and not render any HTML. (cherry picked from commit 5f5bf81)
1 parent 3c349b3 commit 990b14d

File tree

8 files changed

+15
-59
lines changed

8 files changed

+15
-59
lines changed

deps/rabbit/src/rabbit_definitions.erl

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -709,6 +709,8 @@ format({shutdown, _} = Error) ->
709709
rabbit_log:debug("Metadata store is unavailable: ~p", [Error]),
710710
rabbit_data_coercion:to_binary(
711711
rabbit_misc:format("Metadata store is unavailable. Please try again.", []));
712+
format(E) when is_binary(E) ->
713+
E;
712714
format(E) ->
713715
rabbit_data_coercion:to_binary(rabbit_misc:format("~tp", [E])).
714716

@@ -732,8 +734,8 @@ add_parameter(VHost, Param, Username) ->
732734
case Result of
733735
ok -> ok;
734736
{error_string, E} ->
735-
S = rabbit_misc:format(" (~ts/~ts/~ts)", [VHost, Comp, Key]),
736-
exit(rabbit_data_coercion:to_binary(rabbit_misc:escape_html_tags(E ++ S)))
737+
S = rabbit_misc:format(" (vhost: \"~ts\" / component: \"~ts\" / key: \"~ts\")", [VHost, Comp, Key]),
738+
exit(rabbit_data_coercion:to_utf8_binary(E ++ S))
737739
end.
738740

739741
add_global_parameter(Param, Username) ->
@@ -769,8 +771,8 @@ add_policy(VHost, Param, Username) ->
769771
maps:get('apply-to', Param, <<"all">>),
770772
Username) of
771773
ok -> ok;
772-
{error_string, E} -> S = rabbit_misc:format(" (~ts/~ts)", [VHost, Key]),
773-
exit(rabbit_data_coercion:to_binary(rabbit_misc:escape_html_tags(E ++ S)))
774+
{error_string, E} -> S = rabbit_misc:format(" (vhost: \"~ts\" key: \"~ts\")", [VHost, Key]),
775+
exit(rabbit_data_coercion:to_utf8_binary(E ++ S))
774776
end.
775777

776778
-spec add_vhost(map(), rabbit_types:username()) -> ok | no_return().

deps/rabbit_common/src/rabbit_misc.erl

Lines changed: 1 addition & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@
6868
-export([get_parent/0]).
6969
-export([store_proc_name/1, store_proc_name/2, get_proc_name/0]).
7070
-export([moving_average/4]).
71-
-export([escape_html_tags/1, b64decode_or_throw/1]).
71+
-export([b64decode_or_throw/1]).
7272
-export([get_env/3]).
7373
-export([get_channel_operation_timeout/0]).
7474
-export([random/1]).
@@ -1181,25 +1181,6 @@ moving_average(Time, HalfLife, Next, Current) ->
11811181
random(N) ->
11821182
rand:uniform(N).
11831183

1184-
-spec escape_html_tags(string()) -> binary().
1185-
1186-
escape_html_tags(S) ->
1187-
escape_html_tags(rabbit_data_coercion:to_list(S), []).
1188-
1189-
1190-
-spec escape_html_tags(string(), string()) -> binary().
1191-
1192-
escape_html_tags([], Acc) ->
1193-
rabbit_data_coercion:to_binary(lists:reverse(Acc));
1194-
escape_html_tags("<" ++ Rest, Acc) ->
1195-
escape_html_tags(Rest, lists:reverse("&lt;", Acc));
1196-
escape_html_tags(">" ++ Rest, Acc) ->
1197-
escape_html_tags(Rest, lists:reverse("&gt;", Acc));
1198-
escape_html_tags("&" ++ Rest, Acc) ->
1199-
escape_html_tags(Rest, lists:reverse("&amp;", Acc));
1200-
escape_html_tags([C | Rest], Acc) ->
1201-
escape_html_tags(Rest, [C | Acc]).
1202-
12031184
%% If the server we are talking to has non-standard net_ticktime, and
12041185
%% our connection lasts a while, we could get disconnected because of
12051186
%% a timeout unless we set our ticktime to be the same. So let's do

deps/rabbitmq_management/src/rabbit_mgmt_util.erl

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -876,8 +876,7 @@ with_vhost_and_props(Fun, ReqData, Context) ->
876876
bad_request(Error, ReqData1, Context)
877877
end;
878878
{error, Reason} ->
879-
bad_request(rabbit_mgmt_format:escape_html_tags(Reason),
880-
ReqData1, Context)
879+
bad_request(Reason, ReqData1, Context)
881880
end
882881
end.
883882

deps/rabbitmq_management/src/rabbit_mgmt_wm_healthchecks.erl

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,8 +48,7 @@ to_json(ReqData, Context) ->
4848
{badrpc, Err} ->
4949
failure(rabbit_mgmt_format:print("~tp", Err), ReqData, Context);
5050
{error_string, Err} ->
51-
S = rabbit_mgmt_format:escape_html_tags(
52-
rabbit_data_coercion:to_list(rabbit_mgmt_format:print(Err))),
51+
S = rabbit_mgmt_format:print(Err),
5352
failure(S, ReqData, Context)
5453
end.
5554

deps/rabbitmq_management/src/rabbit_mgmt_wm_parameter.erl

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -61,10 +61,7 @@ accept_content(ReqData0, Context = #context{user = User}) ->
6161
ok ->
6262
{true, ReqData, Context};
6363
{error_string, Reason} ->
64-
S = rabbit_mgmt_format:escape_html_tags(
65-
rabbit_data_coercion:to_list(Reason)),
66-
rabbit_mgmt_util:bad_request(
67-
rabbit_data_coercion:to_binary(S), ReqData, Context)
64+
rabbit_mgmt_util:bad_request(Reason, ReqData, Context)
6865
end
6966
end)
7067
end.

deps/rabbitmq_management/src/rabbit_mgmt_wm_policy.erl

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -60,8 +60,7 @@ accept_content(ReqData0, Context = #context{user = #user{username = Username}})
6060
ok ->
6161
{true, ReqData, Context};
6262
{error_string, Reason} ->
63-
rabbit_mgmt_util:bad_request(
64-
rabbit_mgmt_format:escape_html_tags(Reason), ReqData, Context)
63+
rabbit_mgmt_util:bad_request(Reason, ReqData, Context)
6564
end
6665
end)
6766
end.

deps/rabbitmq_management_agent/src/rabbit_mgmt_format.erl

Lines changed: 1 addition & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
-export([to_amqp_table/1, listener/1, web_context/1, properties/1, basic_properties/1]).
1616
-export([record/2, to_basic_properties/1]).
1717
-export([addr/1, port/1]).
18-
-export([format_nulls/1, escape_html_tags/1]).
18+
-export([format_nulls/1]).
1919
-export([print/2, print/1]).
2020

2121
-export([format_queue_stats/1, format_queue_basic_stats/1,
@@ -542,27 +542,6 @@ format_null_item([{_K, _V} | _T] = L) ->
542542
format_null_item(Value) ->
543543
Value.
544544

545-
546-
-spec escape_html_tags(string()) -> binary().
547-
548-
escape_html_tags(S) ->
549-
escape_html_tags(rabbit_data_coercion:to_list(S), []).
550-
551-
552-
-spec escape_html_tags(string(), string()) -> binary().
553-
554-
escape_html_tags([], Acc) ->
555-
rabbit_data_coercion:to_binary(lists:reverse(Acc));
556-
escape_html_tags("<" ++ Rest, Acc) ->
557-
escape_html_tags(Rest, lists:reverse("&lt;", Acc));
558-
escape_html_tags(">" ++ Rest, Acc) ->
559-
escape_html_tags(Rest, lists:reverse("&gt;", Acc));
560-
escape_html_tags("&" ++ Rest, Acc) ->
561-
escape_html_tags(Rest, lists:reverse("&amp;", Acc));
562-
escape_html_tags([C | Rest], Acc) ->
563-
escape_html_tags(Rest, [C | Acc]).
564-
565-
566545
-spec clean_consumer_details(proplists:proplist()) -> proplists:proplist().
567546
clean_consumer_details(Obj) ->
568547
case pget(consumer_details, Obj) of

deps/rabbitmq_web_dispatch/src/rabbit_web_dispatch_access_control.erl

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -274,8 +274,8 @@ not_authorised(Reason, ReqData, Context) ->
274274

275275
halt_response(Code, Type, Reason, ReqData, Context) ->
276276
ReasonFormatted = format_reason(Reason),
277-
Json = #{<<"error">> => Type,
278-
<<"reason">> => ReasonFormatted},
277+
Json = #{error => Type,
278+
reason => ReasonFormatted},
279279
ReqData1 = cowboy_req:reply(Code,
280280
#{<<"content-type">> => <<"application/json">>},
281281
rabbit_json:encode(Json), ReqData),
@@ -287,7 +287,7 @@ not_authenticated(Reason, ReqData, Context, _AuthConfig) ->
287287
format_reason(Tuple) when is_tuple(Tuple) ->
288288
tuple(Tuple);
289289
format_reason(Binary) when is_binary(Binary) ->
290-
Binary;
290+
unicode:characters_to_binary(Binary);
291291
format_reason(Other) ->
292292
case is_string(Other) of
293293
true -> print("~ts", [Other]);

0 commit comments

Comments
 (0)