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

Some servers don't understand quoted boundaries in multipart documents #2544

Closed
FichteFoll opened this issue Nov 21, 2017 · 11 comments · Fixed by #2545
Closed

Some servers don't understand quoted boundaries in multipart documents #2544

FichteFoll opened this issue Nov 21, 2017 · 11 comments · Fixed by #2545

Comments

@FichteFoll
Copy link
Contributor

FichteFoll commented Nov 21, 2017

Long story short

After a prolonged debugging session I discovered that the usage of quotes in the boundary of a multipart document caused my request to be rejected by the server since it didn't support this.
Preventing the quotation marks from being added in the code makes this work.

The server in question is using nginx, but I don't know which version yet (asked their support).

Specification

https://tools.ietf.org/html/rfc7231#section-3.1.1.5
https://tools.ietf.org/html/rfc7231#appendix-D
https://tools.ietf.org/html/rfc7230#section-3.2.6

Content-Type = media-type

media-type = type "/" subtype *( OWS ";" OWS parameter )
parameter = token "=" ( token / quoted-string )

token          = 1*tchar
quoted-string  = DQUOTE *( qdtext / quoted-pair ) DQUOTE
qdtext         = HTAB / SP /%x21 / %x23-5B / %x5D-7E / obs-text
obs-text       = %x80-FF
quoted-pair    = "\" ( HTAB / SP / VCHAR / obs-text )
tchar          = "!" / "#" / "$" / "%" / "&" / "'" / "*"
                   / "+" / "-" / "." / "^" / "_" / "`" / "|" / "~"
                   / DIGIT / ALPHA
                   ; any VCHAR, except delimiters

Expected behaviour

Specify the multipart boundary without quotes, unless they are required.

Actual behaviour

Multipart boundary is always quoted.

Steps to reproduce

Since I don't know which nginx version is affected, the only reliable way to reproduce right now is by filing a request at share-online's upload API.

A sample script is provided below (fill in your data):

import asyncio
import pathlib

import aiohttp

SESSION_URL = "http://www.share-online.biz/upv3_session.php"
USER = ""
PASS = ""
path = pathlib.Path(".gitignore")


async def async_main():
    async with aiohttp.ClientSession() as session:
        data = {'username': USER, 'password': PASS}
        async with session.post(SESSION_URL, data=data) as resp:
            text = await resp.text()
        session_name, session_endpoint = text.split(";")
        session_endpoint = f"http://{session_endpoint}"

        file_size = path.stat().st_size
        with path.open('rb') as fp:
            data = aiohttp.FormData()
            data.add_field('username', USER)
            data.add_field('password', PASS)
            data.add_field('upload_session', session_name)
            data.add_field('chunk_no', "1")
            data.add_field('chunk_number', "1")
            data.add_field('filesize', str(file_size))
            data.add_field('fn', fp, filename=path.name, content_type="application/octet-steam")
            data.add_field('finalize', "1")

            print(f"Starting upload of {path.name}; size: {file_size}")
            async with session.post(session_endpoint, data=data) as resp:
                # print(f"response headers: {resp.headers}")
                resp.raise_for_status()
                text = await resp.text()
                print(f"reply: {text}")


def main():
    loop = asyncio.get_event_loop()
    task = asyncio.ensure_future(async_main())
    try:
        loop.run_until_complete(task)
    finally:
        loop.close()


if __name__ == '__main__':
    main()

With this script, the upload will fail with *** EXCEPTION session creation/reuse failed - 11-21-2017, 3:13 pm ***.

Removing the quotes at

ctype = 'multipart/{}; boundary="{}"'.format(subtype, boundary)
makes the request work.

Your environment

Arch Linux, Python 3.6, aiohttp 2.3.3 (client)

@asvetlov
Copy link
Member

Make sense. Pull Request is needed

@asvetlov
Copy link
Member

@FichteFoll would you propose requested change?

@FichteFoll
Copy link
Contributor Author

FichteFoll commented Nov 21, 2017

I was in fact already working on it, but had issues with the test environment and decided to hold off until a decision was made.

Is there some part in the code that handles parameter quotation as outlined in the above ABNF already? This seems to be used in a couple places.

Edit: And if no, what would be a good place to add it?

For the record, the parameter rule is used for Content-Type and Accept, though I suppose we can expect the user to provide a valid Accept header if they need one.

@asvetlov
Copy link
Member

Perhaps @kxepal is the best person for answering on the question.

@kxepal
Copy link
Member

kxepal commented Nov 22, 2017

@FichteFoll
It's strange that your server fails because it must handle both quoted and plain values. "Always quote" policy is good for clients since it removes a lot heuristics about "to quote or to not".

@asvetlov
Copy link
Member

Perhaps the server is not 100% compliant with HTTP spec.

@asvetlov
Copy link
Member

I recall similar problem with cookies.

@kxepal
Copy link
Member

kxepal commented Nov 22, 2017

IIRC for cookies we also do quote all the time and still happy.

@asvetlov
Copy link
Member

It does. More precisely cookies are started to be quoted in Python 3.4 or 3.5 -- I don't remember.
In aiohttp backlog we have a #802
Not sure how the problem is widespread

@FichteFoll
Copy link
Contributor Author

FichteFoll commented Nov 23, 2017

I agree that the server is behaving badly here, but it should be in the interest of the client to reach maximum compatibility with existing server implementations as long as it doesn't require too much work or is out-of-scope. I already reported this problem to their support and asked for the exact version of nginx they are using, but haven't received a reply since then.

@lock
Copy link

lock bot commented Oct 28, 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].
[new issue]: https://github.com/aio-libs/aiohttp/issues/new

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

Successfully merging a pull request may close this issue.

3 participants