bug #9714 [HttpFoundation] BinaryFileResponse should also return 416 or 200 on some range-requets (SimonSimCity)
This PR was submitted for the 2.3-dev branch but it was merged into the 2.3 branch instead (closes #9714). Discussion ---------- [HttpFoundation] BinaryFileResponse should also return 416 or 200 on some range-requets | Q | A | ------------- | --- | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | | License | MIT | Doc PR | I read around in the docs and tried something with BinaryFileResponse ... There are two things I missed in the implementation here: * If the range, provided in the query, does exceed the file-position, return an 416 (or 200 - I decided to take 416 here) * If the range is logical invalid (f.e. to request from byte 50 to byte 30 ..) return 200 - OK One decision I took in addition, is to provide a 200 code if the full file is requested. For me, it doesn't make sense here to return a 206 for just the complete file. What I'm quite unsure about: Do we need some additional fields for these two options? I can remember reading something about a Content-Range for 416 but I was quite unsure what it should be ... Commits ------- d5dc317 [HttpFoundation] BinaryFileResponse should also return 416 or 200 on some range-requets
This commit is contained in:
commit
70b9df7860
@ -216,17 +216,20 @@ class BinaryFileResponse extends Response
|
||||
$start = (int) $start;
|
||||
}
|
||||
|
||||
$start = max($start, 0);
|
||||
$end = min($end, $fileSize - 1);
|
||||
|
||||
if ($start <= $end) {
|
||||
if ($start < 0 || $end > $fileSize - 1) {
|
||||
$this->setStatusCode(self::HTTP_REQUESTED_RANGE_NOT_SATISFIABLE);
|
||||
} elseif ($start !== 0 || $end !== $fileSize - 1) {
|
||||
$this->maxlen = $end < $fileSize ? $end - $start + 1 : -1;
|
||||
$this->offset = $start;
|
||||
|
||||
$this->setStatusCode(206);
|
||||
$this->setStatusCode(self::HTTP_PARTIAL_CONTENT);
|
||||
$this->headers->set('Content-Range', sprintf('bytes %s-%s/%s', $start, $end, $fileSize));
|
||||
$this->headers->set('Content-Length', $end - $start + 1);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
return $this;
|
||||
}
|
||||
|
@ -84,12 +84,73 @@ class BinaryFileResponseTest extends ResponseTestCase
|
||||
return array(
|
||||
array('bytes=1-4', 1, 4, 'bytes 1-4/35'),
|
||||
array('bytes=-5', 30, 5, 'bytes 30-34/35'),
|
||||
array('bytes=-35', 0, 35, 'bytes 0-34/35'),
|
||||
array('bytes=-40', 0, 35, 'bytes 0-34/35'),
|
||||
array('bytes=30-', 30, 5, 'bytes 30-34/35'),
|
||||
array('bytes=30-30', 30, 1, 'bytes 30-30/35'),
|
||||
array('bytes=30-34', 30, 5, 'bytes 30-34/35'),
|
||||
array('bytes=30-40', 30, 5, 'bytes 30-34/35')
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* @dataProvider provideFullFileRanges
|
||||
*/
|
||||
public function testFullFileRequests($requestRange)
|
||||
{
|
||||
$response = BinaryFileResponse::create(__DIR__.'/File/Fixtures/test.gif')->setAutoEtag();
|
||||
|
||||
// prepare a request for a range of the testing file
|
||||
$request = Request::create('/');
|
||||
$request->headers->set('Range', $requestRange);
|
||||
|
||||
$file = fopen(__DIR__.'/File/Fixtures/test.gif', 'r');
|
||||
$data = fread($file, 35);
|
||||
fclose($file);
|
||||
|
||||
$this->expectOutputString($data);
|
||||
$response = clone $response;
|
||||
$response->prepare($request);
|
||||
$response->sendContent();
|
||||
|
||||
$this->assertEquals(200, $response->getStatusCode());
|
||||
$this->assertEquals('binary', $response->headers->get('Content-Transfer-Encoding'));
|
||||
}
|
||||
|
||||
public function provideFullFileRanges()
|
||||
{
|
||||
return array(
|
||||
array('bytes=0-'),
|
||||
array('bytes=0-34'),
|
||||
array('bytes=-35'),
|
||||
// Syntactical invalid range-request should also return the full resource
|
||||
array('bytes=20-10'),
|
||||
array('bytes=50-40'),
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* @dataProvider provideInvalidRanges
|
||||
*/
|
||||
public function testInvalidRequests($requestRange)
|
||||
{
|
||||
$response = BinaryFileResponse::create(__DIR__.'/File/Fixtures/test.gif')->setAutoEtag();
|
||||
|
||||
// prepare a request for a range of the testing file
|
||||
$request = Request::create('/');
|
||||
$request->headers->set('Range', $requestRange);
|
||||
|
||||
$response = clone $response;
|
||||
$response->prepare($request);
|
||||
$response->sendContent();
|
||||
|
||||
$this->assertEquals(416, $response->getStatusCode());
|
||||
$this->assertEquals('binary', $response->headers->get('Content-Transfer-Encoding'));
|
||||
#$this->assertEquals('', $response->headers->get('Content-Range'));
|
||||
}
|
||||
|
||||
public function provideInvalidRanges()
|
||||
{
|
||||
return array(
|
||||
array('bytes=-40'),
|
||||
array('bytes=30-40')
|
||||
);
|
||||
}
|
||||
|
||||
|
Reference in New Issue
Block a user