Skip to content

GH-55497: [C++] Avoid reusing simdjson values after failed get() - #55498

Open
wgtmac wants to merge 1 commit into
apache:mainfrom
wgtmac:fix_simdjson_ub
Open

wgtmac wants to merge 1 commit into
apache:mainfrom
wgtmac:fix_simdjson_ub

Conversation

@wgtmac

@wgtmac wgtmac commented Nov 11, 2026 •

Copy link
Copy Markdown
Member

Rationale for this change

Fix UB in StructConverter::AppendValue(), which reused a simdjson ondemand value after get(array) failed.

See #55497.

What changes are included in this PR?

  • Determine the JSON type before calling get() in BooleanConverter, TimestampConverter, and StructConverter.
  • Refactor GetJsonAs() and IsJsonNull() to avoid accessing a value after a failed typed access.

Are these changes tested?

Added a case matching the nested struct/object notation from the issue.

Are there any user-facing changes?

No.

Was AI used for this PR?

PR code and description written by:

  • Human
  • AI

Reviewed before submission by:

  • Human
  • AI
  • Not reviewed

Copilot AI balanced review requested due to automatic review settings November 11, 2026 09:24

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota. Try requesting a review at a lower effort level.

@wgtmac

wgtmac commented Nov 11, 2026

Copy link
Copy Markdown
Member Author

Our internal integration crashed due to this UB. I've proposed a fix, please take a look when you have time. Thanks! @kou @pitrou @Reranko05

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.

2 participants