pallets / flask

The Python micro framework for building web applications.
https://flask.palletsprojects.com
BSD 3-Clause "New" or "Revised" License
67.5k stars 16.14k forks source link

send_from_directory unclosed file error #5483

Closed PaulWhitingUVC closed 3 months ago

PaulWhitingUVC commented 3 months ago

When warnings are set to error, pytest claims there is an unclosed file when send_from_directory is called. I assume this indicates there is a resource leak. Even if Python garbage collects this at some point, this causes issues with testing flask applications that use send_from_directory with warnings as errors enabled.

  1. Create app

app.py:

from flask import Flask, send_from_directory
app = Flask(__name__)

@app.route('/<path:name>')
def download_resources(name):
    return send_from_directory('resources', name)
  1. Add resources/file.txt
mkdir resources
echo file > resources/file.txt
  1. Add tests

tests/conftest.py:

import pytest

from app import app as flask_app

@pytest.fixture
def app():
    yield flask_app

@pytest.fixture
def client(app):
    return app.test_client()

tests/test_app.py:

def test_download(app, client):
    res = client.get('/file.txt')
    assert res.status_code == 200
  1. Run tests

Execute pytest -Werror.

The results (results.txt) include the following error: ResourceWarning: unclosed file <_io.BufferedReader name='/flask-test/resources/file.txt'>

Expected Results:

Execute pytest without warnings as errors. This is the expected result:

============================= test session starts ==============================
platform darwin -- Python 3.11.9, pytest-8.2.0, pluggy-1.5.0
rootdir: /flask-test
collected 1 item

tests/test_app.py .                                                      [100%]

============================== 1 passed in 0.02s ===============================

Environment:

davidism commented 3 months ago

Call response.close() at the end. See Flask itself for examples of this in tests.

PaulWhitingUVC commented 3 months ago

Call response.close() at the end. See Flask itself for examples of this in tests.

@davidism, I assume you are referring to the call to rv.close() in the test_send_from_directory code.

Unfortunately, the call to send_from_directory and the associated close both happen in the test code proper. When called by production code the call to response.close() cannot be done without error. When I modify download_resources to the following it causes additional errors:

@app.route('/<path:name>')
def download_resources(name):
    response = send_from_directory('resources', name)
    response.close()
    return response

Error:

self = <werkzeug.wsgi.FileWrapper object at 0x10c061150>

    def __next__(self) -> bytes:
>       data = self.file.read(self.buffer_size)
E       ValueError: read of closed file

venv/lib/python3.11/site-packages/werkzeug/wsgi.py:332: ValueError

The documentation does not mention a call to response.close().

Other documentation similarly does not call response.close().

It seems to me that the rv.close() in Flask's test case masks the unclosed file error, as you are not using send_from_directory via an external call like a Flask app would.

Thoughts?

davidism commented 3 months ago

You don't call it in your application, only in your test. WSGI servers close the response automatically, the test client cannot.