Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Simple support for range headers #1090

Closed
wants to merge 6 commits into from

Conversation

Kojiro20
Copy link

With this change FileResponse can be used to serve media that use the range header such as HTML5 audio and video tags.

For example, using FastAPI app container you can support range like this:

@app.get("/media/{file_path}")
async def media(file_path: str, range_header: Optional[str] = Header('bytes=0-', alias="Range")):
    full_path = os.path.join('media', file_path)
    start, end = range_header.strip('bytes=').split('-')
    size = os.stat(full_path)[6]
    end = min(size-1, int(start)+1024)
    return FileResponse(full_path, status_code=206, offset=int(start), headers={
        'Accept-Ranges': 'bytes',
        'Content-Range': 'bytes %s-%s/%s' % (start, end, size),
        'Content-Length': str(size)
    })

with HTML:

<!DOCTYPE html>
<html>
  <body>
    <audio preload="auto" tabindex="0" controls="" type="audio/mp3">
      <source src="media/tunes.mp3">
    </audio>
  </body>
</html>

@Kojiro20
Copy link
Author

Kojiro20 commented Nov 13, 2020

In case maintainers reject this, for anyone else who needs to support range header queries in FastAPI or Starlette, you can still achieve the same functionality by extending FileResponse like this:

class MediaResponse(FileResponse):
    def __init__(self, *args, **kwargs):
        self.offset = kwargs.pop('offset')
        super(MediaResponse, self).__init__(*args, **kwargs)

    async def __call__(self, scope: Scope, receive: Receive, send: Send) -> None:
        if self.stat_result is None:
            try:
                stat_result = await aiofiles.os.stat(self.path)
                self.set_stat_headers(stat_result)
            except FileNotFoundError:
                raise RuntimeError(f"File at path {self.path} does not exist.")
            else:
                mode = stat_result.st_mode
                if not stat.S_ISREG(mode):
                    raise RuntimeError(f"File at path {self.path} is not a file.")
        await send(
            {
                "type": "http.response.start",
                "status": self.status_code,
                "headers": self.raw_headers,
            }
        )
        if self.send_header_only:
            await send({"type": "http.response.body", "body": b"", "more_body": False})
        else:
            async with aiofiles.threadpool.open(self.path, mode='rb') as file:
                await file.seek(self.offset)
                more_body = True
                while more_body:
                    chunk = await file.read(self.chunk_size)
                    more_body = len(chunk) == self.chunk_size
                    await send(
                        {
                            "type": "http.response.body",
                            "body": chunk,
                            "more_body": more_body,
                        }
                    )
        if self.background is not None:
            await self.background()

and using like this:

@app.get("/media/{path}")
async def media(path: str, range_header: Optional[str] = Header('bytes=0-', alias="Range")):
    if '..' in path:
        raise Exception(path + ' is not allowed')

    full_path = os.path.join('media', path)
    start, end = range_header.strip('bytes=').split('-')
    start = int(start)
    size = os.stat(full_path)[6]
    end = min(size-1, start+10)
    return MediaResponse(path=full_path, status_code=206, offset=start, headers={
        'Accept-Ranges': 'bytes',
        'Content-Range': 'bytes %s-%s/%s' % (start, end, size),
        'Content-Length': str(size-start)
    })

@kevinastone
Copy link
Contributor

This has been in the PR queue for quite a while: #1013

@Kojiro20
Copy link
Author

@kevinastone I wasn't able to use #1013 via monkey-patching or extension, but I agree it is a better solution to enable this out-of-box through staticfiles. I almost didn't make a PR after seeing how long yours had been waiting. However, I wanted to share my work-around since it could be a while before range header support makes it to FastAPI.

This solution could also enable other functionality that is currently lacking. Chunked file transfer or anything that uses 206 to resume is currently impossible (or incredibly slow using StreamResponse) without some trickery.

@kevinastone
Copy link
Contributor

@Kojiro20 Here's a copy of the monkey-patch I'm currently running to support range requests: https://gist.github.com/kevinastone/a6a62db57577b3f24e8a6865ed311463

@Kojiro20
Copy link
Author

@kevinastone thank you! Will use that if I do another server project in python.

But, TBH, will probably stick with node or java for a few more years after this experience. It's too hard to get real http2 support out of python and I'm very confused about all the busted/incompatible versions for asgi tooling. Even Conda not solved dependency tree for fastAPI + h2 etc 😖 ... Will never touch wsgi again, but solid asgi support feels nascent at best. Watching for python to natively support promises 🤔 – that was about the time node seemed mature.

Seems like Guido letting go of the helm was a mixed blessing. Now everyone is fearless to break APIs 🥳 but slow to adopt upstream changes 🥉

@Kojiro20
Copy link
Author

For posterity - starlette has a memory leak with FileResponse.

I think the fix would involve adding a watchdog of some sort inside the async with open (... block to clean up unused connections which haven't sent new data within some timeout. It's currently possible for a client to request data from two files, then ignore one of them while continuing to read from the other. When this happens the ignored file is held open indefinitely. The connection is still open and valid, but nothing cleans up the handle/memory for files which weren't fully transferred.

Honestly curious if anyone runs a python service that doesn't need to be rebooted periodically to stay afloat. I have added something like this to my Dockerfile to mitigate.

RUN echo "0 * * * * /usr/bin/pkill -f server.py && /usr/bin/python3 /server.py &" >> /etc/crontab

@gimebreak
Copy link

@Kojiro20 Here's a copy of the monkey-patch I'm currently running to support range requests: https://gist.github.com/kevinastone/a6a62db57577b3f24e8a6865ed311463

have you done a PR? I think they should update this block of code

@somebuckaroo
Copy link

@Kojiro20 Here's a copy of the monkey-patch I'm currently running to support range requests: https://gist.github.com/kevinastone/a6a62db57577b3f24e8a6865ed311463

Not sure if this is still the preferred solution for ranged support for static files, but I think there's an off-by-one error in OpenRange. This is easily verified with curl.

curl -v -r 1000- http://127.0.0.1:xxxx/somefile

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants