Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.6k
[HttpFoundation] Add support for\SplTempFileObject
inBinaryFileResponse
#49144
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
1ff0704
to0c52104
Comparecarsonbot commentedMar 2, 2023
Hey! I think@alli83 has recently worked with this code. Maybe they can help review this? Cheers! Carsonbot |
Would it be possible to add support in |
0c52104
tob1a7fbf
CompareIt doesn't seem to. |
BinaryFileResponse accepts an SplFileInfo as argument and SplTempFileObject extends SplFileInfo, so it's already possible to pass a SplTempFileObject there. Can't we make BinaryFileResponse able to deal with such instances, skipping all the features that need a physical file when this happens? |
6294b4a
to0151376
CompareStreamedResponse::createFromTempFile()
\SplTempFileObject
inBinaryFileResponse
Thank you for the hint Nicolas, I tried a implementation in |
Uh oh!
There was an error while loading.Please reload this page.
0151376
to8859741
Compare@alexandre-daubois Can you rebase on current 7.1? Thank you. |
8859741
to7b6ad2c
CompareYep of course, here you go 🙂 |
Thank you@alexandre-daubois. |
…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 | MITWe 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-------a104d50 Fix support for \SplTempFileObject in BinaryFileResponse
Uh oh!
There was an error while loading.Please reload this page.
Add support for
\SplTempFileObject
inBinaryFileResponse