Closed rickheil closed 3 years ago
Hey, this PR is not correct and it breaks things even more :(.
The with statement
will close the resource on its exit, so when we do:
with open(binary, 'rb') as f:
files['binary'] = f
return self._post_data(self.url, data, files)
The binary
file is opened, its opened file handle is assigned to files['binary']
and then when we go out of scope the file is closed. The closed file is then attempted to be read by the requests
library somewhere deeper in the self._post_data
function.
Here is an example that shows this issue, ran in an ipython interactive shell:
$ ipython
Python 3.8.2 (default, Apr 8 2021, 23:19:18)
Type 'copyright', 'credits' or 'license' for more information
IPython 7.20.0 -- An enhanced Interactive Python. Type '?' for help.
In [1]: import requests
In [2]: !echo example_content>data
In [3]: !cat data
example_content
In [4]: with open('data', 'rb') as f: files = {'upload_file': f}
In [5]: files
Out[5]: {'upload_file': <_io.BufferedReader name='data'>}
In [6]: r = requests.post('http://localhost/', files=files)
---------------------------------------------------------------------------
ValueError Traceback (most recent call last)
<ipython-input-6-a9d9f29c5118> in <module>
----> 1 r = requests.post('http://localhost/', files=files)
~/Library/Python/3.8/lib/python/site-packages/requests/api.py in post(url, data, json, **kwargs)
117 """
118
--> 119 return request('post', url, data=data, json=json, **kwargs)
120
121
~/Library/Python/3.8/lib/python/site-packages/requests/api.py in request(method, url, **kwargs)
59 # cases, and look like a memory leak in others.
60 with sessions.Session() as session:
---> 61 return session.request(method=method, url=url, **kwargs)
62
63
~/Library/Python/3.8/lib/python/site-packages/requests/sessions.py in request(self, method, url, params, data, headers, cookies, files, auth, timeout, allow_redirects, proxies, hooks, stream, verify, cert, json)
526 hooks=hooks,
527 )
--> 528 prep = self.prepare_request(req)
529
530 proxies = proxies or {}
~/Library/Python/3.8/lib/python/site-packages/requests/sessions.py in prepare_request(self, request)
454
455 p = PreparedRequest()
--> 456 p.prepare(
457 method=request.method.upper(),
458 url=request.url,
~/Library/Python/3.8/lib/python/site-packages/requests/models.py in prepare(self, method, url, headers, files, data, params, auth, cookies, hooks, json)
317 self.prepare_headers(headers)
318 self.prepare_cookies(cookies)
--> 319 self.prepare_body(data, files, json)
320 self.prepare_auth(auth, url)
321
~/Library/Python/3.8/lib/python/site-packages/requests/models.py in prepare_body(self, data, files, json)
505 # Multi-part file uploads.
506 if files:
--> 507 (body, content_type) = self._encode_files(files, data)
508 else:
509 if data:
~/Library/Python/3.8/lib/python/site-packages/requests/models.py in _encode_files(files, data)
157 fdata = fp
158 elif hasattr(fp, 'read'):
--> 159 fdata = fp.read()
160 elif fp is None:
161 continue
ValueError: read of closed file
So TL;DR: the post/get/etc call needs to be within the with statement
, which is not that trivial change when we open file only under a given condition. A simple solution could be to have two return self._post/get/etc
in both cases - when we conditionally open the file and when we don't.
A more robust solution could be to write our own context manager which would do the open/close of the file if its needed (conditionally included in the context manager?).
Fix #31 - close file handles when we are dealing with local files.