Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prevent a UTF-8 decoder error from breaking Celery #78

Closed
wants to merge 203 commits into from

Conversation

djsmith42
Copy link

Prior to this commit, a non-UTF8 encoded header string would cause an uncaught
UnicodeDecodeError, which would cause an unrecoverable Celery
error. Now we will log non-UTF8 header strings (names and values), ignore them, and
continue processing.

Closes https://github.com/celery/celery/issues/2873

ask and others added 30 commits February 20, 2014 18:11
SSL socket read timeouts are not raised as socket.timeout exceptions.
So, to have SSL socket read timeouts behave the same way as TCP socket
read timeouts, we need to find them based off of their text and reraise
as socket.timeout exceptions.

This is similar to the fix in read_timeout in connection.py at:
https://github.com/celery/py-amqp/blob/1.4/amqp/connection.py#L336-L337
and in fact the exception raised here is eventually caught by
read_timeout and replaced with a socket.timeout exception. The problem
is that the transport is already marked as disconnected at that point.
So, we need to catch the timeout here in transport.py
Don't disconnect transport on SSL read timeout
There is a bug in the python socket library wherein attempting
ssl.read/write on a closed socket raises an AttributeError instead of
IOError. We need to catch that case and raise an IOError.
@ask
Copy link
Contributor

ask commented May 27, 2016

I think this is caused by running an older Python 2.7 version, and it was also fixed in master just now!

@djsmith42
Copy link
Author

I rebased the latest master (oops, should have merged), but I'm still getting test failures with a similar error. However, now at least half of the tests are passing. The other half'ish are failing like this:

======================================================================
ERROR: test_roundtrip (amqp.tests.test_serialization.test_serialization)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/dsmith/Dev/sandbox/git/py-amqp/amqp/tests/test_serialization.py", line 61, in test_roundtrip
    datetime(2015, 3, 13, 10, 23),
  File "/home/dsmith/Dev/sandbox/git/py-amqp/amqp/serialization.py", line 294, in dumps
    write(pack('>H', int(val)))
TypeError: Struct() argument 1 must be string, not unicode

======================================================================
ERROR: test_table (amqp.tests.test_serialization.test_serialization)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/dsmith/Dev/sandbox/git/py-amqp/amqp/tests/test_serialization.py", line 83, in test_table
    loads(b'F', dumps(b'F', [table]))[0][0],
  File "/home/dsmith/Dev/sandbox/git/py-amqp/amqp/serialization.py", line 320, in dumps
    _write_table(val or {}, write, bits)
  File "/home/dsmith/Dev/sandbox/git/py-amqp/amqp/serialization.py", line 340, in _write_table
    _write_item(v, twrite, bits)
  File "/home/dsmith/Dev/sandbox/git/py-amqp/amqp/serialization.py", line 378, in _write_item
    write(pack('>ci', b'I', v))
TypeError: Struct() argument 1 must be string, not unicode

@djsmith42
Copy link
Author

I am running Python 2.7.3 on Ubuntu 12.04

Adam Chainz and others added 2 commits May 29, 2016 18:48
…rojects

As per [their blog post of the 27th April](https://blog.readthedocs.com/securing-subdomains/) ‘Securing subdomains’:

> Starting today, Read the Docs will start hosting projects from subdomains on the domain readthedocs.io, instead of on readthedocs.org. This change addresses some security concerns around site cookies while hosting user generated data on the same domain as our dashboard.

Test Plan: Manually visited all the links I’ve modified.
Convert readthedocs link for their .org -> .io migration for hosted projects
@ranman
Copy link

ranman commented May 31, 2016

I think this was fixed in #88

@air-upc
Copy link

air-upc commented Jun 6, 2016

Not fixed all in #88, as too many pack/unpack in the source code...

@auvipy
Copy link
Member

auvipy commented Apr 13, 2017

plz send a new clean pr.

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.

None yet