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

Support for HTTP range requests (for FileResponse) #950

Open
simonw opened this issue May 17, 2020 · 30 comments
Open

Support for HTTP range requests (for FileResponse) #950

simonw opened this issue May 17, 2020 · 30 comments
Assignees
Labels
clean up Refinement to existing functionality feature New feature or request
Milestone

Comments

@simonw
Copy link
Contributor

simonw commented May 17, 2020

I'm trying to embed an mp4 file on a page using the following HTML:

<video controls width="600">
    <source src="/media/video.mp4" type="video/mp4">
</video>

The video file is being served by a Starlette FileResponse.

I'm getting this error in Safari:

localhost_8001_live-photos_and_Real-time_HTML_Editor

It looks to me like Safari is trying to make an HTTP range request in order to stream the video - but Starlette doesn't support that option.

I tried adding accept-ranges: none as a response header but that didn't seem to fix the problem.

So... it would be great if Starlette could handle range requests so you could use it to serve video files to Safari!

Important

  • We're using Polar.sh so you can upvote and help fund this issue.
  • We receive the funding once the issue is completed & confirmed by you.
  • Thank you in advance for helping prioritize & fund our backlog.
Fund with Polar
@tomchristie
Copy link
Member

We should certainly support that in the StaticFiles app, yup.

We could also consider supporting it in Uvicorn too, so that if an application doesn’t have built-in support for it, then the server will still receive the entire response bytes from the application, but will only send the relevant subsection over the wire.

@abersheeran
Copy link
Member

As a pure ASGI server, uvicorn may not assume this function. I mean this should be implemented in starlette.

@jordic
Copy link

jordic commented Sep 20, 2020

Be aware that with uvloop still doesn't exist sendfile API. If using the default python event loop it should work. When sending files it's important to follow that system API.

@tombulled
Copy link

I just ran into this issue myself and using this Stack Overflow post was able to produce a simple working example for my needs.

I added my code to a public gist here, hopefully it helps anyone else currently facing this issue :)

@Kojiro20
Copy link

I made a simple fix for this here: #1090 - ping me if anyone wants to revitalize pull request. I also posted some monkey-patch workaround and @kevinastone has a good handle on proper solution + monkey-patch.

@abersheeran
Copy link
Member

abersheeran commented Mar 7, 2021

https://github.com/abersheeran/baize/blob/master/baize/asgi.py#L333

https://github.com/abersheeran/baize/blob/23791841f30ca92775e50a544a8606d1d4deac93/baize/asgi/responses.py#L184

If someone needs to respond with a file that supports fragmentation, you can try this library. Just replace starlette.responses.FileResponse.

severo added a commit to huggingface/dataset-viewer that referenced this issue Dec 23, 2021
Starlette cannot serve ranges for static files, see
encode/starlette#950.

Also: fix the rights of the assets directory
Also: add the CORS header (doc)
severo added a commit to huggingface/dataset-viewer that referenced this issue Dec 23, 2021
Starlette cannot serve ranges for static files, see
encode/starlette#950.

Also: fix the rights of the assets directory
Also: add the CORS header (doc)
severo added a commit to huggingface/dataset-viewer that referenced this issue Dec 23, 2021
Starlette cannot serve ranges for static files, see
encode/starlette#950.

Also: fix the rights of the assets directory
Also: add the CORS header (doc)
@Kludex Kludex self-assigned this Apr 29, 2022
@Kludex
Copy link
Sponsor Member

Kludex commented May 19, 2022

@abersheeran Your link has expired 😢 Which line were you mentioning? This one?

@abersheeran
Copy link
Member

@abersheeran您的链接已过期😢你提到了哪一行?这个

Yes.

@honglei
Copy link

honglei commented May 27, 2022

After two years, will starlette support this feature ?

@honglei
Copy link

honglei commented May 27, 2022

@abersheeran Will you commit a merge request for this ?

@Kludex
Copy link
Sponsor Member

Kludex commented May 31, 2022

As a less disruptive (and fast...) approach, a PR documenting the usage using baize's FileResponse is welcome. 🙏

@Kludex
Copy link
Sponsor Member

Kludex commented Aug 15, 2022

@abersheeran Do you think Starlette should support this? If not, should we add a note on the FileResponse section itself about baize.asgi.FileResponse, and close this as "Won't do"?

@abersheeran
Copy link
Member

@abersheeran Do you think Starlette should support this? If not, should we add a note on the FileResponse section itself about baize.asgi.FileResponse, and close this as "Won't do"?

I think Starlette could consider adding this feature. It is not complicated and requires little extra maintenance cost.

@Kludex
Copy link
Sponsor Member

Kludex commented Aug 17, 2022

I'm willing to review a PR with it. StaticFiles need to be considered on the PR, I guess.

@Kludex Kludex added clean up Refinement to existing functionality help wanted Feel free to help labels Aug 17, 2022
@kevinastone
Copy link
Contributor

@Kludex The PR was here but was closed as wontfix by Tom: #1013

@Kludex
Copy link
Sponsor Member

Kludex commented Aug 17, 2022

Ah! I didn't remember Tom's comment there.

Let's be objective here.

We have three options to close this issue:

  1. Close this issue without taking any further action.
  2. Recommend baize.asgi.Files and baize.asgi.FileResponse as notes (?) on the StaticFiles and FileResponse sections on our docs.
  3. Implement what this issue proposes (reopening Added Range support to Staticfiles #1013 is an option, ofc).

FWIW, I'm fine with (2) and (3). We usually prefer to add documentation whenever possible... The thing is that we also take a lot of decisions based on decisions from other web frameworks, e.g. Django and Flask.

For Django, this feature was accepted 8 years ago (https://code.djangoproject.com/ticket/22479), but the feature was not merged... There's a comment on that ticket recommending a middleware approach.

For Flask, this feature is supported.

Given the above, considering the options we have, Tom's comment, Django not prioritizing this feature in 8 years... I think the approach that makes more sense to me is to start with documentation, close this issue, and in the future, if further requested, we can reevaluate this decision.

@simonw
Copy link
Contributor Author

simonw commented Aug 17, 2022

I'd be disappointed to see this not land in Starlette. I'd understand if it required additional dependencies and a complex implementation, but I feel like the implementation in #1013 is small enough and clean enough that it shouldn't add undue complexity to the project.

And supporting range requests has a whole bunch of applications beyond just getting the HTML <video> tag to work properly:

  • Support reusable downloads using tools like curl -C and wget --continue - or the download UI in browsers like Chrome and Firefox
  • Enable really neat tricks like the one described in Hosting SQLite databases on Github Pages, where the range header is used to allow queries to be executed against giant SQLite databases without needing to download the entire database first

@simonwarchol
Copy link

simonwarchol commented Aug 17, 2022

^^ Agreed -- I work in Biomedical Imaging, and range requests let me serve specific portions of 100GB+ multi-channel TIFF files for on-the-fly rendering.

@Kludex
Copy link
Sponsor Member

Kludex commented Aug 17, 2022

I'd be disappointed to see this not land in Starlette. I'd understand if it required additional dependencies and a complex implementation, but I feel like the implementation in #1013 is small enough and clean enough that it shouldn't add undue complexity to the project.

And supporting range requests has a whole bunch of applications beyond just getting the HTML <video> tag to work properly:

  • Support reusable downloads using tools like curl -C and wget --continue - or the download UI in browsers like Chrome and Firefox
  • Enable really neat tricks like the one described in Hosting SQLite databases on Github Pages, where the range header is used to allow queries to be executed against giant SQLite databases without needing to download the entire database first

If I may... This issue has more than two years, would you mind sharing what you did to overcome Starlette's limitation here?

@simonwarchol
Copy link

I use from baize.asgi 's FileResponse instead

@simonw
Copy link
Contributor Author

simonw commented Aug 17, 2022

If I may... This issue has more than two years, would you mind sharing what you did to overcome Starlette's limitation here?

I took the easiest possible route and used a YouTube embed instead.

@Kludex Kludex removed the help wanted Feel free to help label Aug 18, 2022
Kludex added a commit that referenced this issue Jan 7, 2023
This PR aims to solve #950.

The implementation differs from [baize's
FileResponse](https://github.com/abersheeran/baize/blob/23791841f30ca92775e50a544a8606d1d4deac93/baize/asgi/responses.py#L184),
since that one takes in consideration the "range" request header. The
desgin decision is justified as the Response classes in Starlette are
naive in regards to the request.
@Kludex
Copy link
Sponsor Member

Kludex commented Jan 7, 2023

An update here: I've implemented #1999, but I'm not satisfied. It follows a similar approach to #1013 i.e. receives a range as a tuple.

I'll try to develop an alternative solution mentioned on the PR, on which I'll take in consideration the request headers.

@Kludex Kludex added this to the Version 1.x milestone Feb 4, 2023
@Kludex
Copy link
Sponsor Member

Kludex commented Feb 4, 2023

Let's release 1.0 first, and come back to this later on. It will not be a breaking change anyway - I think. 👀

@teddy171
Copy link

teddy171 commented Apr 8, 2023

@abersheeran Could you tell me how to use baize.asgi.FileResponse to send file? It seem only can return header.

{"status_code":200,"headers":{"accept-ranges":"bytes","last-modified":"Sat, 08 Apr 2023 06:11:11 GMT","etag":"22f29ae79c1e6389cbde785a591426c3f463b5d7","content-disposition":"attachment; filename=\"demo.md\"; filename*=utf-8''demo.md"},"cookies":[],"filepath":"./demo.md","content_type":"application/octet-stream","download_name":null,"stat_result":[33188,2738687,16777237,1,501,20,19,1680934310,1680934271,1680934271],"chunk_size":262144}

Can you add an example to docs?

@abersheeran
Copy link
Member

@teddy171
Copy link

teddy171 commented Apr 8, 2023

import uvicorn
from fastapi import FastAPI, Response
from baize.asgi.responses import FileResponse


class ChunkFileResponse(Response):
    def __init__(self, *args, **kwargs) -> None:
        if len(args) == 1:
            kwargs = args[0]
            [kwargs.pop(k) for k in ['status_code', 'cookies', 'stat_result']]
        super().__init__(**kwargs)


app = FastAPI()


@app.get("/")
def file():
    return ChunkFileResponse(filepath="./demo.md")


if __name__ == '__main__':
    uvicorn.run('main:app', port=5555, reload=True)

Could you tell me what's wrong with the code? I still cannot get the file. It still shows json.

@septatrix
Copy link

This makes several some WebKit browsers unable to play videos served by starlette. In some forums I saw that iOS was affected, though I am not sure if that is still the case. Otherwise cog (an embedded browser using WPE) and Gnome Web/Epiphany are likely some notable ones. They use gstreamer under the hood which fails to play videos without range request support by the source:

Now playing http://localhost:8000/api/cache/2b05bd47-ce21-4436-a565-953a85b000ad-Comedyspot Vögel.mp4
Prerolling...
ERROR Server does not support seeking. for http://localhost:8000/api/cache/2b05bd47-ce21-4436-a565-953a85b000ad-Comedyspot Vögel.mp4
ERROR debug information: ../ext/soup/gstsouphttpsrc.c(1948): gst_soup_http_src_do_request (): /GstPlayBin:playbin/GstURIDecodeBin:uridecodebin0/GstSoupHTTPSrc:source:
Server does not accept Range HTTP header, URL: http://localhost:8000/api/cache/2b05bd47-ce21-4436-a565-953a85b000ad-Comedyspot Vögel.mp4, Redirect to: (NULL)
Reached end of play list.

@simonw
Copy link
Contributor Author

simonw commented Oct 24, 2023

Found another really compelling use-case for HTTP range headers: PMTiles, which lets you serve a single file with a vector map of the world (107GB for the whole planet to street level) which can then be served to browsers using range requests to get just the data needed for a specific area.

More on that here: https://protomaps.com/

I wrote about my explorations here: https://til.simonwillison.net/gis/pmtiles

@septatrix
Copy link

For Django, this feature was accepted 8 years ago (https://code.djangoproject.com/ticket/22479), but the feature was not merged... There's a comment on that ticket recommending a middleware approach.

For Flask, this feature is supported.

This is also supported by Quart (async Flask)

mattstern31 added a commit to mattstern31/datasets-server-storage-admin that referenced this issue Nov 11, 2023
Starlette cannot serve ranges for static files, see
encode/starlette#950.

Also: fix the rights of the assets directory
Also: add the CORS header (doc)
mattstern31 added a commit to mattstern31/datasets-server-storage-admin that referenced this issue Nov 11, 2023
Starlette cannot serve ranges for static files, see
encode/starlette#950.

Also: fix the rights of the assets directory
Also: add the CORS header (doc)
@ubipo
Copy link

ubipo commented Nov 21, 2023

@teddy171

Could you tell me what's wrong with the code? I still cannot get the file. It still shows json.

I had the same issue. I resorted to proxying to the Baize FileResponse class instead of inheriting from it. This worked for me:

from baize.asgi.responses import FileResponse as BaizeFileResponse
from fastapi.responses import Response as FastApiResponse


class FastApiBaizeFileResponse(FastApiResponse):
    _baize_response: BaizeFileResponse

    def __init__(self, path, **kwargs) -> None:
        filepath = str(kwargs.get("filepath", kwargs.get("path", path)))
        kwargs.pop("filepath", None)
        kwargs.pop("path", None)
        self._baize_response = BaizeFileResponse(filepath, **kwargs)
        super().__init__(None)

    def __call__(self, *args, **kwargs):
        return self._baize_response(*args, **kwargs)
    
    def __getattr__(self, name):
        return getattr(self._baize_response, name)

Use it in the same way you'd use the FastAPI FileResponse:

return FastApiBaizeFileResponse(path.absolute())

I suppose FastAPI does some instanceof(x, fastapi.responses.Response) check somewhere.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clean up Refinement to existing functionality feature New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.