Code review comment for ~ilasc/launchpad:bug-1910403

Revision history for this message
Colin Watson (cjwatson) wrote :

You can reduce the original OOPS to whether an exception e from `parse_maintainer_bytes` can be rendered as text using `six.text_type(e)`; that's not literally what the email layer does, but it's close enough. This patch doesn't fix that. (In fact, while the cleanup in `ParseMaintError` looks reasonable, I don't think it fixes anything, as the new test you've added passes with or without that cleanup.)

>>> import six
>>> from lp.archiveuploader.utils import parse_maintainer_bytes
>>> try:
... parse_maintainer_bytes(b'No\xc3\xa8l K\xc3\xb6the', 'Maintainer')
... except Exception as e:
... pass
>>> six.text_type(e)
Traceback (most recent call last):
  File "<console>", line 1, in <module>
UnicodeDecodeError: 'ascii' codec can't decode byte 0xc3 in position 2: ordinal not in range(128)

Consider instead just constructing the `ParseMaintError` exceptions raised by `parse_maintainer` as Unicode rather than encoding text to UTF-8 there. I think that should work better.

You may also want to fold the test for this into a simple modification to `testParseMaintainerRaises`, rather than adding a new test case. It looks as though a change to that test will be needed for a correct fix anyway.

review: Needs Fixing

« Back to merge proposal