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

Setup uploaded filename if field value is binary and transfer encoding is not specified #349

Merged
merged 1 commit into from
Apr 29, 2015

Conversation

asvetlov
Copy link
Member

I have stuck again with subtle bug in aiohttp file uploading.

On call:

with open(fname, 'rb') as f:
    body = f.read()
yield from request('POST', url, data={'name': body})

body is sent as textual url-encoded string, which is unexpected.

I guess to assume the binary field value which sent without Transfer-Encoding is file body, filename is the same as field name if not specified.

requests library does the same, BTW.

Patch is attached.

@kxepal
Copy link
Member

kxepal commented Apr 29, 2015

-0

I worry that data param turns into heavy overloaded by underlying logic so it's hard to say what kind of request payload would be sent exact for the some provided arguments. Imagine, that you'd read body somewhere far away from actual request call.

May be, instead, make things more explicit? For instance, by specifying Content-Type header and then dispatch data encoding according their value? At least, you'll be sure that data will be send exact in the format you want without giving you any surprises.

@asvetlov
Copy link
Member Author

We need either fix current behavior (for passing data={'name': b'content'}) or directly forbid it. I prefer the former.

+1 for adding more explicit API for data sending but we need fix the current behavior first anyway.

@kxepal
Copy link
Member

kxepal commented Apr 29, 2015

Ah, ok with the fix then.

asvetlov added a commit that referenced this pull request Apr 29, 2015
Setup uploaded filename if field value is binary and transfer encoding is not specified
@asvetlov asvetlov merged commit 475ee7a into master Apr 29, 2015
@asvetlov asvetlov deleted the file_upload branch April 29, 2015 14:19
@lock
Copy link

lock bot commented Oct 30, 2019

This thread has been automatically locked since there has not been
any recent activity after it was closed. Please open a new issue for
related bugs.

If you feel like there's important points made in this discussion,
please include those exceprts into that new issue.

@lock lock bot added the outdated label Oct 30, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Oct 30, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants