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

chore: fix potential state corruption in UnmarshalJSON #10705

Closed
wants to merge 1 commit into from

Conversation

brawncode
Copy link

I noticed an issue where t could be modified even if time.Parse failed. This could lead to unexpected behavior when handling invalid timestamps.
The fix ensures that t is only updated after a successful parse.

@brawncode brawncode requested a review from a team as a code owner February 10, 2025 15:49
@lidel lidel requested a review from gammazero February 11, 2025 15:42
@lidel
Copy link
Member

lidel commented Feb 11, 2025

Triage note:

  • unsure if this was by design or not, needs analysis

@lidel lidel added the need/analysis Needs further analysis before proceeding label Feb 11, 2025
@brawncode
Copy link
Author

Triage note:

  • unsure if this was by design or not, needs analysis

This change is necessary to maintain correct Go syntax and prevent potential compilation issues.

@gammazero
Copy link
Contributor

Is there anyplace that the error is handled? I do not see this code used anywhere. If it is unused, then I would rather just remove it.

@gammazero
Copy link
Contributor

Thank you for the contribution. Decided to go with #10708 instead.

@gammazero gammazero closed this Feb 11, 2025
@brawncode
Copy link
Author

LOL.I created a pull request earlier...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
need/analysis Needs further analysis before proceeding
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants