Project

General

Profile

Actions

Bug #68506

closed

Extbase must use response-object to set response-headers/statuscode

Added by Stefan Neufeind almost 9 years ago. Updated over 6 years ago.

Status:
Closed
Priority:
Must have
Assignee:
-
Category:
Extbase
Target version:
Start date:
2015-07-24
Due date:
% Done:

100%

Estimated time:
TYPO3 Version:
7
PHP Version:
Tags:
Complexity:
medium
Is Regression:
Yes
Sprint Focus:
Stabilization Sprint

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.


Related issues 3 (0 open3 closed)

Related to TYPO3 Core - Task #67558: Introduce Request/Response objects for the Bootstrap based on PSR-7ClosedBenni Mack2015-06-17

Actions
Related to TYPO3 Core - Bug #68595: Module Menu vanishes when creating a new Content ElementClosed2015-07-28

Actions
Related to TYPO3 Core - Bug #68670: Do not let legacy code take precedence over HTTP headersClosed2015-07-31

Actions
Actions #1

Updated by Susanne Moog almost 9 years ago

  • Target version set to 7.4 (Backend)
  • Sprint Focus set to Stabilization Sprint
Actions #2

Updated by Markus Klein almost 9 years ago

  • Category set to Extbase
  • Status changed from New to Accepted
Actions #3

Updated by Helmut Hummel almost 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.

Actions #4

Updated by Stefan Neufeind almost 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!

Actions #5

Updated by Markus Klein almost 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.

Actions #6

Updated by Stefan Neufeind almost 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.)

Actions #7

Updated by Helmut Hummel almost 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.

Actions #8

Updated by Helmut Hummel almost 9 years ago

  • Status changed from Needs Feedback to Accepted
Actions #9

Updated by Markus Klein almost 9 years ago

Ah ok, that makes sense, thanks for investigation.

I see no other way than hacking the psr-7 objects into extbase.

Actions #10

Updated by Alexander Opitz almost 9 years ago

Does

if (headers_sent() || count(headers_list()) > 0) {}

helps?

Actions #11

Updated by Gerrit Code Review almost 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

Actions #12

Updated by Benni Mack almost 9 years ago

thanks Alex!

Actions #13

Updated by Benni Mack almost 9 years ago

  • Status changed from Under Review to Resolved
  • % Done changed from 0 to 100
Actions #14

Updated by Alexander Opitz almost 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)
Actions #15

Updated by Daniel Goerz almost 9 years ago

For documentation purposes:
The Patch (42049) introduced a regression: https://forge.typo3.org/issues/68595 and therefore was reverted.

Actions #16

Updated by Gerrit Code Review almost 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

Actions #17

Updated by Gerrit Code Review almost 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

Actions #18

Updated by Gerrit Code Review almost 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

Actions #19

Updated by Gerrit Code Review almost 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

Actions #20

Updated by Gerrit Code Review almost 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

Actions #21

Updated by Markus Klein almost 9 years ago

  • Status changed from Under Review to Resolved
Actions #22

Updated by Riccardo De Contardi over 6 years ago

  • Status changed from Resolved to Closed
Actions

Also available in: Atom PDF