Support Content-Transfer-Encoding binary #1169
Conversation
This fixes #1168
Current coverage is 98.40% (diff: 100%)@@ master #1169 diff @@
==========================================
Files 29 29
Lines 6535 6519 -16
Methods 0 0
Messages 0 0
Branches 1096 1094 -2
==========================================
- Hits 6431 6415 -16
Misses 54 54
Partials 50 50
|
|
LGTM |
| @@ -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 | |||
There was a problem hiding this comment.
Does not work for me (ClientRequestError: Can not write request body'), we need:
for chunk in stream:
yield chunk
There was a problem hiding this comment.
Hm...ok, but why? This should be roughly the same.
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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!
There was a problem hiding this comment.
Thank you for quick response!
There was a problem hiding this comment.
There was a problem hiding this comment.
@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?
There was a problem hiding this comment.
Sorry, seems that I've tested not on the latest version of aiohttp, after updating it returns 405 response.
|
@asvetlov Yes, let's do that. I'll try to get to the code soon to close this one. |
|
Now the PR is ready, isn't it? |
|
@asvetlov Indeed it is. |
|
Awesome! |
|
@asvetlov Great! Do you plan to cut the new release today? |
|
Yes. We was waiting it for very long, at least couple weeks longer that I expected. |
|
@kxepal hey, i am playing with rust async stuff. would you be interested in help me with mime type parsing? |
|
@fafhrd91 I'm not very good with rust, but sure, ping me if you need a help - will try to make it. |
|
@kxepal i am not good either, i just started week ago :) |
|
@fafhrd91 Ok, will check, thanks! |
This fixes #1168