Skip to content

Conversation

@mkuratczyk
Copy link
Contributor

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 ignored.

@mkuratczyk mkuratczyk requested a review from ansd October 16, 2024 09:08
@mkuratczyk mkuratczyk force-pushed the filtex-handle-invalid-filters branch 2 times, most recently from b78beae to ffa4cc4 Compare October 16, 2024 21:37
{Key, unwrap(TaggedVal)}
{Key, unwrap(TaggedVal)};
(_) ->
invalid_filter
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your modification violates the spec when sending the filter field from RabbitMQ to the consumer:

the sending endpoint sets the filter actually in place

In your change, the desired filter as sent from the client to RabbitMQ is echoed back from RabbitMQ to the client while RabbitMQ silently modifies the filter.
This function should return error if the filter is invalid such that the entire filter doesn't take effect.
For example, the Java client compares only the filter names that were desired (i.e. sent from consumer to RabbitMQ) with the filter names actually in place (i.e. sent from RabbitMQ to the consumer).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. I've changed it so that the function will return an error

@mkuratczyk mkuratczyk force-pushed the filtex-handle-invalid-filters branch from ffa4cc4 to 2616e01 Compare October 17, 2024 07:44
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
@mkuratczyk mkuratczyk force-pushed the filtex-handle-invalid-filters branch from 724e7fa to e8c77f5 Compare October 17, 2024 10:54
@ansd
Copy link
Member

ansd commented Oct 17, 2024

Once #12541 is merged, we can easily add an integration test since it will allow the Erlang client to be configured to notify about the attach reply from RabbitMQ such that we can check the list of filters returned from RabbitMQ within the test case.

ansd added a commit that referenced this pull request Oct 18, 2024
application-properties keys are restricted to be strings.

Prior to this commit, a function_clause error occurred if the client
requested an invalid filter:
```
  │ *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[]}
```

After this commit, the filter won't actually take effect without a crash occurring.

Supersedes #12520
@ansd
Copy link
Member

ansd commented Oct 18, 2024

Closed in favour of #12549

@ansd ansd closed this Oct 18, 2024
@mkuratczyk mkuratczyk deleted the filtex-handle-invalid-filters branch January 17, 2025 15:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants