Skip to content

fix(libSceCesCs): report the length of a rejected MBCS character as the console does - #10722

Merged
oneandonlydean merged 1 commit into
boykopovar:mainfrom
rooklz:fix/cescs-error-length
Oct 25, 2026
Merged

oneandonlydean merged 1 commit into
boykopovar:mainfrom
rooklz:fix/cescs-error-length

Conversation

@rooklz

@rooklz rooklz commented Oct 24, 2026 •

Copy link
Copy Markdown
Contributor

What

A rejected multibyte character left every output alone. On a PS5 it reports its length and clears the character. I measured it with homebrew payloads that load the console's libSceCesCs.sprx, over every one- and two-byte sequence of all 15 MBCS profiles with sceCesMbcToUtf32, every 0x8F-led three-byte and every 0x81 0x30-led three- and four-byte sequence, and the same two-byte sweep through sceCesMbcTo{Ucs2,Utf8,Utf16} and each encoding's own To{Ucs2,Utf8,Utf16,Utf32} for eight profiles. Every form agreed with sceCesMbcToUtf32 on the result and the length for all 65792 inputs. On an error the console:

  • reports the length: the bytes a truncated sequence needs (0x805C0011, 4 for 0x81 0x30 in GB 18030), one byte for a bad lead, the bytes before a NUL found after the lead (checked byte by byte, before more bytes are asked for), and the whole sequence for a bad or unassigned trail;
  • clears the character: a UCS-2 or UTF-32 output becomes 0, and the UTF-8 and UTF-16 forms report 0 units and leave their buffer alone.

The decoders now set that length on each error path, and the single-character wrappers store it with the cleared output. Argument errors keep the outputs untouched. sceCesEucJpToUcs2 and sceCesEucJpToUtf8 are left out: #10184 measured those two on a PS5 and changes them, with the same length rule.

Tested

Merge order: this lands before #10718, which is rebased onto it (both change the Shift-JIS and EUC-JP decode paths in Export.cpp and the same GuestCesJapanese.cpp checks).

Checklist

  • Based on current main; no other open PR implements the same functions
  • One topic per PR; follow-ups go in a new PR
  • No comments in code except technical debt
  • Unimplemented paths throw (NotImplemented_nid_no_patch); silent stubs are listed under Silent stubs in their library's technical debt file
  • No notes, investigation .md files or images added to the repository (attach them to this PR)
  • New third-party code is a submodule built from source
  • Generic behaviour, not specific to one title
  • Depends on: none; fix(libSceCesCs): convert single EUC-JP characters as measured on a PS5 #10184 changes the EUC-JP scalar forms with the same rule
  • Fixes: none
  • Unblocks: none
  • GPUs tested: n/a
  • AI-assisted: yes

@rooklz
rooklz force-pushed the fix/cescs-error-length branch from 2f81a79 to 67e4ffb Compare October 24, 2026 19:57
@github-actions

github-actions Bot commented Oct 24, 2026 •

Copy link
Copy Markdown

Other open pull requests touch the same code. Merging one will break the other, or both implement the same thing. If this one builds on them, list them in Depends on.

Pull request Conflicting files (lines on main) Same exports Same new files
PR 10056 docs/dev/debt/libSceCesCs.md L22-24

Some comparisons could not be completed because a pull request conflicts with the base branch. Review these shared files manually before concluding that there is no overlap.

Pull request Shared files not compared
PR 9912 core/libs/prx/libSceCesCs/Export.cpp
docs/dev/debt/libSceCesCs.md

@MarcZX10 MarcZX10 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.

Reviewed the decoder error paths, wrapper output handling, test changes, and debt update. The new helpers distinguish invalid lead, bad/NUL trail, truncated sequence, and unassigned sequence lengths; the wrappers update only the outputs the measured behavior changes. The tests include console-captured sequences and preserve the separate EUC-JP output behavior called out in the debt entry. Hosted conventions/report checks pass; the PR also reports its focused macOS suite and a Linux guest_ces run on the same tree. I could not dispatch the manual full build from this account (GitHub returned HTTP 403). I found no blocking issue.

@MarcZX10 MarcZX10 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.

Reviewed the full diff at 67e4ffb. The changes propagate measured failure lengths from the decoders and clear only the decoded character output while preserving argument-error behavior. The added MBCS cases cover truncation, bad lead/trail, embedded NUL, unassigned values and GB18030 four-byte lengths across all eight conversion forms; the PR also reports exhaustive console sweeps across 65,792 inputs/profile combinations and passing hosted Linux CES tests. Conventions, labels and eligibility passed. Product Linux/Windows and other focused workflow jobs were skipped under current repository policy; the submitted report separately records a macOS harness run and the Linux runner run.

@MarcZX10

Copy link
Copy Markdown
Contributor

main has advanced since this approval and the current PR head is now DIRTY. git merge-tree upstream/main refs/remotes/upstream/pr/10722 reports a content conflict in core/libs/tests/GuestCesMbc.cpp, which this PR changes to add the rejected-sequence length cases. Please rebase/resolve against the current canonical CES headers while preserving the new length assertions, then rerun the focused MBCS test and conventions check on the integrated head. I have not modified your branch.

@rooklz
rooklz force-pushed the fix/cescs-error-length branch from 67e4ffb to 4785322 Compare October 25, 2026 00:29
@github-actions github-actions Bot removed the needs rebase Conflicts with main, rebase needed label Oct 25, 2026
@rooklz

rooklz commented Oct 25, 2026

Copy link
Copy Markdown
Contributor Author

@MarcZX10 rebased onto current main (head 4785322ed8). The conflict was the old extern "C" block in GuestCesMbc.cpp, which #10706 replaced with the export headers. I took main's side there; the three profile initializers the length cases use are declared in Api.hpp, and the rejected-sequence length assertions are unchanged. On the integrated head, in a Linux x86-64 container (Ubuntu 24.04, GCC 13.3, Release), all 14 guest_ces* tests pass, and check_conventions.py passes.

@github-actions github-actions Bot added the needs rebase Conflicts with main, rebase needed label Oct 25, 2026
@rooklz
rooklz force-pushed the fix/cescs-error-length branch from 4785322 to 155abef Compare October 25, 2026 01:47
@github-actions github-actions Bot removed the needs rebase Conflicts with main, rebase needed label Oct 25, 2026

@MarcZX10 MarcZX10 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.

Reviewed updated exact head 155abef132c3b7b9a4361d3160d7a39c183a031c. The rebase replaced the old local export declarations with the canonical CES headers while preserving the rejected-sequence length assertions. The current tests still cover invalid lead/trail, truncation, embedded NUL, and unassigned cases across the eight MBCS output forms; the author reports all 14 guest_ces* tests and conventions passing on this head. Hosted conventions, eligibility, labels and report pass; product build jobs are skipped. I did not independently rerun the CES suite on this head. No blocking issue remains in the reviewed integration.

@MarcZX10

Copy link
Copy Markdown
Contributor

A follow-up on the overlap checklist: your overlap report names #10718, and both PRs change shared CES decoder/encoder paths in Export.cpp. CONTRIBUTING.md says to agree on the merge order and declare it with Depends on. Could you coordinate with #10718 and update both checklists to record the agreed order? I have not modified your branch.

@rooklz

rooklz commented Oct 25, 2026

Copy link
Copy Markdown
Contributor Author

@MarcZX10 the order with #10718 is now recorded: this PR lands first, and #10718 is rebased onto it with Depends on: #10722. This branch is unchanged (still 155abef132).

@oneandonlydean oneandonlydean self-assigned this Oct 25, 2026
@github-actions github-actions Bot added the needs rebase Conflicts with main, rebase needed label Oct 25, 2026
@rooklz
rooklz force-pushed the fix/cescs-error-length branch from 155abef to 8e08731 Compare October 25, 2026 13:10
@github-actions github-actions Bot removed the needs rebase Conflicts with main, rebase needed label Oct 25, 2026
@oneandonlydean
oneandonlydean merged commit 2a16d79 into boykopovar:main Oct 25, 2026
10 checks passed
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.

3 participants