Use with statements for handling files

[rickheil]

Fix #31 - close file handles when we are dealing with local files.

[bryanheinz]

Looks good to me!

[disconnect3d]

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?).