Problem/Motivation
While working on #2737719: EntityResource: Provide comprehensive test coverage: for every entity type, every format, every method, I noticed that responses to DELETE
requests correctly have status 204, correctly have no body, but incorrectly have a Content-Type
header, and it's even set tot he completely nonsensical text/html; charset=UTF-8
. See https://tools.ietf.org/html/rfc2616#section-10.2.5 and #2737719-94: EntityResource: Provide comprehensive test coverage: for every entity type, every format, every method.
The root cause: \Symfony\Component\HttpFoundation\Response::prepare()
:
// Fix Content-Type
$charset = $this->charset ?: 'UTF-8';
if (!$headers->has('Content-Type')) {
$headers->set('Content-Type', 'text/html; charset='.$charset);
} elseif (0 === stripos($headers->get('Content-Type'), 'text/') && false === stripos($headers->get('Content-Type'), 'charset')) {
// add the charset
$headers->set('Content-Type', $headers->get('Content-Type').'; charset='.$charset);
}
Nope, this was fixed a long time ago: https://github.com/symfony/symfony/issues/12744 + https://github.com/symfony/symfony/commit/75abd1a451d97f5075128ea10b2598....
The real root cause is PHP: https://bugs.php.net/bug.php?id=70189. This bug exists even in PHP 7.0 (at least in 7.0.12). The fix was committed in March 2016, but it either didn't make it to PHP 7, or it didn't actually solve the problem.
I can reproduce this both with the CLI (when running PHPUnit), and when accessing Drupal via Apache (when performing actual DELETE
requests to an actual Drupal instance).
Proposed resolution
Fix the logic.
Wait for PHP to be fixed, then make the tests more strict.
Remaining tasks
TBD
User interface changes
None.
API changes
None.
Data model changes
None.
Comment | File | Size | Author |
---|---|---|---|
#11 | 2821711-11.patch | 1.25 KB | Wim Leers |
Comments
Comment #2
Wim LeersComment #3
Wim LeersComment #4
Wim LeersI looked into this again, and wanted to make sure this is not a PHP problem.
I created:
test.php
test.html
(empty file)
Then I performed DELETE requests to both:
Both have
Content-Type: text/html
response headers. Even when I addheader_remove('Content-Type');
totest.php
, the server still contains aContent-Type: text/html
response header.Also, it's impossible to detect which HTTP server is used from within the PHP CLI. So… simply documenting all this is all we can do for now.
Comment #6
tedbowThis look good to me. I like the more verbose explanation. The only thing I was wondering is if we should have an issue that we immediately postpone on the Apache fix and add a @todo. But not sure if we create core issues that are postponed on 3rd party projects. So RTBC
Comment #7
Wim LeersWe could create such an issue, but it's likely to linger forever. This has been a default for many, many, many years. It's unlikely to change. So best to just document it.
Comment #9
Wim LeersRandom fail in
Drupal\Tests\outside_in\FunctionalJavascript\OutsideInBlockFormTest
.Comment #10
cilefen CreditAttribution: cilefen commentedThis is too bad about PHP's behavior.
Wim, there is a typo in the comment. There is not a verb in "In the future."
Comment #11
Wim LeersFixed comment.
Comment #12
cilefen CreditAttribution: cilefen commentedI think that is the wrong interdiff ;-)
Comment #13
Wim LeersIt is. But then again it's pretty silly for a patch this small :) So I hope it's okay like this?
Comment #14
cilefen CreditAttribution: cilefen commentedI tried pinging you on IRC that I wasn't asking you to waste your time on the diff. I'll commit this doc improvement in a bit.
Comment #17
cilefen CreditAttribution: cilefen commentedCommitted c1ec091 and pushed to 8.4.x. Cherry-picked as ba41feb and pushed to 8.3.x. Thanks!