Update JsonEncoder.php

[AntonioCS]

Added JSON_INVALID_UTF8_IGNORE to $jsonEncodeOptions

Q	A
Branch?	main for features / current stable version branch for bug fixes
Tickets	#...
License	MIT
Doc PR	api-platform/docs#...

[dunglas]

LGTM!

Could you rebase against the 2.6 branch as it's a bug fix and add a unit test to prevent any future regression please?

[AntonioCS]

I had a few issues rebasing, so I guess it would be ok to start with 2.6?
Also, I'm not sure how to replicate this error, as this is something coming from a doctrine exception that API Platform tries to serialize. I'll try something I guess :)

[dunglas]

Just create a test that serializes a string containing invalid UTF-8 sequences.

By the way, I wonder if it's a good idea to always enable this. Maybe should it be enabled only for the dev environment. Or maybe should would it be better to fix the exception message in Doctrine after all.

I fear that generating JSON documents containing non-valid UTF-8 sequences in prod leads to more harm than good.

[AntonioCS]

Yeah, it was simple, I thought I had to do something with something from doctrine, but I got it.
Well... this being displayed in prod will also cause issues (at least in my case) as it just hides errors

[AntonioCS]

Not sure how to fix this is doctrine, as it's trying to insert binary data in a binary field in the DB

[AntonioCS]

Did this for testing:

    public function testUTF8MalformedHandling()
    {
        $data = ['random_bytes' => random_bytes(16)];

        try {
            $this->assertNotNull($this->encoder->encode($data, 'json'));
        } catch (\Exception $exception) {
            $this->fail('An exception was thrown: ' . $exception->getMessage());
        }
    }
}

With the added flag (JSON_INVALID_UTF8_IGNORE) no exception was triggered.

[dunglas]

Regarding the test, you could use pack() instead to create the string containing invalid UTF-8 sequences. Using the random number will make the test flaky (what if the generated sequence is a valid UTF-8 string)?

But I wonder if this is the best fix possible: api-platform/api-platform#2000 (comment)

[AntonioCS]

I've changed the test and branched from 2.6 but I can branch from main and then cherry pick. Is this still a valid approach? (the fix in the jsonencoder)

[soyuka]

sure, if you have issues just open a new pr no problem

[divine]

JFYI, this can be closed.
@soyuka @dunglas

Thanks!