From 4896ad8236d8f49cbb6ef58f2647810e6851047b Mon Sep 17 00:00:00 2001 From: Ace Breakpoint Date: Wed, 4 Dec 2024 10:39:16 +0800 Subject: [PATCH 1/2] Fix crash caused by mishandling of non-ascii amqp_error explaination MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `rabbit_binary_generator:map_exception/3` will crash when there are unicode characters in the `explaination` field of `Reason#amqp_error` parameter. The explaination string (list) is assumed to be ascii, with each character/member in the range of a byte. Any unicode characters in the string will trigger `badarg` crash of `list_to_binary/1` in `rabbit_binary_generator:amqp_exception_explanation/2`. Amqp091 shovel crash due to this is reported, https://github.com/rabbitmq/rabbitmq-server/discussions/12874 When a queue as shovel source/destination does not exist, and its name contains non-ascii characters, the explaination of amqp_error will be like `no queue non_ascii_name_😍 in vhost /`. It will subsequently crash and even affect management console. To fix this, `unicode:characters_to_binary/1` is used instead of `list_to_binary/1`, and unicode-safe truncation of long explaination with `io_lib:format/3` chars_limit replaces direct bytes truncation. (cherry picked from commit ee41983e8481c1f27027ff8ef876f86a9ff01950) --- deps/rabbit_common/src/rabbit_binary_generator.erl | 7 ++----- deps/rabbit_common/test/unit_SUITE.erl | 8 ++++++++ 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/deps/rabbit_common/src/rabbit_binary_generator.erl b/deps/rabbit_common/src/rabbit_binary_generator.erl index eb256052c0b6..fa480394b4d9 100644 --- a/deps/rabbit_common/src/rabbit_binary_generator.erl +++ b/deps/rabbit_common/src/rabbit_binary_generator.erl @@ -228,8 +228,5 @@ lookup_amqp_exception(Other, Protocol) -> {ShouldClose, Code, Text, none}. amqp_exception_explanation(Text, Expl) -> - ExplBin = list_to_binary(Expl), - CompleteTextBin = <>, - if size(CompleteTextBin) > 255 -> <>; - true -> CompleteTextBin - end. + LimitedText = io_lib:format("~ts - ~ts", [Text, Expl], [{chars_limit, 255}]), + unicode:characters_to_binary(LimitedText). diff --git a/deps/rabbit_common/test/unit_SUITE.erl b/deps/rabbit_common/test/unit_SUITE.erl index 0336bdb7fb57..f0bc9de2d109 100644 --- a/deps/rabbit_common/test/unit_SUITE.erl +++ b/deps/rabbit_common/test/unit_SUITE.erl @@ -46,6 +46,7 @@ groups() -> pid_decompose_compose, platform_and_version, frame_encoding_does_not_fail_with_empty_binary_payload, + map_exception_does_not_fail_with_unicode_explaination, amqp_table_conversion, name_type, get_erl_path, @@ -415,6 +416,13 @@ frame_encoding_does_not_fail_with_empty_binary_payload(_Config) -> ]], ok. +map_exception_does_not_fail_with_unicode_explaination(_Config) -> + NonAsciiExplaination = "no queue 'non_ascii_name_😍_你好' in vhost '/'", + rabbit_binary_generator:map_exception(0, + #amqp_error{name = not_found, explanation = NonAsciiExplaination, method = 'queue.declare'}, + rabbit_framing_amqp_0_9_1), + ok. + amqp_table_conversion(_Config) -> assert_table(#{}, []), assert_table(#{<<"x-expires">> => 1000}, From b791974f90ad516b9d022700e586e1293cf4566d Mon Sep 17 00:00:00 2001 From: Michael Klishin Date: Tue, 3 Dec 2024 23:07:58 -0500 Subject: [PATCH 2/2] One more test case for #12888 (cherry picked from commit 9b1953994ee8b0aa958f17389364d8fc06bcf622) --- deps/rabbit_common/test/unit_SUITE.erl | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/deps/rabbit_common/test/unit_SUITE.erl b/deps/rabbit_common/test/unit_SUITE.erl index f0bc9de2d109..deed8051303c 100644 --- a/deps/rabbit_common/test/unit_SUITE.erl +++ b/deps/rabbit_common/test/unit_SUITE.erl @@ -46,7 +46,8 @@ groups() -> pid_decompose_compose, platform_and_version, frame_encoding_does_not_fail_with_empty_binary_payload, - map_exception_does_not_fail_with_unicode_explaination, + map_exception_does_not_fail_with_unicode_explaination_case1, + map_exception_does_not_fail_with_unicode_explaination_case2, amqp_table_conversion, name_type, get_erl_path, @@ -416,13 +417,20 @@ frame_encoding_does_not_fail_with_empty_binary_payload(_Config) -> ]], ok. -map_exception_does_not_fail_with_unicode_explaination(_Config) -> +map_exception_does_not_fail_with_unicode_explaination_case1(_Config) -> NonAsciiExplaination = "no queue 'non_ascii_name_😍_你好' in vhost '/'", rabbit_binary_generator:map_exception(0, #amqp_error{name = not_found, explanation = NonAsciiExplaination, method = 'queue.declare'}, rabbit_framing_amqp_0_9_1), ok. +map_exception_does_not_fail_with_unicode_explaination_case2(_Config) -> + NonAsciiExplaination = "no queue 'кролик 🐰' in vhost '/'", + rabbit_binary_generator:map_exception(0, + #amqp_error{name = not_found, explanation = NonAsciiExplaination, method = 'queue.declare'}, + rabbit_framing_amqp_0_9_1), + ok. + amqp_table_conversion(_Config) -> assert_table(#{}, []), assert_table(#{<<"x-expires">> => 1000},