Skip to content

Commit

Permalink
bug #58712 [HttpFoundation] Fix support for \SplTempFileObject in `…
Browse files Browse the repository at this point in the history
…BinaryFileResponse` (elementaire)

This PR was merged into the 7.1 branch.

Discussion
----------

[HttpFoundation] Fix support for `\SplTempFileObject` in `BinaryFileResponse`

| Q             | A
| ------------- | ---
| Branch?       | 7.1
| Bug fix?      | yes
| PR original Feature | #49144
| New feature?  | no
| Deprecations? | no
| Issues        |  -
| License       | MIT

We can not call some methods with an object of `\SplTempFileObject()`. We get this error: `[...] stat failed for php://temp`.

I have checked the code against methods listed in [this note](https://www.php.net/manual/en/class.spltempfileobject.php#128962).

I have found:
- `getMTime()` called in `BinaryFileResponse::setAutoLastModified()`
- `getSize()` called in `BinaryFileResponse::prepare()`
- `isReadable()` called in `BinaryFileResponse::setFile()` (already safe)

I have updated the unit test and patched the class `BinaryFileResponse`.

Note: calling `SplFileObject::fstat()` gives `mtime` equals to `0`. I think it is nonsense to use that as value for last modified. I have decided to use `time()` because i guess, we can not do better. Indeed, we have no idea how much time have passed between making the temp file and the call to `setAutoLastModified()` by the developper.

Commits
-------

a104d50cb7a Fix support for \SplTempFileObject in BinaryFileResponse
  • Loading branch information
nicolas-grekas committed Nov 5, 2024
2 parents 6472a9f + 40f90f6 commit 4f4d5a2
Show file tree
Hide file tree
Showing 2 changed files with 7 additions and 2 deletions.
6 changes: 4 additions & 2 deletions BinaryFileResponse.php
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ public function setChunkSize(int $chunkSize): static
*/
public function setAutoLastModified(): static
{
$this->setLastModified(\DateTimeImmutable::createFromFormat('U', $this->file->getMTime()));
$this->setLastModified(\DateTimeImmutable::createFromFormat('U', $this->tempFileObject ? time() : $this->file->getMTime()));

return $this;
}
Expand Down Expand Up @@ -197,7 +197,9 @@ public function prepare(Request $request): static
$this->offset = 0;
$this->maxlen = -1;

if (false === $fileSize = $this->file->getSize()) {
if ($this->tempFileObject) {
$fileSize = $this->tempFileObject->fstat()['size'];
} elseif (false === $fileSize = $this->file->getSize()) {
return $this;
}
$this->headers->remove('Transfer-Encoding');
Expand Down
3 changes: 3 additions & 0 deletions Tests/BinaryFileResponseTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -451,6 +451,9 @@ public function testCreateFromTemporaryFile()
$this->assertEquals('attachment; filename=temp', $response->headers->get('Content-Disposition'));

ob_start();
$response->setAutoLastModified();
$response->prepare(new Request());
$this->assertSame('7', $response->headers->get('Content-Length'));
$response->sendContent();
$string = ob_get_clean();
$this->assertSame('foo,bar', $string);
Expand Down

0 comments on commit 4f4d5a2

Please # to comment.