Bug #68506
closedExtbase must use response-object to set response-headers/statuscode
100%
Description
With #67558 the core got a request/response-model according to PSR-7. This however seems to lead to problems with Extbase, since the response-handling of Extbase isn't yet adapted.
Extbase senders headers to the browser directly instead of returning those through the response-object we have now. This happens in typo3/sysext/extbase/Classes/Mvc/Web/Response.php, sendHeaders(). When Extbase wants to set a custom statusCode, it sends a header directly. But when finishing the request the response-object overwrites that status to "200 OK" again.
Headers shouldn't be sent by Extbase anyway but instead be properly passed through to the response-object.
Updated by Susanne Moog over 9 years ago
- Target version set to 7.4 (Backend)
- Sprint Focus set to Stabilization Sprint
Updated by Markus Klein over 9 years ago
- Category set to Extbase
- Status changed from New to Accepted
Updated by Helmut Hummel over 9 years ago
This imho is a bigger feature, that should not be tackled in the stabilization sprint.
With PSR-7 in, we most likely need to reconsider even more parts of the reques/response handling.
Updated by Stefan Neufeind over 9 years ago
Afaik Benni said he saw a chance for a temporary workaround to make HTTP-status-codes set from an extbase-extension work again (simply put: not override the status-header from the response-object). If possible that would be great, if not (for 7.4) then fine for the moment. But it's something that worked in the past and now broke!
Updated by Markus Klein over 9 years ago
- Status changed from Accepted to Needs Feedback
I took a look into the implementation now.
I'm not sure how to reproduce this issue, since the Bootstrap code checks for headers_sent()
, hence if Extbase sent the headers, it will not do anything.
Updated by Stefan Neufeind over 9 years ago
The response-object (for psr-7) added with c048cede7a402275e501d7a86cb560995f29360b sends the status in the header, which defaults to 200.
If in typo3/sysext/extbase/Classes/Mvc/Web/Response.php you use setStatus() to set a different status-code through an extbase-plugin, extbase will output that as the header. If I put an immediate stop after the extbase-output I get the status. If however the request finishes the normal way, it seems to get overwritten with 200 again.
You are right that typo3/sysext/core/Classes/Core/Bootstrap.php checks for headers_sent(). But I assume the headers are just set and not yet sent to the browser, as no output was done inbetween. The output from extbase is passed back to the CMS-core to be output - it's just that headers were already set.
Maybe a possible hack would be to make Extbase send its output itself after setting the headers (and returning an empty string to the Bootstrap). Thus headers_sent() would then be true and Bootstrap wouldn't try to write headers itself. (I can't test that right now, but could later.)
Updated by Helmut Hummel over 9 years ago
I got it now.
The status header is ALWAYS overridden by the PSR-7 implementation.
Markus Klein wrote:
I'm not sure how to reproduce this issue, since the Bootstrap code checks for
headers_sent()
, hence if Extbase sent the headers, it will not do anything.
Using the header() function does not mean that PHP does send the headers! Headers are sent once the first body line needs to be sent, so this check under normal conditions always is true.
Steps to reproduce:
Create an Extbase action that only does a
$this->redirect('anotherAction');
use curl to check the response of that:
curl -s -D - 'http://test.t3/url/to/redirect/action/' -o /dev/null
You will see a Location: header but with a 200 response code, which is invalid. We did not recognize this as browser seem to be graceful and redirect anyway.
Updated by Helmut Hummel over 9 years ago
- Status changed from Needs Feedback to Accepted
Updated by Markus Klein over 9 years ago
Ah ok, that makes sense, thanks for investigation.
I see no other way than hacking the psr-7 objects into extbase.
Updated by Alexander Opitz over 9 years ago
Does
if (headers_sent() || count(headers_list()) > 0) {}
helps?
Updated by Gerrit Code Review over 9 years ago
- Status changed from Accepted to Under Review
Patch set 1 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at http://review.typo3.org/42049
Updated by Benni Mack over 9 years ago
- Status changed from Under Review to Resolved
- % Done changed from 0 to 100
Applied in changeset 8e9a766da84cba945767423c722ace454885041a.
Updated by Alexander Opitz over 9 years ago
- Status changed from Resolved to Accepted
Patch reverted as it has to much side effects.
- Doesn't respect X-PHP-Powered (if set in ini) and so catches every time
- Do not catch for status "header"
- X-AJAX in some requests destroy all (See #68595)
Updated by Daniel Goerz over 9 years ago
For documentation purposes:
The Patch (42049) introduced a regression: https://forge.typo3.org/issues/68595 and therefore was reverted.
Updated by Gerrit Code Review over 9 years ago
- Status changed from Accepted to Under Review
Patch set 1 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at http://review.typo3.org/42122
Updated by Gerrit Code Review over 9 years ago
Patch set 2 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at http://review.typo3.org/42122
Updated by Gerrit Code Review over 9 years ago
Patch set 3 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at http://review.typo3.org/42122
Updated by Gerrit Code Review over 9 years ago
Patch set 4 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at http://review.typo3.org/42122
Updated by Gerrit Code Review over 9 years ago
Patch set 5 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at http://review.typo3.org/42122
Updated by Markus Klein over 9 years ago
- Status changed from Under Review to Resolved
Applied in changeset 6b2e1318b6bed2622c5a3e82df1e13736a83f65f.
Updated by Riccardo De Contardi about 7 years ago
- Status changed from Resolved to Closed