Skip to content

[HttpKernel] Fix missing Request in RequestStack for StreamedResponse #51396

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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conversation

nicolas-grekas
Copy link
Member

Q A
Branch? 6.3
Bug fix? yes
New feature? no
Deprecations? no
Tickets Fix #46743
License MIT
Doc PR -

Similar to #51139 by @DaDeather, see discussion there.

Copy link
Contributor

@alexander-schranz alexander-schranz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks fine from my side.

@nicolas-grekas
Copy link
Member Author

Thank you Ismail Turan.

@nicolas-grekas nicolas-grekas merged commit a3e9a00 into symfony:6.3 Aug 16, 2023
@DaDeather
Copy link
Contributor

Thank you Ismail Turan.

Sure happy to help!
But more important thank you guys for your effort 😀!

@nicolas-grekas nicolas-grekas deleted the DaDeather/fix-httpkernel-issue-46743 branch August 23, 2023 16:56
@fabpot fabpot mentioned this pull request Aug 26, 2023
nicolas-grekas added a commit that referenced this pull request Oct 12, 2023
…` (elementaire)

This PR was merged into the 6.3 branch.

Discussion
----------

[HttpKernel] Handle nullable callback of `StreamedResponse`

| Q             | A
| ------------- | ---
| Branch?       | 6.3
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       |  -
| License       | MIT

This PR fixes a bug introduced by #51396. Having a callback in `StreamResponsed` is not a mandatory by design.

So the method `getCallback` must check it like `sendContent` does.

When we are dealing with cacheable ressource, only headers are sent from `StreamResponsed`, for example:

```php
#[Route(path: '/download')]
public function download(Request $request): StreamedResponse
{
    // Get the last modified of a file.
    $lastModified = \DateTime::createFromFormat('d/m/Y H:i:s', '10/10/2023 13:22:00');

    $response = (new StreamedResponse())->setLastModified($lastModified);

    if ($response->isNotModified($request)) {
        return $response;
    }

    // Get the stream of a file and attach it to the callback.
    // ...

    return $response;
}
```

`HttpKernel` class have to handle it else if you obtain `HTTP 500` with message `Value of type null is not callable`.

cc `@nicolas`-grekas

Commits
-------

66648c5 [HttpKernel] Handle nullable callback of StreamedResponse
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants