Drupal\Core\Routing\ContentTypeHeaderMatcher checks $request->headers->has('Content-type') to decide which error message to show if the content type header does not match any route.

Drupal\Tests\rest\Functional\EntityResource\EntityResourceTestBase::testPost() (and all test classes that inherit this) verify this behavior by sending a request without a content-type header.

However, some webserver environments (this occurs in nginx with fastcgi) appear to default this header to an empty value if the client does not send it.

In this case, has(...) will return true even though the header is empty, and the wrong exception will be thrown.

The solution would be to not call has(), but instead check if the content-type header is empty.

CommentFileSizeAuthor
#4 drupal-rest-2860913-4.patch785 bytescburschka
#2 drupal-rest-2860913-2.patch754 bytescburschka
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

cburschka created an issue. See original summary.

cburschka’s picture

cburschka
they
CreditAttribution: cburschka at PwC's Experience Center commented
Version: 8.3.x-dev » 8.4.x-dev
Status: Active » Needs review
FileSize
drupal-rest-2860913-2.patch754 bytes

Status: Needs review » Needs work

The last submitted patch, 2: drupal-rest-2860913-2.patch, failed testing.

cburschka’s picture

cburschka
they
CreditAttribution: cburschka at PwC's Experience Center commented
Status: Needs work » Needs review
FileSize
drupal-rest-2860913-4.patch785 bytes

Okay, it looks like getContentType() has extra logic beyond checking the header. Then replace the has() call with a get() call instead.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.0-alpha1 will be released the week of July 31, 2017, which means new developments and disruptive changes should now be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

cburschka’s picture

cburschka
they
CreditAttribution: cburschka at PwC's Experience Center commented

Retesting.

cburschka’s picture

cburschka
they
CreditAttribution: cburschka at PwC's Experience Center commented
Version: 8.5.x-dev » 8.4.x-dev

Also, I rather suspect this is still okay for 8.4.x.

cburschka’s picture

cburschka
they
CreditAttribution: cburschka at PwC's Experience Center commented
cburschka’s picture

cburschka
they
CreditAttribution: cburschka at PwC's Experience Center commented
Title: ContentTypeHeaderMatcher returns wrong exception message when Content-Type header is pre-filled. » Use "missing content-type" exception message for empty content-type header.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.4 was released on January 3, 2018 and is the final full bugfix release for the Drupal 8.4.x series. Drupal 8.4.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.5.0 on March 7, 2018. (Drupal 8.5.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.5.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.6 was released on August 1, 2018 and is the final bugfix release for the Drupal 8.5.x series. Drupal 8.5.x will not receive any further development aside from security fixes. Sites should prepare to update to 8.6.0 on September 5, 2018. (Drupal 8.6.0-rc1 is available for testing.)

Bug reports should be targeted against the 8.6.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

mr.baileys’s picture

mr.baileys
Primary language Dutch
Location 🇧🇪 (Ghent)
CreditAttribution: mr.baileys at SQLI - Belgium commented
Version: 8.6.x-dev » 8.9.x-dev
Status: Needs review » Needs work
Issue tags: +Needs tests

I can confirm this issue after spending a debugging a test that was failing on my local environment (ddev, nginx+fastcgi). The test kept failing with:

There was 1 failure:

1) Drupal\Tests\datetime\Functional\EntityResource\EntityTest\EntityTestDateonlyTest::testPatch
Failed asserting that two strings are identical.
--- Expected
+++ Actual
@@ @@
-'{"message":"No \u0022Content-Type\u0022 request header specified"}'
+'{"message":"No route found that matches \u0022Content-Type: \u0022"}'

/var/www/html/core/modules/rest/tests/src/Functional/ResourceTestBase.php:393
/var/www/html/core/modules/rest/tests/src/Functional/ResourceTestBase.php:455
/var/www/html/core/modules/rest/tests/src/Functional/EntityResource/EntityResourceTestBase.php:1105

After applying the patch in #4 (patch still applies cleanly against 8.9.x-dev), the test came back green.

The fix looks sane to me, although:

  1. We should add expand the test in \Drupal\Tests\Core\Routing\ContentTypeHeaderMatcherTest::testContentTypeRequestHeaderMissing so it includes a request with an empty (but present) Content-Type header;
  2. -    if (!$request->headers->has('Content-Type')) {
+    if (empty($request->headers->get('Content-Type'))) {
       throw new UnsupportedMediaTypeHttpException('No "Content-Type" request header specified');
     }

    Nitpick, but the exception message should become 'Empty or no "Content-Type" request header specified' or something along those lines?

mr.baileys’s picture

mr.baileys
Primary language Dutch
Location 🇧🇪 (Ghent)
CreditAttribution: mr.baileys at SQLI - Belgium commented
Status: Needs work » Closed (outdated)

This was also reported and fixed in #3090626: ContentTypeHeaderMatcher dx tests broken with some nginx configs