Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[8.x] fix JsonResponse::fromJasonString() double encoding string #37076

Merged
merged 1 commit into from Apr 21, 2021

Conversation

alieffring
Copy link

The Illuminate\Http\JsonResponse class inherits a static fromJsonString() method from the base Symfony\Component\HttpFoundation\JsonResponse class for creating a JsonResponse from an already encoded JSON string, but because of changes to the constructor signature it does not perform as expected, returning a response with the provided string reencoded as a literal string. While you can achieve the desired result like so:

$response = new JsonResponse();
$response->setJson($jsonString);

(and that's probably the preferred way to do it anyway) it seems wrong to just leave the broken function there.

@taylorotwell
Copy link
Member

taylorotwell commented Apr 21, 2021

Why did you need to redefine fromJsonString at all? Its method body is the same as the parent class so couldn't you just let the parent class define it?

@alieffring
Copy link
Author

I hope I'm not missing something obvious, but the parent class method doesn't work because of the extra $options parameter that was added to the laravel implementation's constructor

return new static($data, $status, $headers, true); in the original
vs
return new static($data, $status, $headers, 0, true); in the new one

The test I added doesn't pass without the redefined fromJasonString.

@taylorotwell
Copy link
Member

Ah, gotcha.

@taylorotwell taylorotwell merged commit f356e1b into laravel:8.x Apr 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants