Problem/Motivation

The listener calls $final_response->setCharset($response->getCharset); With symfony 3, this works, because setCharset will take NULL and just use it, but in Symfony 4, $response->setCharset() requires a string, while ->getCharset() can return null. This creates the following error.

Exception: TypeError: Argument 1 passed to Symfony\Component\HttpFoundation\Response::setCharset() must be of the type string, null given, called in /var/www/html/core/modules/jsonapi/src/EventSubscriber/ResourceResponseSubscriber.php on line 176
Symfony\Component\HttpFoundation\Response->setCharset()() (Line: 496)

Proposed resolution

We should only call setCharset if getCharset returns something other than NULL.

Remaining tasks

Do it
Commit it

User interface changes

None

API changes

None

Data model changes

None

Release notes snippet

CommentFileSizeAuthor
#10 3042694-10.drupal.JSONAPI-ResourceResponseSubscriber-can-pass-NULL-toSymfonyComponentHttpFoundationResponsesetCharset.patch1017 bytesmikelutz
#10 interdiff.3042694.8-10.txt870 bytesmikelutz
#8 interdiff.3042694.2-8.txt986 bytesmikelutz
#8 3042694-8.drupal.JSONAPI-ResourceResponseSubscriber-can-pass-NULL-toSymfonyComponentHttpFoundationResponsesetCharset.patch1.01 KBmikelutz
#2 3042694-2.drupal.JSONAPI-ResourceResponseSubscriber-can-pass-NULL-toSymfonyComponentHttpFoundationResponsesetCharset.patch1021 bytesmikelutz

Comments

mikelutz created an issue. See original summary.

mikelutz’s picture

mikelutz
He/Him
Primary language English
Location Michigan, USA
commented
Status: Active » Needs review
StatusFileSize
new 3042694-2.drupal.JSONAPI-ResourceResponseSubscriber-can-pass-NULL-toSymfonyComponentHttpFoundationResponsesetCharset.patch1021 bytes
mikelutz’s picture

mikelutz
He/Him
Primary language English
Location Michigan, USA
commented
Component: base system » jsonapi.module
mikelutz’s picture

mikelutz
He/Him
Primary language English
Location Michigan, USA
commented
Issue summary: View changes
mikelutz’s picture

mikelutz
He/Him
Primary language English
Location Michigan, USA
commented
Parent issue: » #2976394: Allow Symfony 4.4 to be installed in Drupal 8
mikelutz’s picture

mikelutz
He/Him
Primary language English
Location Michigan, USA
commented
Title: JSONAPI ResourceResponseSubscriber can pass NULL toSymfony\Component\HttpFoundation\Response::setCharset() » [Symfony 4] JSONAPI ResourceResponseSubscriber can pass NULL toSymfony\Component\HttpFoundation\Response::setCharset()
jibran’s picture

jibran
he/him
Primary language Urdu
Location Toronto, Canada
commented
Status: Needs review » Needs work 
+++ b/core/modules/jsonapi/src/EventSubscriber/ResourceResponseSubscriber.php
@@ -173,7 +173,9 @@ protected static function flattenResponse(ResourceResponse $response, Request $r
+    if ($response->getCharset()) {
+      $final_response->setCharset($response->getCharset());

Let's not call the method again we can store the result in local var.

mikelutz’s picture

mikelutz
He/Him
Primary language English
Location Michigan, USA
commented
Status: Needs work » Needs review
StatusFileSize
new 3042694-8.drupal.JSONAPI-ResourceResponseSubscriber-can-pass-NULL-toSymfonyComponentHttpFoundationResponsesetCharset.patch1.01 KB
new interdiff.3042694.2-8.txt986 bytes

There ya go.

jibran’s picture

jibran
he/him
Primary language Urdu
Location Toronto, Canada
commented
Status: Needs review » Needs work

Thanks, for addressing the feedback.

+++ b/core/modules/jsonapi/src/EventSubscriber/ResourceResponseSubscriber.php
@@ -173,7 +173,10 @@ protected static function flattenResponse(ResourceResponse $response, Request $r
+    $charset = $response->getCharset()
+    if ($charset) {

These lines can be combined.

mikelutz’s picture

mikelutz
He/Him
Primary language English
Location Michigan, USA
commented
StatusFileSize
new interdiff.3042694.8-10.txt870 bytes
new 3042694-10.drupal.JSONAPI-ResourceResponseSubscriber-can-pass-NULL-toSymfonyComponentHttpFoundationResponsesetCharset.patch1017 bytes

Missed a semicolon anyway, lol. There you go.

mikelutz’s picture

mikelutz
He/Him
Primary language English
Location Michigan, USA
commented
Status: Needs work » Needs review
jibran’s picture

jibran
he/him
Primary language Urdu
Location Toronto, Canada
commented
Status: Needs review » Reviewed & tested by the community

Great!

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Title: [Symfony 4] JSONAPI ResourceResponseSubscriber can pass NULL toSymfony\Component\HttpFoundation\Response::setCharset() » [Symfony 4] JSON:API ResourceResponseSubscriber can pass NULL to Symfony\Component\HttpFoundation\Response::setCharset()
Issue tags: +API-First Initiative

👍 Looks good!

I don't think this needs backporting to the contrib module for use on Drupal 8.5/8.6, since sites on that core minor version will not be updating to Symfony 4.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Version: 8.8.x-dev » 8.7.x-dev
Status: Reviewed & tested by the community » Patch (to be ported)

Committed 0df3be0 and pushed to 8.8.x. Thanks!

Will backport to 8.7.x once the branch is open again.

  • alexpott committed 0df3be0 on 8.8.x 
    Issue #3042694 by mikelutz, jibran: [Symfony 4] JSON:API...
alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Patch (to be ported) » Fixed

  • alexpott committed b681a61 on 8.7.x 
    Issue #3042694 by mikelutz, jibran: [Symfony 4] JSON:API...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented

Ported to the JSON:API contrib module: https://git.drupalcode.org/project/jsonapi/commit/d2bb36d