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

Fix json-encoding when JSON_THROW_ON_ERROR is used #31869

Merged
merged 1 commit into from Jun 5, 2019

Conversation

nicolas-grekas
Copy link
Member

Q A
Branch? 3.4
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets -
License MIT
Doc PR -

As hinted in #31860 by @lt

@fabpot
Copy link
Member

fabpot commented Jun 5, 2019

Thank you @nicolas-grekas.

@fabpot fabpot merged commit d18f42c into symfony:3.4 Jun 5, 2019
fabpot added a commit that referenced this pull request Jun 5, 2019
…s-grekas)

This PR was merged into the 3.4 branch.

Discussion
----------

Fix json-encoding when JSON_THROW_ON_ERROR is used

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | -
| License       | MIT
| Doc PR        | -

As hinted in #31860 by @lt

Commits
-------

d18f42c Fix json-encoding when JSON_THROW_ON_ERROR is used
fabpot added a commit that referenced this pull request Jun 5, 2019
…nicolas-grekas)

This PR was merged into the 4.3 branch.

Discussion
----------

[HttpClient] revert bad logic around JSON_THROW_ON_ERROR

| Q             | A
| ------------- | ---
| Branch?       | 4.3
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | -
| License       | MIT
| Doc PR        | -

Same as #31869, reverts #31861

Commits
-------

846116e [HttpClient] revert bad logic around JSON_THROW_ON_ERROR
@nicolas-grekas nicolas-grekas deleted the fix-json-enc branch June 5, 2019 14:41
@fabpot fabpot mentioned this pull request Jun 6, 2019
This was referenced Jun 26, 2019
nicolas-grekas added a commit that referenced this pull request Jun 28, 2019
This PR was merged into the 3.4 branch.

Discussion
----------

Catch JsonException and rethrow in JsonEncode

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | adjustment to implementation of previous PRs for issue #31447
| License       | MIT
| Doc PR        | not applicable

PR #31860 provided handling of PHP  7.3 `JSON_THROW_ON_ERROR` behavior in the various `JsonEncode` and related classes/methods.

PR #31869 adjusted that. In particular, it adjusted ` src/Symfony/Component/Serializer/Encoder/JsonDecode.php` so that it catches any `JsonException` and re-throws it as `NotEncodableValueException`. That preserves the previous behavior of `JsonDecode:decode` - it always throws `NotEncodableValueException` when something goes wrong.

IMO `JsonEncode:encode` needs the same logic. At the moment, if a caller specifies `JSON_THROW_ON_ERROR` then the method can throw `JsonException`, but actually the "standard" for `JsonEncode:encode` is that it throws `NotEncodableValueException`

Adjust `JsonEncode:encode` to catch `JsonException` and rethrow it as `NotEncodableValueException`

Commits
-------

9c76790 Catch JsonException and rethrow in JsonEncode
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants