8000 Range Requests are not correctly handled · Issue #38295 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

Range Requests are not correctly handled #38295

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.

Sign up for GitHub

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

Jump to bottom
Closed
halaei opened this issue Sep 24, 2020 · 0 comments
Closed

Range Requests are not correctly handled #38295

halaei opened this issue Sep 24, 2020 · 0 comments
Labels
Bug HttpFoundation Status: Needs Review

Comments

@halaei
Copy link
halaei commented Sep 24, 2020

Symfony version(s) affected: dev-master

Description
According to https://tools.ietf.org/html/rfc7233, I see a few issues in implementation of range support in Symfony\Component\HttpFoundation\BinaryFileResponse. Here are some parts of rfc that I think are note implemented correctly:

  1. "The 416 (Range Not Satisfiable) status code indicates that none of the ranges in the request's Range header field overlap the current extent of the selected resource or ...".
    Current behaviour: The server respond with 416 status code when the end of selected range is greater than (or equal) the size of the file. It is problematic because because it prevents Nginx reverse proxies with slice config (used for caching large files) to correctly serve files, because the server returns 416 response for the last sliced requested by Nginx. My case is probably not a good use-case for a Symfony-based project, but I used it just for testing Nginx caching:

    location / {
    slice 1m;
    proxy_cache mycache;
    proxy_cache_key   $uri$is_args$args$slice_range;
    proxy_cache_valid 200 206 7d;
    proxy_pass http://example.com;
    proxy_set_header  Range $slice_range;
}

    For a 1.5 MB files, the Nginx cache sends 2 requests to the upstream, one with Range: bytes=0-1048575 for the first 1-megabyte slice, and one with Range: bytes=1048576-2097151 for the rest of file. The second request is technically valid, because its range includes the rest of the file, so overlap is not empty. Based on Nginx implementation and the meaning of word "overlap" mentioned in the rfc, I thing it is not necessary for the end of the range to be smaller that the file size, however, such requests are rejected by this code:

    symfony/src/Symfony/Component/HttpFoundation/BinaryFileResponse.php

    Lines 251 to 254 in 713f30e

    if ($start < 0 || $end > $fileSize - 1) {
    $this->setStatusCode(416);
    $this->headers->set('Content-Range', sprintf('bytes */%s', $fileSize));
    } elseif (0 !== $start || $end !== $fileSize - 1) {

  2. "An origin server MUST ignore a Range header field that contains a range unit it does not understand. A proxy MAY discard a Range header field that contains a range unit it does not understand."
    Current behaviour: 500 if length of unit isn't 5, and bytes range is assumed if length is 5.

  3. "A server MUST ignore a Range header field received with a request method other than GET."
    Current behaviour: Range header is not ignored for POST requests. I guess this is a minor/negligible issue.

  4. Multiple ranges are allowed in the rfc, but Symfony only handles the first range. This is probably fine too.

How to reproduce
Serve some random files with 2000 byte length and log the headers while sending requests to the server via curl/postman with Range header. Since I am using Laravel, here is my Laravel code:

Route::any('file', function (\Illuminate\Http\Request $request) {
    Log::info('headers', $request->header());
    file_put_contents('random.txt', \Illuminate\Support\Str::random(2000));
    return \Illuminate\Support\Facades\Response::file(
        'random.txt'
    )->setImmutable();
});
curl -X GET http://localhost:8000/file -H 'Range: bytes=1000-4000' 
curl -X POST http://localhost:8000/file -H 'Range: bytes=0-1'
curl -X POST http://localhost:8000/file -H 'Range: foo=1-1000'
curl -X POST http://localhost:8000/file -H 'Range: bytes=0-10, 1000-1010'
@halaei halaei added the Bug label Sep 24, 2020
@BattleRattle BattleRattle mentioned this issue Oct 11, 2020
Merged
@fabpot fabpot closed this as completed Oct 14, 2020
fabpot added a commit that referenced this issue Oct 14, 2020
@fabpot
bug #38516 [HttpFoundation] Fix Range Requests (BattleRattle)
89dddd1 
This PR was merged into the 3.4 branch.

Discussion
----------

[HttpFoundation] Fix Range Requests

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | #38295
| License       | MIT
| Doc PR        |

This PR fixes some deviations from [RFC 7233](https://tools.ietf.org/html/rfc7233) for handling range requests, mentioned in #38295.

- overlapping ranges are now satisfiable (e.g. when requested range end is larger than the file size)
- range units other than `bytes` will get ignored
- range requests for methods other than `GET` will be ignored

I did not manage yet to implement the support for multiple ranges, but also don't know, if that's needed here.

Commits
-------

681804b [HttpFoundation] Fix Range Requests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Bug HttpFoundation Status: Needs Review
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@fabpot @xabbuh @halaei @carsonbot
0