Merge pull request #1883 from pallets/send_file-pathlib

use pathlib internally in send_file

3 files changed, 48 insertions, 72 deletions

diff --git a/CHANGES.rst b/CHANGES.rst
index 947a7f85..82590509 100644
--- a/CHANGES.rst
+++ b/CHANGES.rst
@@ -41,8 +41,8 @@ Unreleased
     ``RAW_URI``. :issue:`1781`
 -   Switch the parameter order of ``default_stream_factory`` to match
     the order used when calling it. :pr:`1085`
--   Add ``send_file`` to generate a response that serves a file, based
-    on Flask's implementation. :issue:`265`, :pr:`1850`
+-   Add ``send_file()`` function to generate a response that serves a
+    file, adapted from Flask's implementation. :issue:`265`, :pr:`1850`
 
 
 Version 1.0.2
diff --git a/src/werkzeug/utils.py b/src/werkzeug/utils.py
index 6ffdac37..1cacdc89 100644
--- a/src/werkzeug/utils.py
+++ b/src/werkzeug/utils.py
@@ -2,6 +2,7 @@ import codecs
 import io
 import mimetypes
 import os
+import pathlib
 import pkgutil
 import re
 import sys
@@ -552,7 +553,7 @@ def append_slash_redirect(environ, code=301):
 
 
 def send_file(
-    filename_or_fp,
+    path_or_file,
     environ=None,
     mimetype=None,
     as_attachment=False,
@@ -582,7 +583,7 @@ def send_file(
     will tell the server to send the given path, which is much more
     efficient than reading it in Python.
 
-    :param filename_or_fp: The path to the file to send, relative to the
+    :param path_or_file: The path to the file to send, relative to the
         current working directory if a relative path is given.
         Alternatively, a file-like object opened in binary mode. Make
         sure the file pointer is seeked to the start of the data.
@@ -614,18 +615,20 @@ def send_file(
     if response_class is None:
         from .wrappers import Response as response_class
 
-    if hasattr(filename_or_fp, "__fspath__"):
-        filename_or_fp = os.fspath(filename_or_fp)
-
-    if isinstance(filename_or_fp, str):
-        filename = filename_or_fp
+    if isinstance(path_or_file, (str, os.PathLike)) or hasattr(
+        path_or_file, "__fspath__"
+    ):
+        path = pathlib.Path(path_or_file).absolute()
+        stat = path.stat()
+        size = stat.st_size
+        mtime = stat.st_mtime
         file = None
     else:
-        file = filename_or_fp
-        filename = None
+        path = size = mtime = None
+        file = path_or_file
 
-    if attachment_filename is None and filename is not None:
-        attachment_filename = os.path.basename(filename)
+    if attachment_filename is None and path is not None:
+        attachment_filename = path.name
 
     if mimetype is None:
         if attachment_filename is not None:
@@ -638,18 +641,18 @@ def send_file(
             raise ValueError(
                 "Unable to detect the MIME type because a file name is"
                 " not available. Either set 'attachment_filename', pass"
-                " a file path instead of a file-like object, or pass"
-                " 'mimetype' directly."
+                " a path instead of a file, or set 'mimetype'."
             )
 
     headers = Headers()
 
     if as_attachment:
         if attachment_filename is None:
-            raise TypeError("filename unavailable, required for sending as attachment")
-
-        if not isinstance(attachment_filename, str):
-            attachment_filename = attachment_filename.decode("utf-8")
+            raise TypeError(
+                "No name provided for attachment. Either set"
+                " 'attachment_filename' or pass a path instead of a"
+                " file."
+            )
 
         try:
             attachment_filename = attachment_filename.encode("ascii")
@@ -663,32 +666,26 @@ def send_file(
 
         headers.add("Content-Disposition", "attachment", **filenames)
 
-    mtime = None
-    fsize = None
-
-    if use_x_sendfile and filename:
-        headers["X-Sendfile"] = filename
-        fsize = os.path.getsize(filename)
+    if use_x_sendfile and path:
+        headers["X-Sendfile"] = str(path)
         data = None
     else:
         if file is None:
-            file = open(filename, "rb")
-            mtime = os.path.getmtime(filename)
-            fsize = os.path.getsize(filename)
+            file = path.open("rb")
         elif isinstance(file, io.BytesIO):
-            fsize = file.getbuffer().nbytes
+            size = file.getbuffer().nbytes
         elif isinstance(file, io.TextIOBase):
             raise ValueError("Files must be opened in binary mode or use BytesIO.")
 
         data = wrap_file(environ or {}, file)
 
-    if fsize is not None:
-        headers["Content-Length"] = fsize
-
     rv = response_class(
         data, mimetype=mimetype, headers=headers, direct_passthrough=True
     )
 
+    if size is not None:
+        rv.content_length = size
+
     if last_modified is not None:
         rv.last_modified = last_modified
     elif mtime is not None:
@@ -700,24 +697,16 @@ def send_file(
         rv.cache_control.max_age = cache_timeout
         rv.expires = int(time() + cache_timeout)
 
-    if add_etags and filename is not None:
-        try:
-            f_enc = filename.encode("utf-8") if isinstance(filename, str) else filename
-            check = adler32(f_enc) & 0xFFFFFFFF
-            rv.set_etag(
-                f"{os.path.getmtime(filename)}-{os.path.getsize(filename)}-{check}"
-            )
-        except OSError:
-            warnings.warn(
-                f"Accessing {filename!r} failed, can't generate etag.", stacklevel=2,
-            )
+    if add_etags and path is not None:
+        check = adler32(str(path).encode("utf-8")) & 0xFFFFFFFF
+        rv.set_etag(f"{mtime}-{size}-{check}")
 
     if conditional:
         if environ is None:
             raise TypeError("'environ' is required with 'conditional=True'.")
 
         try:
-            rv = rv.make_conditional(environ, accept_ranges=True, complete_length=fsize)
+            rv = rv.make_conditional(environ, accept_ranges=True, complete_length=size)
         except RequestedRangeNotSatisfiable:
             if file is not None:
                 file.close()
diff --git a/tests/test_send_file.py b/tests/test_send_file.py
index dd72e697..8a44f739 100644
--- a/tests/test_send_file.py
+++ b/tests/test_send_file.py
@@ -5,54 +5,41 @@ import pathlib
 
 import pytest
 
-from werkzeug.test import Client
+from werkzeug.test import EnvironBuilder
 from werkzeug.utils import send_file
-from werkzeug.wrappers.response import Response
-from werkzeug.wsgi import responder
 
-res_path = os.path.join(os.path.dirname(__file__), "res")
-html_path = os.path.join(res_path, "index.html")
-txt_path = os.path.join(res_path, "test.txt")
+res_path = pathlib.Path(__file__).parent / "res"
+html_path = res_path / "index.html"
+txt_path = res_path / "test.txt"
 
 
-@pytest.mark.parametrize("path", [html_path, pathlib.Path(html_path)])
+@pytest.mark.parametrize("path", [html_path, str(html_path)])
 def test_path(path):
     rv = send_file(path)
     assert rv.mimetype == "text/html"
     assert rv.direct_passthrough
     rv.direct_passthrough = False
-
-    with open(html_path, "rb") as f:
-        assert rv.data == f.read()
-
+    assert rv.data == html_path.read_bytes()
     rv.close()
 
 
 def test_x_sendfile():
     rv = send_file(html_path, use_x_sendfile=True)
-    assert rv.headers["x-sendfile"] == html_path
+    assert rv.headers["x-sendfile"] == str(html_path)
     assert rv.data == b""
     rv.close()
 
 
 def test_last_modified():
+    environ = EnvironBuilder().get_environ()
     last_modified = datetime.datetime(1999, 1, 1)
-
-    def foo(environ, start_response):
-        return send_file(
-            io.BytesIO(b"party like it's"),
-            environ=environ,
-            last_modified=last_modified,
-            mimetype="text/plain",
-        )
-
-    client = Client(responder(foo), Response)
-    rv = client.get("/")
+    rv = send_file(txt_path, environ=environ, last_modified=last_modified)
     assert rv.last_modified == last_modified
+    rv.close()
 
 
 @pytest.mark.parametrize(
-    "file_factory", [lambda: open(txt_path, "rb"), lambda: io.BytesIO(b"test")],
+    "file_factory", [lambda: txt_path.open("rb"), lambda: io.BytesIO(b"test")],
 )
 def test_object(file_factory):
     rv = send_file(file_factory(), mimetype="text/plain", use_x_sendfile=True)
@@ -75,7 +62,7 @@ def test_object_mimetype_from_attachment():
 
 
 @pytest.mark.parametrize(
-    "file_factory", [lambda: open(txt_path), lambda: io.StringIO("test")],
+    "file_factory", [lambda: txt_path.open(), lambda: io.StringIO("test")],
 )
 def test_text_mode_fails(file_factory):
     with file_factory() as f, pytest.raises(ValueError, match="binary mode"):
@@ -83,7 +70,7 @@ def test_text_mode_fails(file_factory):
 
 
 @pytest.mark.parametrize(
-    ("filename", "ascii", "utf8"),
+    ("name", "ascii", "utf8"),
     (
         ("index.html", "index.html", None),
         (
@@ -97,8 +84,8 @@ def test_text_mode_fails(file_factory):
         ("те:/ст", '":/"', "%D1%82%D0%B5%3A%2F%D1%81%D1%82"),
     ),
 )
-def test_non_ascii_filename(filename, ascii, utf8):
-    rv = send_file(html_path, as_attachment=True, attachment_filename=filename)
+def test_non_ascii_filename(name, ascii, utf8):
+    rv = send_file(html_path, as_attachment=True, attachment_filename=name)
     rv.close()
     content_disposition = rv.headers["Content-Disposition"]
     assert f"filename={ascii}" in content_disposition