Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unhandled function clause exception when retrieving queue for nonexistent virtual host #10901

Closed
LoisSotoLopez opened this issue Apr 2, 2024 · 2 comments
Labels

Comments

@LoisSotoLopez
Copy link
Contributor

Describe the bug

Retrieving messages on a queue through rabbit_mgmt_wm_queue_get.erl for an invalid virtual host results on an unhandled function_clause exception.

This issue seems to have been introduced by commit 439c2b4 , more precisely by this change .

-    mnesia:async_dirty(
-      fun () ->
-              case VHost of
-                  '_' -> ok;
-                  _   -> rabbit_vhost:assert(VHost)
-              end,
-              Match = #runtime_parameters{key = {VHost, Component, '_'},
-                                          _   = '_'},
-              [p(P) || #runtime_parameters{key = {_VHost, Comp, _Name}} = P <-
-                           mnesia:match_object(?TABLE, Match, read),
-                       Comp =/= <<"policy">> orelse Component =:= <<"policy">>]
-      end).
+    [p(P) || #runtime_parameters{key = {_VHost, Comp, _Name}} = P <-
+             rabbit_db_rtparams:get_all(VHost, Component),
+             Comp =/= <<"policy">> orelse Component =:= <<"policy">>].

While on the previous version of the file the rabbit_vhost:assert/1 was called before searching for the vhost data on the Mnesia table, the current version calls rabbit_db_rtparams:get_all/2. As seen below, that function ends up calling to rabbit_vhost:assert/1 on the vhost too (at get_all_in_mnesia/2) but only if the VHostName parameter passed to get_all/2 is either a binary() or the '_' atom.

+get_all(VHostName, Comp)
+  when (is_binary(VHostName) orelse VHostName =:= '_') andalso
+       (is_binary(Comp) orelse Comp =:= '_') ->
+    rabbit_db:run(
+      #{mnesia => fun() -> get_all_in_mnesia(VHostName, Comp) end}).
+
+get_all_in_mnesia(VHostName, Comp) ->
+    mnesia:async_dirty(
+      fun () ->
+              case VHostName of
+                  '_' -> ok;
+                  _   -> rabbit_vhost:assert(VHostName)
+              end,
+              Match = #runtime_parameters{key = {VHostName, Comp, '_'},
+                                          _   = '_'},
+              mnesia:match_object(?MNESIA_TABLE, Match, read)
+      end).

That new type check is incompatible with a previous step on the data flow, on rabbit_mgmt_wm_queue_get where the parsing of the request leads to the variable for the virtual host having the not_found atom as value if the virtual host does not exist.

The parsing is done by

VHost = rabbit_mgmt_util:vhost(ReqData0),
which calls to
rabbit_web_dispatch_access_control:vhost(ReqData).
which calls to

vhost(ReqData) ->
    Value = case id(vhost, ReqData) of
      none  -> vhost_from_headers(ReqData);
      VHost -> VHost
    end,
    case Value of
      none -> none;
      Name ->
        case rabbit_vhost:exists(Name) of
          true  -> Name;
          false -> not_found % <---------- This propagates and leads to `function_clause`
        end
    end.
Expand this to see the stacktrace associated to the **Reproduction steps** below.

2024-04-02 06:59:28.296087+00:00 [error] <0.1965.0>   crasher:
2024-04-02 06:59:28.296087+00:00 [error] <0.1965.0>     initial call: cowboy_stream_h:request_process/3
2024-04-02 06:59:28.296087+00:00 [error] <0.1965.0>     pid: <0.1965.0>
2024-04-02 06:59:28.296087+00:00 [error] <0.1965.0>     registered_name: []
2024-04-02 06:59:28.296087+00:00 [error] <0.1965.0>     exception error: no case clause matching
2024-04-02 06:59:28.296087+00:00 [error] <0.1965.0>                      {error,
2024-04-02 06:59:28.296087+00:00 [error] <0.1965.0>                          {{'EXIT',
2024-04-02 06:59:28.296087+00:00 [error] <0.1965.0>                               {function_clause,
2024-04-02 06:59:28.296087+00:00 [error] <0.1965.0>                                   [{rabbit_db_rtparams,get_all,
2024-04-02 06:59:28.296087+00:00 [error] <0.1965.0>                                        [not_found,<<"vhost-limits">>],
2024-04-02 06:59:28.296087+00:00 [error] <0.1965.0>                                        [{file,"rabbit_db_rtparams.erl"},
2024-04-02 06:59:28.296087+00:00 [error] <0.1965.0>                                         {line,178}]},
2024-04-02 06:59:28.296087+00:00 [error] <0.1965.0>                                    {rabbit_runtime_parameters,list,2,
2024-04-02 06:59:28.296087+00:00 [error] <0.1965.0>                                        [{file,"rabbit_runtime_parameters.erl"},
2024-04-02 06:59:28.296087+00:00 [error] <0.1965.0>                                         {line,261}]},
2024-04-02 06:59:28.296087+00:00 [error] <0.1965.0>                                    {rabbit_vhost_limit,get_limit,2,
2024-04-02 06:59:28.296087+00:00 [error] <0.1965.0>                                        [{file,"rabbit_vhost_limit.erl"},
2024-04-02 06:59:28.296087+00:00 [error] <0.1965.0>                                         {line,192}]},
2024-04-02 06:59:28.296087+00:00 [error] <0.1965.0>                                    {rabbit_vhost_limit,
2024-04-02 06:59:28.296087+00:00 [error] <0.1965.0>                                        is_over_connection_limit,1,
2024-04-02 06:59:28.296087+00:00 [error] <0.1965.0>                                        [{file,"rabbit_vhost_limit.erl"},
2024-04-02 06:59:28.296087+00:00 [error] <0.1965.0>                                         {line,87}]},
2024-04-02 06:59:28.296087+00:00 [error] <0.1965.0>                                    {rabbit_direct,
2024-04-02 06:59:28.296087+00:00 [error] <0.1965.0>                                        is_over_vhost_connection_limit,3,
2024-04-02 06:59:28.296087+00:00 [error] <0.1965.0>                                        [{file,"rabbit_direct.erl"},
2024-04-02 06:59:28.296087+00:00 [error] <0.1965.0>                                         {line,173}]},
2024-04-02 06:59:28.296087+00:00 [error] <0.1965.0>                                    {rabbit_direct,connect,5,
2024-04-02 06:59:28.296087+00:00 [error] <0.1965.0>                                        [{file,"rabbit_direct.erl"},
2024-04-02 06:59:28.296087+00:00 [error] <0.1965.0>                                         {line,95}]}]}},
2024-04-02 06:59:28.296087+00:00 [error] <0.1965.0>                           rabbit@34b640576ca8}}
2024-04-02 06:59:28.296087+00:00 [error] <0.1965.0>       in function  rabbit_mgmt_util:with_channel/5 (rabbit_mgmt_util.erl, line 868)
2024-04-02 06:59:28.296087+00:00 [error] <0.1965.0>       in call from rabbit_mgmt_util:with_decode/5 (rabbit_mgmt_util.erl, line 714)
2024-04-02 06:59:28.296087+00:00 [error] <0.1965.0>       in call from rabbit_mgmt_wm_queue_get:accept_content/2 (rabbit_mgmt_wm_queue_get.erl, line 42)
2024-04-02 06:59:28.296087+00:00 [error] <0.1965.0>       in call from cowboy_rest:call/3 (src/cowboy_rest.erl, line 1590)
2024-04-02 06:59:28.296087+00:00 [error] <0.1965.0>       in call from cowboy_rest:process_content_type/3 (src/cowboy_rest.erl, line 1096)
2024-04-02 06:59:28.296087+00:00 [error] <0.1965.0>       in call from cowboy_rest:upgrade/4 (src/cowboy_rest.erl, line 284)
2024-04-02 06:59:28.296087+00:00 [error] <0.1965.0>       in call from cowboy_stream_h:execute/3 (src/cowboy_stream_h.erl, line 306)
2024-04-02 06:59:28.296087+00:00 [error] <0.1965.0>       in call from cowboy_stream_h:request_process/3 (src/cowboy_stream_h.erl, line 295)

Reproduction steps

  1. docker run --rm -it -p 15673:15672 rabbitmq:3.13.0-management
  2. curl -sv -u guest:guest -X POST -H 'Content-Type: application/json' --data '{ "ackmode": "reject_requeue_true", "encoding": "auto", "count": 1 }' http://localhost:15673/api/queues/does-not-exist/foo/get
  3. A 500 error returns.

Expected behavior

A 404 HTTP error code specifying the provided virtual host does not exist.

Additional context

We are already working on this.

LoisSotoLopez added a commit to cloudamqp/rabbitmq-server that referenced this issue Apr 2, 2024
@michaelklishin
Copy link
Member

michaelklishin commented Apr 2, 2024

@LoisSotoLopez I mean, good to see someone working on this but honestly, polling over HTTP API when a virtual host does not exist? This is as low a priority as it gets.

Perhaps CloudAMQP, a company that directly makes money off of RabbitMQ and has been doing that since 2012, should consider more substantial contributions.

@gomoripeti
Copy link
Contributor

For the record Lois just started working with RabbitMQ a few weeks ago. A low priority task is the perfect first step to get familiar with the concepts and notions of RabbitMQ, the code base which is admittedly complex, custom solutions and peculiarities. This is the slow way forward for more substantial contributions. Please expect some more low priority contributions. Thanks for understanding.

LoisSotoLopez added a commit to cloudamqp/rabbitmq-server that referenced this issue Apr 4, 2024
LoisSotoLopez added a commit to cloudamqp/rabbitmq-server that referenced this issue Apr 4, 2024
LoisSotoLopez added a commit to cloudamqp/rabbitmq-server that referenced this issue Apr 5, 2024
LoisSotoLopez added a commit to cloudamqp/rabbitmq-server that referenced this issue Apr 5, 2024
LoisSotoLopez added a commit to cloudamqp/rabbitmq-server that referenced this issue Apr 9, 2024
LoisSotoLopez added a commit to cloudamqp/rabbitmq-server that referenced this issue Apr 10, 2024
LoisSotoLopez added a commit to cloudamqp/rabbitmq-server that referenced this issue Apr 10, 2024
mergify bot pushed a commit that referenced this issue Apr 12, 2024
Before this change some Management API endpoints handling POST requests crashed and returned HTTP 500 error code when called for a non-existing vhost. The reason was that parsing of the virtual host name could return a `not_found` atom which could potentially reach later steps of the data flow, which expect a vhost name binary only. Instead of returning `not_found`, now the code fails early with HTTP 400 error code and a descriptive error reason.

See more details in the github issue
Fixes #10901

(cherry picked from commit dfddfe3)
mergify bot pushed a commit that referenced this issue Apr 12, 2024
Issue #10901

(cherry picked from commit 0b4fff9)
mergify bot pushed a commit that referenced this issue Apr 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants