JSON responses encoded inconsistently: make them all RFC4627-compliant

+++ b/core/modules/serialization/src/Encoder/JsonEncoder.php
@@ -18,6 +20,18 @@ class JsonEncoder extends BaseJsonEncoder implements EncoderInterface, DecoderIn
+    // 15 === JSON_HEX_TAG | JSON_HEX_APOS | JSON_HEX_AMP | JSON_HEX_QUOT
...
+    $json_encoding_options = 15;

Is there a reason we don't just inline this line? This would be more readable, IMHO

Does it makes sense to backport to SF?
I mean to change JsonResponse default serialization

One question though I'd ask in general, shouldn't we strip of HTML out of those errors messages? For me those don't make sense, beside making it harder to read.

\Drupal\Component\Render\PlainTextOutput::renderFromHtml or a potential different implementation, could be used here.

+++ b/core/modules/rest/tests/src/Functional/EntityResource/EntityResourceTestBase.php
@@ -577,11 +573,7 @@ public function testPost() {
+    $this->assertResourceErrorResponse(422, "Unprocessable Entity: validation failed.\nuuid.0.value: <em class=\"placeholder\">UUID</em>: may not be longer than 128 characters.\n", $response);

+++ b/core/modules/rest/tests/src/Functional/EntityResource/User/UserResourceTestBase.php
@@ -163,11 +163,7 @@ public function testPatchDxForSecuritySensitiveBaseFields() {
+    $this->assertResourceErrorResponse(422, "Unprocessable Entity: validation failed.\nmail: Your current password is missing or incorrect; it's required to change the <em class=\"placeholder\">Email</em>.\n", $response);

Re #13 and #14 are we going strip the html out of these messages? Maybe that is separate issue?

Other than I look back through the issue and it looks good.

+++ b/core/modules/rest/tests/src/Unit/EventSubscriber/ResourceResponseSubscriberTest.php
@@ -48,7 +48,7 @@ public function testSerialization($data, $expected_response = FALSE) {
-    $this->assertEquals($expected_response !== FALSE ? $expected_response : json_encode($data), $event->getResponse()->getContent());
+    $this->assertEquals($expected_response !== FALSE ? $expected_response : json_encode($data, JSON_HEX_TAG | JSON_HEX_APOS | JSON_HEX_AMP | JSON_HEX_QUOT), $event->getResponse()->getContent());

This line could use \Drupal\Component\Serialization\Json::encode directly. I'm not sure its a good idea, but its the same code

I'm a bit confused by this change. Isn't it just a special case of #2739617: Make it easier to write on4xx() exception subscribers basically?

I think #35 demonstrates it is an encoding issue and seems it is important to get this in so clients have consistent encoding on errors.

Regarding stripping html as out of scope that seems right was checking we hadn't forgot something.

Looks good to me.

Also agree this looks good!

1. +++ b/core/modules/hal/src/Encoder/JsonEncoder.php
   @@ -2,34 +2,20 @@
   -  public function supportsEncoding($format) {
   

   Nice, I noticed these were potentially redundant the other day (i.e. we could extend our own encoder like you are here).

2. +++ b/core/modules/serialization/src/Encoder/JsonEncoder.php
   @@ -21,6 +23,18 @@ class JsonEncoder extends BaseJsonEncoder implements EncoderInterface, DecoderIn
   +  public function __construct(JsonEncode $encodingImpl = null, JsonDecode $decodingImpl = null) {
   

   This looks good to me, and probably the simplest way. We could have injected a new argument for the JsonEncode class instead with these parameters. I don't think it buys us much/anything though.

+++ b/core/modules/serialization/src/Encoder/JsonEncoder.php
@@ -21,6 +23,18 @@ class JsonEncoder extends BaseJsonEncoder implements EncoderInterface, DecoderIn
+  public function __construct(JsonEncode $encodingImpl = null, JsonDecode $decodingImpl = null) {
+    // Encode <, >, ', &, and " for RFC4627-compliant JSON, which may also be
+    // embedded into HTML.
+    // @see \Symfony\Component\HttpFoundation\JsonResponse
+    $json_encoding_options = JSON_HEX_TAG | JSON_HEX_APOS | JSON_HEX_AMP | JSON_HEX_QUOT;
+    $this->encodingImpl = $encodingImpl ?: new JsonEncode($json_encoding_options);
+    $this->decodingImpl = $decodingImpl ?: new JsonDecode(true);
+  }

I've been mulling the behaviour change for a while trying to think if there are any possible BC impacts. I can't think of any. Am I right? Anything that decodes the JSON encoded with these options is just going to see the same data - only the JSON will be slightly different right?

Committed badbcd3 and pushed to 8.3.x. Thanks!

FILE: ...ev/drupal/core/modules/serialization/src/Encoder/JsonEncoder.php
----------------------------------------------------------------------
FOUND 3 ERRORS AFFECTING 2 LINES
----------------------------------------------------------------------
 26 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
    |       |     "NULL" but found "null"
 26 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
    |       |     "NULL" but found "null"
 32 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
    |       |     "TRUE" but found "true"
----------------------------------------------------------------------
PHPCBF CAN FIX THE 3 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------

Fixed on commit.

I concerned whether or not this was worth a change record but I really don't think so since all JSON parsers should support this encoding.