diff --git a/CHANGES/5149.bugfix b/CHANGES/5149.bugfix new file mode 100644 index 00000000000..a30bf39da1f --- /dev/null +++ b/CHANGES/5149.bugfix @@ -0,0 +1 @@ +Fixed static files handling for loops without .sendfile() diff --git a/CONTRIBUTORS.txt b/CONTRIBUTORS.txt index 500223149c6..7f52ae14d56 100644 --- a/CONTRIBUTORS.txt +++ b/CONTRIBUTORS.txt @@ -269,6 +269,7 @@ Vaibhav Sagar Vamsi Krishna Avula Vasiliy Faronov Vasyl Baran +Viacheslav Greshilov Victor Collod Victor Kovtun Vikas Kawadia diff --git a/aiohttp/web_fileresponse.py b/aiohttp/web_fileresponse.py index 2b497085a2e..320aba15e51 100644 --- a/aiohttp/web_fileresponse.py +++ b/aiohttp/web_fileresponse.py @@ -2,7 +2,7 @@ import mimetypes import os import pathlib -from functools import partial +import sys from typing import ( # noqa IO, TYPE_CHECKING, @@ -17,10 +17,6 @@ from . import hdrs from .abc import AbstractStreamWriter -from .base_protocol import BaseProtocol -from .helpers import set_exception, set_result -from .http_writer import StreamWriter -from .log import server_logger from .typedefs import LooseHeaders from .web_exceptions import ( HTTPNotModified, @@ -42,95 +38,6 @@ NOSENDFILE = bool(os.environ.get("AIOHTTP_NOSENDFILE")) -class SendfileStreamWriter(StreamWriter): - def __init__( - self, - protocol: BaseProtocol, - loop: asyncio.AbstractEventLoop, - fobj: IO[Any], - offset: int, - count: int, - on_chunk_sent: _T_OnChunkSent = None, - ) -> None: - super().__init__(protocol, loop, on_chunk_sent) - self._sendfile_buffer = [] # type: List[bytes] - self._fobj = fobj - self._count = count - self._offset = offset - self._in_fd = fobj.fileno() - - def _write(self, chunk: bytes) -> None: - # we overwrite StreamWriter._write, so nothing can be appended to - # _buffer, and nothing is written to the transport directly by the - # parent class - self.output_size += len(chunk) - self._sendfile_buffer.append(chunk) - - def _sendfile_cb(self, fut: "asyncio.Future[None]", out_fd: int) -> None: - if fut.cancelled(): - return - try: - if self._do_sendfile(out_fd): - set_result(fut, None) - except Exception as exc: - set_exception(fut, exc) - - def _do_sendfile(self, out_fd: int) -> bool: - try: - n = os.sendfile(out_fd, self._in_fd, self._offset, self._count) - if n == 0: # in_fd EOF reached - n = self._count - except (BlockingIOError, InterruptedError): - n = 0 - self.output_size += n - self._offset += n - self._count -= n - assert self._count >= 0 - return self._count == 0 - - def _done_fut(self, out_fd: int, fut: "asyncio.Future[None]") -> None: - self.loop.remove_writer(out_fd) - - async def sendfile(self) -> None: - assert self.transport is not None - loop = self.loop - data = b"".join(self._sendfile_buffer) - if hasattr(loop, "sendfile"): - # Python 3.7+ - self.transport.write(data) - if self._count != 0: - await loop.sendfile( - self.transport, self._fobj, self._offset, self._count - ) - await super().write_eof() - return - - self._fobj.seek(self._offset) - out_socket = self.transport.get_extra_info("socket").dup() - out_socket.setblocking(False) - out_fd = out_socket.fileno() - - try: - await loop.sock_sendall(out_socket, data) - if not self._do_sendfile(out_fd): - fut = loop.create_future() - fut.add_done_callback(partial(self._done_fut, out_fd)) - loop.add_writer(out_fd, self._sendfile_cb, fut, out_fd) - await fut - except asyncio.CancelledError: - raise - except Exception: - server_logger.debug("Socket error") - self.transport.close() - finally: - out_socket.close() - - await super().write_eof() - - async def write_eof(self, chunk: bytes = b"") -> None: - pass - - class FileResponse(StreamResponse): """A response object can be used to send files.""" @@ -150,52 +57,12 @@ def __init__( self._path = path self._chunk_size = chunk_size - async def _sendfile_system( - self, request: "BaseRequest", fobj: IO[Any], offset: int, count: int - ) -> AbstractStreamWriter: - # Write count bytes of fobj to resp using - # the os.sendfile system call. - # - # For details check - # https://github.com/KeepSafe/aiohttp/issues/1177 - # See https://github.com/KeepSafe/aiohttp/issues/958 for details - # - # request should be an aiohttp.web.Request instance. - # fobj should be an open file object. - # count should be an integer > 0. - - transport = request.transport - assert transport is not None - if ( - transport.get_extra_info("sslcontext") - or transport.get_extra_info("socket") is None - or self.compression - ): - writer = await self._sendfile_fallback(request, fobj, offset, count) - else: - writer = SendfileStreamWriter( - request.protocol, request._loop, fobj, offset, count - ) - request._payload_writer = writer - - await super().prepare(request) - await writer.sendfile() - - return writer - async def _sendfile_fallback( - self, request: "BaseRequest", fobj: IO[Any], offset: int, count: int + self, writer: AbstractStreamWriter, fobj: IO[Any], offset: int, count: int ) -> AbstractStreamWriter: - # Mimic the _sendfile_system() method, but without using the - # os.sendfile() system call. This should be used on systems - # that don't support the os.sendfile(). - # To keep memory usage low,fobj is transferred in chunks # controlled by the constructor's chunk_size argument. - writer = await super().prepare(request) - assert writer is not None - chunk_size = self._chunk_size loop = asyncio.get_event_loop() @@ -212,10 +79,26 @@ async def _sendfile_fallback( await writer.drain() return writer - if hasattr(os, "sendfile") and not NOSENDFILE: # pragma: no cover - _sendfile = _sendfile_system - else: # pragma: no cover - _sendfile = _sendfile_fallback + async def _sendfile( + self, request: "BaseRequest", fobj: IO[Any], offset: int, count: int + ) -> AbstractStreamWriter: + writer = await super().prepare(request) + assert writer is not None + + if NOSENDFILE or sys.version_info < (3, 7) or self.compression: + return await self._sendfile_fallback(writer, fobj, offset, count) + + loop = request._loop + transport = request.transport + assert transport is not None + + try: + await loop.sendfile(transport, fobj, offset, count) + except NotImplementedError: + return await self._sendfile_fallback(writer, fobj, offset, count) + + await super().write_eof() + return writer async def prepare(self, request: "BaseRequest") -> Optional[AbstractStreamWriter]: filepath = self._path diff --git a/tests/test_web_sendfile_functional.py b/tests/test_web_sendfile_functional.py index fdae26838a2..60a542b83cb 100644 --- a/tests/test_web_sendfile_functional.py +++ b/tests/test_web_sendfile_functional.py @@ -15,12 +15,21 @@ ssl = None # type: ignore -@pytest.fixture(params=["sendfile", "fallback"], ids=["sendfile", "fallback"]) -def sender(request): +@pytest.fixture +def loop_without_sendfile(loop): + def sendfile(*args, **kwargs): + raise NotImplementedError + + loop.sendfile = sendfile + return loop + + +@pytest.fixture(params=["sendfile", "no_sendfile"], ids=["sendfile", "no_sendfile"]) +def sender(request, loop_without_sendfile): def maker(*args, **kwargs): ret = web.FileResponse(*args, **kwargs) - if request.param == "fallback": - ret._sendfile = ret._sendfile_fallback + if request.param == "no_sendfile": + asyncio.set_event_loop(loop_without_sendfile) return ret return maker