Skip to content

Fix corrupted JSON caused by goccy library#37214

Merged
wxiaoguang merged 6 commits intogo-gitea:mainfrom
wxiaoguang:fix-goccy-json
Apr 14, 2026
Merged

Fix corrupted JSON caused by goccy library#37214
wxiaoguang merged 6 commits intogo-gitea:mainfrom
wxiaoguang:fix-goccy-json

Conversation

@wxiaoguang
Copy link
Copy Markdown
Contributor

Fix #37211

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Apr 14, 2026
@wxiaoguang
Copy link
Copy Markdown
Contributor Author

This PR only removes the goccy library from our code, just use the official library. No need to write test for the 3rd library's bug.

@wxiaoguang wxiaoguang added the backport/v1.26 This PR should be backported to Gitea 1.26 label Apr 14, 2026
@silverwind
Copy link
Copy Markdown
Member

silverwind commented Apr 14, 2026

BTW could we switch over to stdlib v2 once it's stable? I think we should drop all non-stdlib json libs eventually.

@wxiaoguang
Copy link
Copy Markdown
Contributor Author

BTW could we switch over to stdlib v2 once it's stable? I think we should drop all non-stdlib json libs eventually.

We have been using v2 for a long time. It is enabled by default.

This PR fixes the goccy in "chi-binding" package.

@wxiaoguang
Copy link
Copy Markdown
Contributor Author

I think we should drop all non-stdlib json libs eventually.

But you can't control 3rd libraries:

image

@silverwind
Copy link
Copy Markdown
Member

Right, we can't control indirect deps. Just wondering how this bug came to be.

@wxiaoguang
Copy link
Copy Markdown
Contributor Author

Right, we can't control indirect deps. Just wondering how this bug came to be.

Actually, we can write our "goccy-fake" package, and use "replace" in go.mod to force our package to be used.

The bug is simply a bug, I have provided reproducible code in the issue.

@silverwind
Copy link
Copy Markdown
Member

silverwind commented Apr 14, 2026

Actually, we can write our "goccy-fake" package, and use "replace" in go.mod to force our package to be used.

That might be dangerous because those dependencies could depend on things like goccy-specific errors or behaviours. Best would be to switch them to stdlib directly or find better dependencies.

Copy link
Copy Markdown
Member

@silverwind silverwind left a comment

Choose a reason for hiding this comment

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

Maybe add a test, if simple.

@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Apr 14, 2026
@wxiaoguang
Copy link
Copy Markdown
Contributor Author

Maybe add a test, if simple.

Maybe read the comment ?

#37214 (comment)

image

@wxiaoguang
Copy link
Copy Markdown
Contributor Author

Maybe add a test, if simple.

Maybe read the comment ?

Just curious, why you seldom read code or comments? For most questions you asked, I have explained ahead, just like #37207 (comment) and more.

If you can read before ask, you can save the time of us all, right?

@silverwind
Copy link
Copy Markdown
Member

silverwind commented Apr 14, 2026

Is the bug not observable in first-party code/tests?

@wxiaoguang
Copy link
Copy Markdown
Contributor Author

Is the bug not obersable in first-party code/tests?

The buggy library has been removed, why you want to test a non-existing bug?

@silverwind
Copy link
Copy Markdown
Member

If the test change in tests/integration/api_issue_reaction_test.go covers it, it's fine.

@wxiaoguang
Copy link
Copy Markdown
Contributor Author

wxiaoguang commented Apr 14, 2026

If the test change in tests/integration/api_issue_reaction_test.go covers it, it's fine.

No, don't waste time on the removed 3rd library.

tests/integration/api_issue_reaction_test.go is another legacy problem, just wrong code & wrong logic, no relation to this.

@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Apr 14, 2026
@wxiaoguang wxiaoguang enabled auto-merge (squash) April 14, 2026 12:32
@wxiaoguang wxiaoguang disabled auto-merge April 14, 2026 13:13
@wxiaoguang
Copy link
Copy Markdown
Contributor Author

Found more bugs. Fixed in 5f65a33

Added backward compatibility logic, so this PR isn't "breaking".

@wxiaoguang wxiaoguang enabled auto-merge (squash) April 14, 2026 13:30
@wxiaoguang wxiaoguang merged commit b9961e1 into go-gitea:main Apr 14, 2026
26 checks passed
@GiteaBot GiteaBot added this to the 1.27.0 milestone Apr 14, 2026
@wxiaoguang wxiaoguang deleted the fix-goccy-json branch April 14, 2026 14:00
@GiteaBot
Copy link
Copy Markdown
Collaborator

I was unable to create a backport for 1.26. @wxiaoguang, please send one manually. 🍵

go run ./contrib/backport 37214
...  // fix git conflicts if any
go run ./contrib/backport --continue

@GiteaBot GiteaBot added the backport/manual No power to the bots! Create your backport yourself! label Apr 14, 2026
wxiaoguang added a commit to wxiaoguang/gitea that referenced this pull request Apr 14, 2026
@wxiaoguang
Copy link
Copy Markdown
Contributor Author

-> Fix corrupted JSON caused by goccy library (#37214) #37220

@wxiaoguang wxiaoguang added the backport/done All backports for this PR have been created label Apr 14, 2026
silverwind pushed a commit that referenced this pull request Apr 14, 2026
Backport #37214

The only conflict is go.mod

Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>
zjjhot added a commit to zjjhot/gitea that referenced this pull request Apr 17, 2026
* main:
  Replace `dropzone` with `@deltablot/dropzone` (go-gitea#37237)
  Add `ExternalIDClaim` option for OAuth2 OIDC auth source (go-gitea#37229)
  Remove error returns from crypto random helpers and callers (go-gitea#37240)
  Use Content-Security-Policy: script nonce (go-gitea#37232)
  Remove htmx (go-gitea#37224)
  Refactor "htmx" to "fetch action" (go-gitea#37208)
  Fix UI regression (go-gitea#37218)
  Fix corrupted JSON caused by goccy library (go-gitea#37214)
  Add test for "fetch redirect", add CSS value validation for external render (go-gitea#37207)
  Fix incorrect concurrency check (go-gitea#37205)
  refactor: simplify ParseCatFileTreeLine and catBatchParseTreeEntries (go-gitea#37210)
  Update go js py dependencies (go-gitea#37204)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport/done All backports for this PR have been created backport/manual No power to the bots! Create your backport yourself! backport/v1.26 This PR should be backported to Gitea 1.26 lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. type/bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

PATCH(JSON) corrupts long UTF-8 Japanese text for issues and issue comments on v1.25.5

4 participants