Skip to content

Fix Body::text() and Body::json() to handle BOMs#5621

Merged
anonrig merged 3 commits intomainfrom
yagiz/fix-wpt-tests-text-json
Dec 18, 2025
Merged

Fix Body::text() and Body::json() to handle BOMs#5621
anonrig merged 3 commits intomainfrom
yagiz/fix-wpt-tests-text-json

Conversation

@anonrig
Copy link
Copy Markdown
Member

@anonrig anonrig commented Dec 1, 2025

Fixes #4712
Fixes #4977

@anonrig anonrig requested review from a team as code owners December 1, 2025 19:00
@anonrig anonrig requested review from jasnell and npaun December 1, 2025 19:15
@npaun
Copy link
Copy Markdown
Member

npaun commented Dec 1, 2025

This one is technically observable, isn't it? Like what if I had crappy code that expected the BOM there

@codspeed-hq

This comment was marked as outdated.

@anonrig anonrig changed the title Fix Body::text() to handle BOMs Fix Body::text() and Body::json() to handle BOMs Dec 1, 2025
@anonrig anonrig force-pushed the yagiz/fix-wpt-tests-text-json branch from 560fc65 to d475224 Compare December 1, 2025 20:48
@anonrig anonrig requested review from jasnell, kentonv and npaun December 1, 2025 20:48
@anonrig anonrig force-pushed the yagiz/fix-wpt-tests-text-json branch from d475224 to dc9792f Compare December 1, 2025 21:14
@anonrig anonrig requested a review from jasnell December 1, 2025 21:14
@anonrig anonrig requested a review from jasnell December 3, 2025 22:23
@anonrig anonrig enabled auto-merge (squash) December 3, 2025 22:29
@anonrig anonrig closed this Dec 10, 2025
auto-merge was automatically disabled December 10, 2025 02:51

Pull request was closed

@jasnell jasnell reopened this Dec 10, 2025
@jasnell
Copy link
Copy Markdown
Collaborator

jasnell commented Dec 10, 2025

Re-opening this. The change is necessary to improve correctness and spec compliance.

changes:

  • Changed the compat flag. Added a new one specific to this with an implied by pedanticWpt
  • Changed the public API to remove the option. It is still there internally in streams but it doesn't surface all the way out to http.c++, for instance
  • Fixed up one minor pointer arithmetic bit to rely more on the safer kj apis
  • Other minor tweaks to ensure tests pass.

@jasnell jasnell force-pushed the yagiz/fix-wpt-tests-text-json branch from 14748e5 to 3e89a25 Compare December 10, 2025 20:47
@jasnell jasnell requested a review from npaun December 10, 2025 20:48
@jasnell
Copy link
Copy Markdown
Collaborator

jasnell commented Dec 10, 2025

@npaun and @kentonv ... if either or both of you could review this again, I'd appreciate it. I can no longer do the sign off since it includes my own changes.

@jasnell jasnell force-pushed the yagiz/fix-wpt-tests-text-json branch from 3e89a25 to 55ce052 Compare December 17, 2025 22:42
@anonrig anonrig enabled auto-merge (squash) December 18, 2025 21:21
@anonrig anonrig merged commit d6a2740 into main Dec 18, 2025
30 of 33 checks passed
@anonrig anonrig deleted the yagiz/fix-wpt-tests-text-json branch December 18, 2025 21:29
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.

Investigate BOM handling during JSON parsing Investigate our handling of the UTF BOM (byte order mark)

4 participants