Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[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 to0c52104Comparecarsonbot commentedMar 2, 2023
Hey! I think@alli83 has recently worked with this code. Maybe they can help review this? Cheers! Carsonbot |
fabpot commentedOct 11, 2023
Would it be possible to add support in |
0c52104 tob1a7fbfComparealexandre-daubois commentedOct 11, 2023
It doesn't seem to. |
nicolas-grekas commentedOct 11, 2023
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 to0151376CompareStreamedResponse::createFromTempFile()\SplTempFileObject inBinaryFileResponsealexandre-daubois commentedOct 11, 2023
Thank you for the hint Nicolas, I tried a implementation in |
Uh oh!
There was an error while loading.Please reload this page.
0151376 to8859741Comparefabpot commentedFeb 3, 2024
@alexandre-daubois Can you rebase on current 7.1? Thank you. |
8859741 to7b6ad2cComparealexandre-daubois commentedFeb 5, 2024
Yep of course, here you go 🙂 |
fabpot commentedFeb 5, 2024
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
\SplTempFileObjectinBinaryFileResponse