php-amqplib / php-amqplib

The most widely used PHP client for RabbitMQ
http://www.rabbitmq.com/getstarted.html
GNU Lesser General Public License v2.1
4.46k stars 1.03k forks source link

AMQPWriter::write_array() doesn't support table array-members #216

Closed kernins closed 9 years ago

kernins commented 9 years ago

Subj method threats both associative and indexed arrays as indexed, and so will lose array keys in former case. To be clear, I'm talking about nested arrays and is_array() check, not original argument itself.

Whether it is unlikely or not to have such complex structs in msg headers, it is theoretically possible, so should be properly handled or at least exception should be thrown.

michaelklishin commented 9 years ago

Other clients support such nesting just fine, so I consider this a legitimate bug.

kernins commented 9 years ago

There is also some sort of inconsistency here. write_table() requires types to specified explicitly, and write_array() instead performs some heuristics to determine types. But there is no fundamential difference between A and F, they're both collections, no reason to treat'em differently.

It would be much better to have such heuristics in a separate helper class instead. It would be usable for both methods and allow users to extend it if needed.

kernins commented 9 years ago

Currently both write_array() and read_array() omits type specifiers. This becomes a real hell on complex nested structs, especially read_table() result which is essentially a crazy mix of two formats in such cases.

So I have 2 questions:

videlalvaro commented 9 years ago

@kernins I'm not sure who might be using these functions at user land. So if they don't break users apps, then I'm OK for improving these methods