Repository: zopefoundation/Zope

Issue:
`ResourceWarning` after uploading large files

### What I did:

Upload a file with an html like this that one:

```html
<html>
    <form action="/" method="post" enctype="multipart/form-data">
        <input type="file" id="f" name="f">
        <button type="submit">Upload</button>
    </form>
</html>
```


### What I expect to happen:

no warning

### What actually happened:

warning on the console:

```
./Zope/src/ZPublisher/HTTPRequest.py:195: ResourceWarning: unclosed file <_io.BufferedRandom name=18>
  self.form.clear()
ResourceWarning: Enable tracemalloc to get the object allocation traceback
```

Very small files do not cause this warning, because a `BytesIO` is used


### What version of Python and Zope/Addons I am using:

This happens on current Zope master branch.


### Suggested fix


Multipart documentation recommends closing `MultipartPart` at the end of request, quoting https://multipart.readthedocs.io/en/latest/usage.html#streaming-parser 

> Do not forget to `close` all parts after use to free up resources and avoid `ResourceWarnings`. Framework developers may want to add logic that automatically frees up resources after the request ended.


Zope request does not keep a reference to the `MultipartPart`, but creates a `FormField`

https://github.com/zopefoundation/Zope/blob/78025426ecb021e6296881f20afbf7e1db32a809/src/ZPublisher/HTTPRequest.py#L1497-L1501

which later becomes a `FileUpload`

https://github.com/zopefoundation/Zope/blob/78025426ecb021e6296881f20afbf7e1db32a809/src/ZPublisher/HTTPRequest.py#L547-L550

so calling `MultipartPart.close` is not directly applicable, but since what this method does ( [code for reference](https://github.com/defnull/multipart/blob/73f9a15dfb5d784c351811be44a5ff611b315272/multipart.py#L907-L911) ) is just calling `close` on the underlying file, calling `close` on the `FileUpload` is equivalent.

The fix I'm suggesting is to call `close` on all `FileUpload`s during `HTTPRequest.clear` ( I'm sending a pull request doing this ).


Discussion:

Fix (patch):
diff --git a/CHANGES.rst b/CHANGES.rst
index 388724654..a57b862b5 100644
--- a/CHANGES.rst
+++ b/CHANGES.rst
@@ -10,6 +10,9 @@ https://github.com/zopefoundation/Zope/blob/4.x/CHANGES.rst
 5.11.2 (unreleased)
 -------------------
 
+- Fix a ``ResourceWarning`` emitted when uploading large files.
+  (`#1242 <https://github.com/zopefoundation/Zope/issues/1242>`_)
+
 - OFS/cachable: fix *Cache this object using* label in ZMI.
 
 - Include versions constraints for production and non-production dependencies
diff --git a/src/ZPublisher/HTTPRequest.py b/src/ZPublisher/HTTPRequest.py
index 56f445d58..789fd621a 100644
--- a/src/ZPublisher/HTTPRequest.py
+++ b/src/ZPublisher/HTTPRequest.py
@@ -192,6 +192,9 @@ class HTTPRequest(BaseRequest):
         self.stdin = None
         self._file = None
         self._fs = None
+        for f in self.form.values():
+            if isinstance(f, FileUpload):
+                f.close()
         self.form.clear()
         # we want to clear the lazy dict here because BaseRequests don't have
         # one.  Without this, there's the possibility of memory leaking


Test changes:
diff --git a/src/ZPublisher/tests/testHTTPRequest.py b/src/ZPublisher/tests/testHTTPRequest.py
index 2bc1b7dad..2581a4575 100644
--- a/src/ZPublisher/tests/testHTTPRequest.py
+++ b/src/ZPublisher/tests/testHTTPRequest.py
@@ -901,8 +901,10 @@ class HTTPRequestTests(unittest.TestCase, HTTPRequestFactoryMixin):
         req.processInputs()
         f = req.form.get('largefile')
         self.assertTrue(f.name)
-        self.assertEqual(4006, len(f.file.read()))
-        f.file.close()
+        self.assertEqual(40006, len(f.file.read()))
+        self.assertTrue(f.file.fileno())
+        req.clear()
+        self.assertTrue(f.file.closed)
 
     def test_processInputs_with_file_upload_gets_iterator(self):
         # checks fileupload object supports the iterator protocol
@@ -1640,7 +1642,7 @@ Content-Type: application/octet-stream
 test %s
 
 --12345--
-''' % (b'test' * 1000)
+''' % (b'test' * 10000)
 
 TEST_ISSUE_1095_DATA = b'''
 --12345