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

adds JSON_ERROR_NON_BACKED_ENUM to allowed JSON::encode() errors. #1707

Merged
merged 5 commits into from
Feb 27, 2024
Merged

adds JSON_ERROR_NON_BACKED_ENUM to allowed JSON::encode() errors. #1707

merged 5 commits into from
Feb 27, 2024

Conversation

ju1ius
Copy link
Contributor

@ju1ius ju1ius commented Feb 27, 2024

The JSON_ERROR_NON_BACKED_ENUM error code is not yet documented (see php/doc-en#2747), but it is returned when trying to encode encode a non-backed enum to JSON.

Since this error code is semantically equivalent to JSON_ERROR_UNSUPPORTED_TYPE which is already in the allow list, it makes sense to also add it.

For context, I was adding a Monolog record as a Breadcrumd metadata item, which looked like this:

[
    'type' => 'default',
    'category' => 'doctrine',
    'level' => 'debug',
    'timestamp' => 1709047886.0,
    'message' => 'Executing statement: {sql} (parameters: {params}, types: {types})',
    'data' => [
        'sql' => 'SELECT id, key FROM ... WHERE id = ? AND key = ?',
        'params' => [42, 'somekey'],
        'types' => [
            \Doctrine\DBAL\ParameterType::INTEGER,
            \Doctrine\DBAL\ParameterType::STRING,
        ],
    ],
],

The encoding error is triggered by Doctrine\DBAL\ParameterType, which is a unit enum in Doctrine DBAL v4.
Since the JSON_ERROR_NON_BACKED_ENUM is not handled by JSON::encode(), this caused an exception to be thrown in PayloadSerializerInterface::serialize() and as a result the whole event was ignored (nothing was sent to the sentry endpoint).

The `JSON_ERROR_NON_BACKED_ENUM` error code is not yet documented (see php/doc-en#2747), but it is returned when trying to encode encode a non-backed enum to JSON.

Since this error code is semantically equivalent to `JSON_ERROR_UNSUPPORTED_TYPE` which is already in the allow list, it makes sense to also add it.
Copy link
Member

@cleptric cleptric left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a test for this. You can skip it on PHP versions that do not support JSON_ERROR_NON_BACKED_ENUM.

Copy link
Member

@cleptric cleptric left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks

@cleptric cleptric enabled auto-merge (squash) February 27, 2024 19:02
@cleptric cleptric merged commit 6f8cce5 into getsentry:master Feb 27, 2024
31 checks passed
@ju1ius
Copy link
Contributor Author

ju1ius commented Feb 27, 2024

Thanks

You're welcome

@ju1ius ju1ius deleted the patch-1 branch February 27, 2024 19:07
@cleptric
Copy link
Member

cleptric commented Mar 8, 2024

This was released with 4.6.1.

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

2 participants