Supplying Unicode string for basic_publish raises an exception. #4

Open
majek opened this Issue Sep 13, 2011 · 4 comments

Comments

Projects
None yet
3 participants
@majek
Owner

majek commented Sep 13, 2011

It should just work.

@grncdr

This comment has been minimized.

Show comment Hide comment
@grncdr

grncdr May 19, 2012

I was bit by this today when my application code was generating unicode routing keys, even if calling str(routing_key) unconditionally is a bad idea (I admit I'm not very familiar with intricacies of handling string encodings in Python) throwing a more informative error earlier than connection._send_frames would be a big improvement.

grncdr commented May 19, 2012

I was bit by this today when my application code was generating unicode routing keys, even if calling str(routing_key) unconditionally is a bad idea (I admit I'm not very familiar with intricacies of handling string encodings in Python) throwing a more informative error earlier than connection._send_frames would be a big improvement.

@majek

This comment has been minimized.

Show comment Hide comment
@majek

majek Jun 6, 2012

Owner

In the commit fc6f87d I made an attempt to automatically convert all strings to and from utf-8. (except properties and tables, where unicode support is not updated here).

It's kind of okay, but:

  1. Puka will be slower.
  2. I'm not entirely sure it's correct. What happens if someone else actually defined a queue which is not a valid utf-8 string? With this patch puka will fail.

Actually, I'm less sure that Puka should deal with unicode at all.

Owner

majek commented Jun 6, 2012

In the commit fc6f87d I made an attempt to automatically convert all strings to and from utf-8. (except properties and tables, where unicode support is not updated here).

It's kind of okay, but:

  1. Puka will be slower.
  2. I'm not entirely sure it's correct. What happens if someone else actually defined a queue which is not a valid utf-8 string? With this patch puka will fail.

Actually, I'm less sure that Puka should deal with unicode at all.

@pr3d4t0r

This comment has been minimized.

Show comment Hide comment
@pr3d4t0r

pr3d4t0r Aug 11, 2014

@majek - we are bitten by those issues too. We're getting around it by normalizing all strings going into puka using str() and using an abstract wrapper around it:

    def send(self, message):
        self._client = puka.Client("amqp://"+str(self._messagingServer))
        promise      = self._client.connect()

        self._client.wait(promise)

        promise = self._client.queue_declare(queue = self._messageQueue)
        self._client.wait(promise)
        promise = self._client.basic_publish(exchange = "", routing_key = self._messageQueue, body = str(message), mandatory = True) 
        self._client.wait(promise)
        promise = self._client.close()
        self._client.wait(promise)

It would be nice to normalize everything to UTF-8/Unicode inside puka given the international nature of most applications nowadays. str()/ASCII should be the exception, not the rule, in our opinion (we've dealt with a lot of i18n implementations in the last 7 years).

Thoughts?

@majek - we are bitten by those issues too. We're getting around it by normalizing all strings going into puka using str() and using an abstract wrapper around it:

    def send(self, message):
        self._client = puka.Client("amqp://"+str(self._messagingServer))
        promise      = self._client.connect()

        self._client.wait(promise)

        promise = self._client.queue_declare(queue = self._messageQueue)
        self._client.wait(promise)
        promise = self._client.basic_publish(exchange = "", routing_key = self._messageQueue, body = str(message), mandatory = True) 
        self._client.wait(promise)
        promise = self._client.close()
        self._client.wait(promise)

It would be nice to normalize everything to UTF-8/Unicode inside puka given the international nature of most applications nowadays. str()/ASCII should be the exception, not the rule, in our opinion (we've dealt with a lot of i18n implementations in the last 7 years).

Thoughts?

@majek

This comment has been minimized.

Show comment Hide comment
@majek

majek Aug 11, 2014

Owner

Here's the problem: I could run str() within puka but this would slow it down significantly! Puka does need to work on raw strings (ie: not unicode), as it needs to calculate byte lengths etc.

So in balance, unfortunately, it's "cheaper" for me to do nothing and get users to supply strings, as opposed to normalizing all the time and slow down puka...

Owner

majek commented Aug 11, 2014

Here's the problem: I could run str() within puka but this would slow it down significantly! Puka does need to work on raw strings (ie: not unicode), as it needs to calculate byte lengths etc.

So in balance, unfortunately, it's "cheaper" for me to do nothing and get users to supply strings, as opposed to normalizing all the time and slow down puka...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment