From e8c77f5912a703e5991d176d59359481b16cff02 Mon Sep 17 00:00:00 2001
From: Michal Kuratczyk <mkuratczyk@vmware.com>
Date: Tue, 15 Oct 2024 13:12:36 +0200
Subject: [PATCH] AMQP filter expressions: handle invalid filters
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

An invalid application property filter could cause
a function_clause that was returned to the client, eg `omq`
output looked like this:
```
2024/10/15 13:10:55 ERRO consumer failed to create a receiver id=1
  error=
  │ *Error{Condition: amqp:internal-error, Description: Session error: function_clause
  │ [{rabbit_amqp_filtex,'-validate0/2-fun-0-',
  │                      [{{symbol,<<"subject">>},{utf8,<<"var">>}}],
  │                      [{file,"rabbit_amqp_filtex.erl"},{line,119}]},
  │  {lists,map,2,[{file,"lists.erl"},{line,2077}]},
  │  {rabbit_amqp_filtex,validate0,2,[{file,"rabbit_amqp_filtex.erl"},{line,119}]},
  │  {rabbit_amqp_filtex,validate,1,[{file,"rabbit_amqp_filtex.erl"},{line,28}]},
  │  {rabbit_amqp_session,parse_filters,2,
  │                       [{file,"rabbit_amqp_session.erl"},{line,3068}]},
  │  {rabbit_amqp_session,parse_filter,1,
  │                       [{file,"rabbit_amqp_session.erl"},{line,3014}]},
  │  {rabbit_amqp_session,'-handle_attach/2-fun-0-',21,
  │                       [{file,"rabbit_amqp_session.erl"},{line,1371}]},
  │
  {rabbit_misc,with_exit_handler,2,[{file,"rabbit_misc.erl"},{line,465}]}],
  Info: map[]}
```

Now such filters are handled better
---
 deps/rabbit/src/rabbit_amqp_filtex.erl | 11 +++++++++--
 deps/rabbit/test/amqp_filtex_SUITE.erl | 21 ++++++++++++++++++++-
 2 files changed, 29 insertions(+), 3 deletions(-)

diff --git a/deps/rabbit/src/rabbit_amqp_filtex.erl b/deps/rabbit/src/rabbit_amqp_filtex.erl
index bcdd289e4723..7ea57b2c386f 100644
--- a/deps/rabbit/src/rabbit_amqp_filtex.erl
+++ b/deps/rabbit/src/rabbit_amqp_filtex.erl
@@ -119,9 +119,16 @@ validate0(Descriptor, KVList0) when
     KVList = lists:map(fun({{utf8, Key}, {utf8, String}}) ->
                                {Key, parse_string_modifier_prefix(String)};
                           ({{utf8, Key}, TaggedVal}) ->
-                               {Key, unwrap(TaggedVal)}
+                               {Key, unwrap(TaggedVal)};
+                          (_) ->
+                               invalid_filter
                        end, KVList0),
-    {ok, {application_properties, KVList}};
+    case lists:member(invalid_filter, KVList) of
+        false ->
+            {ok, {application_properties, KVList}};
+        true ->
+            error
+    end;
 validate0(_, _) ->
     error.
 
diff --git a/deps/rabbit/test/amqp_filtex_SUITE.erl b/deps/rabbit/test/amqp_filtex_SUITE.erl
index 51469821a83b..88e6c5ef5cca 100644
--- a/deps/rabbit/test/amqp_filtex_SUITE.erl
+++ b/deps/rabbit/test/amqp_filtex_SUITE.erl
@@ -32,7 +32,8 @@
 
 all() ->
     [
-     {group, cluster_size_1}
+     {group, cluster_size_1},
+     {group, filter_validation}
     ].
 
 groups() ->
@@ -44,6 +45,10 @@ groups() ->
        multiple_sections,
        filter_few_messages_from_many,
        string_modifier
+      ]},
+     {filter_validation, [shuffle],
+      [
+       invalid_application_property_filter
       ]}
     ].
 
@@ -579,6 +584,20 @@ string_modifier(Config) ->
     ok = end_session_sync(Session),
     ok = close_connection_sync(Connection).
 
+invalid_application_property_filter(_Config) ->
+            %% the only expression is invalid
+            ?assertEqual(error,
+                         rabbit_amqp_filtex:validate(
+                           {described,{symbol,?DESCRIPTOR_NAME_APPLICATION_PROPERTIES_FILTER},
+                                      {map,[{{symbol,<<"subject">>},{utf8,<<"var">>}}]}})),
+
+            %% one expressions is valid but another one is not
+            ?assertEqual(error,
+                         rabbit_amqp_filtex:validate(
+                           {described,{symbol,?DESCRIPTOR_NAME_APPLICATION_PROPERTIES_FILTER},
+                                      {map,[{{utf8, <<"valid">>}, {symbol, <<"no match">>}},
+                                            {{symbol,<<"subject">>}, {utf8,<<"var">>}}]}})).
+
 %% -------------------------------------------------------------------
 %% Helpers
 %% -------------------------------------------------------------------