AMQP 1.0 parser: revisit how we treat exotic collection combinations

[michaelklishin]

Sequences of zero-length arrays need a fixed length budgets.

This implementation may seem a bit naive, but consider this:

1. Due to its use of the Erlang process dictionary, the cost is fixed regardless of message size, and stays within 10-18 ns (depending on the host), well below
2. Memory footprint stays flat even in intentionally abusive scenarios with multiple connections

The hot path footprint is under 1% on PerfTest and 3-6% on parser microbenchmarks, depending on the number of metadata sections the message has (not its length).

[mergify]

Tick the box to add this pull request to the merge queue (same as @mergifyio queue).

• Queue this pull request

[lhoguin]

Reviewed the parser changes. They are probably the simplest fix. I don't know whether the max budget constant is appropriate so I'll refrain from approving as David has more context.

Alternatively to avoid the process dictionary a counter (counters module) would work here. It would need to be passed around to parse functions, but it doesn't need to be returned as it is simply a ref, so while the diff would be somewhat larger (new argument) there would be no need to reset anything before parsing, and no orphaned value in the process dictionary after parsing. The B argument may also use the same counters ref as they can hold multiple counters.

Also reviewer the max_heap_size change, if we want to keep it it should probably be applied to other protocols. Even if they don't have this particular parser issue, they might have other issues we don't know about. I don't know if it helps against attackers though, they could just open more connections I think. But it helps against user error.

[ansd]

@michaelklishin the solution in this PR is exactly the same Claude came up with when I asked Claude yesterday night. I think Claude's solution is poor for several reasons:

• It only treats the symptoms. It doesn't eliminate the root cause: RabbitMQ allocating too much memory for a constant size AMQP frame
• It doesn't protect against using many connections in parallel
• Using the process dictionary is an anti pattern.

@michaelklishin @lhoguin I suggest we go with #16994 :

• This fixes the root cause: no unbounded memory allocation anymore
• removes arbitrary limits and budgets on the max length of such arrays
• high performance

[ansd]

They are probably the simplest fix.

@lhoguin #16994 is simpler

Also reviewer the max_heap_size change, if we want to keep it it should probably be applied to other protocols.

I agree.

I don't know if it helps against attackers though, they could just open more connections I think.

@lhoguin #16994 helps against many connections.