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

Support Content-Transfer-Encoding binary #1169

Merged
merged 2 commits into from
Sep 16, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion aiohttp/multipart.py
Original file line number Diff line number Diff line change
Expand Up @@ -443,7 +443,7 @@ def decode(self, data):
Supports ``gzip``, ``deflate`` and ``identity`` encodings for
`Content-Encoding` header.

Supports ``base64``, ``quoted-printable`` encodings for
Supports ``base64``, ``quoted-printable``, ``binary`` encodings for
`Content-Transfer-Encoding` header.

:param bytearray data: Data to decode.
Expand Down Expand Up @@ -477,6 +477,8 @@ def _decode_content_transfer(self, data):
return base64.b64decode(data)
elif encoding == 'quoted-printable':
return binascii.a2b_qp(data)
elif encoding == 'binary':
return data
else:
raise RuntimeError('unknown content transfer encoding: {}'
''.format(encoding))
Expand Down Expand Up @@ -854,6 +856,8 @@ def _apply_content_transfer_encoding(self, stream):
elif encoding == 'quoted-printable':
for chunk in stream:
yield binascii.b2a_qp(chunk)
elif encoding == 'binary':
yield from stream

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does not work for me (ClientRequestError: Can not write request body'), we need:

 for chunk in stream:
      yield chunk

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm...ok, but why? This should be roughly the same.

Copy link

@AleksandrIakhnev AleksandrIakhnev Sep 14, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have you tried to make actual POST request? It is exception here https://github.com/KeepSafe/aiohttp/blob/master/aiohttp/client_reqrep.py#L427 (got: class 'generator' - seems pretty clear that is not the same return

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, basically whole logic here is based around generators. Ok, will try to make a better test then to catch that problem. Thanks for report!

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for quick response!

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great, thanks a lot.

Copy link
Member Author

@kxepal kxepal Sep 16, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AleksandrIakhnev Sorry, but I failed to reproduce your issue: script you provided works for me returning 405 response from Google. I also add test about to make sure that it works right and expected.

Are you sure that nothing is missed there in your example?

Copy link

@AleksandrIakhnev AleksandrIakhnev Sep 16, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, seems that I've tested not on the latest version of aiohttp, after updating it returns 405 response.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great! (:

else:
raise RuntimeError('unknown content transfer encoding: {}'
''.format(encoding))
Expand Down
24 changes: 24 additions & 0 deletions tests/test_multipart.py
Original file line number Diff line number Diff line change
Expand Up @@ -339,6 +339,15 @@ def test_read_with_content_transfer_encoding_quoted_printable(self):
self.assertEqual(b'\xd0\x9f\xd1\x80\xd0\xb8\xd0\xb2\xd0\xb5\xd1\x82,'
b' \xd0\xbc\xd0\xb8\xd1\x80!', result)

def test_read_with_content_transfer_encoding_binary(self):
obj = aiohttp.multipart.BodyPartReader(
self.boundary, {CONTENT_TRANSFER_ENCODING: 'binary'},
Stream(b'\xd0\x9f\xd1\x80\xd0\xb8\xd0\xb2\xd0\xb5\xd1\x82,'
b' \xd0\xbc\xd0\xb8\xd1\x80!\r\n--:--'))
result = yield from obj.read(decode=True)
self.assertEqual(b'\xd0\x9f\xd1\x80\xd0\xb8\xd0\xb2\xd0\xb5\xd1\x82,'
b' \xd0\xbc\xd0\xb8\xd1\x80!', result)

def test_read_with_content_transfer_encoding_unknown(self):
obj = aiohttp.multipart.BodyPartReader(
self.boundary, {CONTENT_TRANSFER_ENCODING: 'unknown'},
Expand Down Expand Up @@ -941,6 +950,21 @@ def test_serialize_with_content_transfer_encoding_quote_printable(self):
self.assertEqual(b'\r\n', next(stream))
self.assertIsNone(next(stream, None))

def test_serialize_with_content_transfer_encoding_binary(self):
part = aiohttp.multipart.BodyPartWriter(
'Привет, мир!'.encode('utf-8'),
{CONTENT_TRANSFER_ENCODING: 'binary'})
stream = part.serialize()
self.assertEqual(b'Content-Transfer-Encoding: binary\r\n'
b'Content-Type: application/octet-stream',
next(stream))
self.assertEqual(b'\r\n\r\n', next(stream))

self.assertEqual(b'\xd0\x9f\xd1\x80\xd0\xb8\xd0\xb2\xd0\xb5\xd1\x82,'
b' \xd0\xbc\xd0\xb8\xd1\x80!', next(stream))
self.assertEqual(b'\r\n', next(stream))
self.assertIsNone(next(stream, None))

def test_serialize_with_content_transfer_encoding_unknown(self):
part = aiohttp.multipart.BodyPartWriter(
'Time to Relax!', {CONTENT_TRANSFER_ENCODING: 'unknown'})
Expand Down
32 changes: 32 additions & 0 deletions tests/test_web_functional.py
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,38 @@ def handler(request):
yield from resp.release()


@asyncio.coroutine
def test_multipart_content_transfer_encoding(loop, test_client):
"""For issue #1168"""
with multipart.MultipartWriter() as writer:
writer.append(b'\x00' * 10,
headers={'Content-Transfer-Encoding': 'binary'})

@asyncio.coroutine
def handler(request):
reader = yield from request.multipart()
assert isinstance(reader, multipart.MultipartReader)

part = yield from reader.next()
assert isinstance(part, multipart.BodyPartReader)
assert part.headers['Content-Transfer-Encoding'] == 'binary'
thing = yield from part.read()
assert thing == b'\x00' * 10

resp = web.Response()
resp.content_type = 'application/json'
resp.body = b''
return resp

app = web.Application(loop=loop)
app.router.add_post('/', handler)
client = yield from test_client(app)

resp = yield from client.post('/', data=writer, headers=writer.headers)
assert 200 == resp.status
yield from resp.release()


@asyncio.coroutine
def test_render_redirect(loop, test_client):

Expand Down