rabbitmq / rabbitmq-amqp1.0

AMQP 1.0 support for RabbitMQ
https://www.rabbitmq.com/
Other
93 stars 20 forks source link

Missing pattern for to_expiration #13

Closed flaper87 closed 7 years ago

flaper87 commented 9 years ago

Apparently, based on my very bad erlang knowledge (and some Erlangers help), there's a missing pattern in the to_expiration function.

** Reason for termination == 
** {function_clause,
       [{rabbit_amqp1_0_message,to_expiration,[{uint,5000000}],[]},
        {rabbit_amqp1_0_message,translate_header,2,[]},
        {rabbit_amqp1_0_message,assemble,4,[]},
        {rabbit_amqp1_0_message,assemble,1,[]},
        {rabbit_amqp1_0_incoming_link,transfer,4,[]},
        {rabbit_amqp1_0_session_process,handle_control,2,[]},
        {rabbit_amqp1_0_session_process,handle_cast,2,[]},
        {gen_server2,handle_msg,2,[]}]}
michaelklishin commented 9 years ago

How did you trigger this?

flaper87 commented 9 years ago

Here's a snippet to reproduce this, sorry for not adding it to the report:

from proton import Message
from proton.handlers import MessagingHandler
from proton.reactor import Container

class HelloWorld(MessagingHandler):
    def __init__(self, server, address):
        super(HelloWorld, self).__init__()
        self.server = server
        self.address = address

    def on_start(self, event):
        conn = event.container.connect(self.server, reconnect=False)
        event.container.create_receiver(conn, self.address)
        event.container.create_sender(conn, self.address)

    def on_sendable(self, event):
        event.sender.send(Message(body=u"Hello World", ttl=500))
        event.sender.close()

    def on_message(self, event):
        print event.message.body
        event.connection.close()

Container(HelloWorld("127.0.0.1:5672", "examples")).run()
sstruchtrup commented 8 years ago

I am seeing the same error when sending a message with the ttl set via qpip-proton (C++ binding).

According to the AMQP 1.0 spec, section 3.2.1, ttl should be encoded as milliseconds (which itself is an uint, 2.8.6).

Proton does indeed send milliseconds as uint, but the AMQP 1.0 plugin seems to expect a timestamp (which would be a 64-bit unsigned int).

The relevant code in rabbit_amqp1_0_message.erl:

to_expiration(undefined) ->
    undefined;
to_expiration({timestamp, Num}) ->
    list_to_binary(integer_to_list(Num)).

If I change timestamp to uint, everything works as expected for me.

This looks correct to me (although I am neither an erlang nor AMQP expert), but I didn't test other clients though.

sstruchtrup commented 8 years ago

RabbitMQ sends the following raw header for an AMQP 1.0 message with a ttl of 3599 seconds:

c0 0f 05 41 50 04 83 00 00 00 00 00 36 ea 98 41 40

Which decodes as:

c0 0f 05                     constructor  
41                           durable: bool / true
50 04                        priority: ubyte/ 04
83 00 00 00 00 00 36 ea 98   ttl: ms64 / 3599000 ms
41                           first-acquirer: bool / true
40                           delivery-count: null

0x83 is a timestamp encoded as ms64, fixed with of 8 bytes and representing milliseconds since the unix epoch - which doesn't match the encoded number of 3599000/0x36ea98 (3599s)

As far as what I could see in the code, qpid-proton seems to be lenient in parsing and simply skip the invalid ttl and interpret it as "no ttl"

sstruchtrup commented 8 years ago

The following is what proton sends (raw AMQP 1.0 Header):

d0 00 00 00 0f 00 00 00 05 41 50 04 70 00 36 ee 80 42 52 00 00 53 73

Which decodes as:

d0 00 00 00 0f 00 00 00 05   constructor: list32: 15 bytes/5 entries
41                           durable: bool/true
50 04                        priority: ubyte/4
70 00 36 ee 80               ttl: uint / 3600000
42                           first-acquirer: bool / false
52 00                        delivery-count:  small-uint / 0
00 53 73                     padding (???)
michaelklishin commented 7 years ago

@kjnilsson perhaps this can be considered for backporting?

kjnilsson commented 7 years ago

@michaelklishin I think I already have

dpocock commented 7 years ago

I'm using qpid-proton and I also observed this problem.

Are there any client side workarounds when creating/sending a message to RabbitMQ?

kjnilsson commented 7 years ago

@dpocock until the next release you'd have to avoid using ttls.