aio-libs / aiohttp

Asynchronous HTTP client/server framework for asyncio and Python
https://docs.aiohttp.org
Other
15.15k stars 2.02k forks source link

Redirect on upload from file causes IO error from file being closed #5530

Open MatthewScholefield opened 3 years ago

MatthewScholefield commented 3 years ago

🐞 Describe the bug As seen from #1907, aiohttp takes ownership of passed files and closes them after writing. However, in the case of a 302 redirect, where I assume that aiohttp is re-performing the request, it attempts to re-read the data from the closed file.

πŸ’‘ To Reproduce

Client code:

from aiohttp import ClientSession
with open(abs_storage_path(rel_path), "rb") as f:
    async with ClientSession() as session:
        async with session.put("http://localhost:5000/foo", data=f) as r:
            print("Response:", r.status)

Server code:

from flask import Flask, request, redirect

app = Flask(__name__)

@app.route("/foo", methods=['PUT'])
def redirct_route():
    return redirect("http://localhost:5000/bar", code=302)

@app.route("/bar", methods=['PUT'])
def put_route():
    print('Uploading data of size:', len(request.data))
    return 'success', 200

app.run(debug=True)

πŸ’‘ Expected behavior Successful PUT upload (or to not follow redirects and return a 302 response).

πŸ“‹ Logs/tracebacks

    async with session.put("http://localhost:5000/foo", data=f) as r:
               β”‚       β”‚                                     β”” <_io.BufferedReader name='../data/foo.dat'>
               β”‚       β”” <function ClientSession.put at 0x7faa1294de50>
               β”” <aiohttp.client.ClientSession object at 0x7faa101d9a00>

  File ".../aiohttp/client.py", line 1117, in __aenter__
    self._resp = await self._coro
    β”‚    β”‚             β”‚    β”” <member '_coro' of '_BaseRequestContextManager' objects>
    β”‚    β”‚             β”” <aiohttp.client._RequestContextManager object at 0x7faa101e7bc0>
    β”‚    β”” <member '_resp' of '_BaseRequestContextManager' objects>
    β”” <aiohttp.client._RequestContextManager object at 0x7faa101e7bc0>
  File ".../aiohttp/client.py", line 492, in _request
    req = self._request_class(
          β”‚    β”” <class 'aiohttp.client_reqrep.ClientRequest'>
          β”” <aiohttp.client.ClientSession object at 0x7faa101d9a00>
  File ".../aiohttp/client_reqrep.py", line 313, in __init__
    self.update_body_from_data(data)
    β”‚    β”‚                     β”” <_io.BufferedReader name='../data/foo.dat'>
    β”‚    β”” <function ClientRequest.update_body_from_data at 0x7faa12c5e0d0>
    β”” <aiohttp.client_reqrep.ClientRequest object at 0x7faa1015e580>
  File ".../aiohttp/client_reqrep.py", line 519, in update_body_from_data
    size = body.size
           β”‚    β”” <property object at 0x7faa12c8ecc0>
           β”” <aiohttp.payload.BufferedReaderPayload object at 0x7faa1015e730>
  File ".../aiohttp/payload.py", line 362, in size
    return os.fstat(self._value.fileno()).st_size - self._value.tell()
           β”‚  β”‚     β”‚    β”‚      β”‚                   β”‚    β”‚      β”” <method 'tell' of '_io.BufferedReader' objects>
           β”‚  β”‚     β”‚    β”‚      β”‚                   β”‚    β”” <_io.BufferedReader name='../data/foo.dat'>
           β”‚  β”‚     β”‚    β”‚      β”‚                   β”” <aiohttp.payload.BufferedReaderPayload object at 0x7faa1015e730>
           β”‚  β”‚     β”‚    β”‚      β”” <method 'fileno' of '_io.BufferedReader' objects>
           β”‚  β”‚     β”‚    β”” <_io.BufferedReader name='../data/foo.dat'>
           β”‚  β”‚     β”” <aiohttp.payload.BufferedReaderPayload object at 0x7faa1015e730>
           β”‚  β”” <built-in function fstat>
           β”” <module 'os' from '/usr/lib/python3.9/os.py'>

ValueError: I/O operation on closed file

πŸ“‹ Your version of the Python

$ python --version
Python 3.9.1

πŸ“‹ Your version of the aiohttp/yarl/multidict distributions

$ python -m pip show aiohttp
Name: aiohttp
Version: 3.7.4
Summary: Async http client/server framework (asyncio)
Home-page: https://github.com/aio-libs/aiohttp
Author: Nikolay Kim
Author-email: fafhrd91@gmail.com
License: Apache 2
Location: .../python3.9/site-packages
Requires: yarl, attrs, typing-extensions, chardet, async-timeout, multidict
$ python -m pip show multidict
Name: multidict
Version: 5.1.0
Summary: multidict implementation
Home-page: https://github.com/aio-libs/multidict
Author: Andrew Svetlov
Author-email: andrew.svetlov@gmail.com
License: Apache 2
Location: .../python3.9/site-packages
Requires: 
Required-by: yarl, async-asgi-testclient, aiohttp
$ python -m pip show yarl
Name: yarl
Version: 1.6.3
Summary: Yet another URL library
Home-page: https://github.com/aio-libs/yarl/
Author: Andrew Svetlov
Author-email: andrew.svetlov@gmail.com
License: Apache 2
Location: .../python3.9/site-packages
Requires: idna, multidict
Required-by: aiohttp

πŸ“‹ Additional context Relates to the client aspect of aiohttp.

ghost commented 3 years ago

I'm facing the same error, though my case is a 307 instead of 302. The workaround I'm using for now is https://github.com/aio-libs/aiohttp/issues/3654#issuecomment-473873122

Dreamsorcerer commented 2 months ago

This appears to be pretty difficult to do correctly. Currently the file gets closed when the connection is closed. I was hoping we could use expect100 as some kind of solution, but this doesn't resolve the problem and reveals a limitation on the server-side:

The server-side code doesn't have an easy way to send the 302 without first sending a 100-Continue (maybe you can use a custom expect handler, but it would then need to know about all your endpoints, in order to know whether they can be resolved without further data. Maybe we could improve this somehow to only send 100-Continue when a handler tries to read the body?).