fix(api): utf8mb4 end-to-end + drop xajax-era entity decode in ban/comms flows - #1119
Merged
Conversation
5 tasks
…mms flows The JSON dispatcher now replaces legacy-encoded player/host name bytes with U+FFFD (JSON_INVALID_UTF8_SUBSTITUTE) so a single bad byte from a gameserver rcon response can no longer collapse the per-server admin tile into an empty body (#971). The xajax-era `htmlspecialchars_decode` / `html_entity_decode` calls in `bans.add`, `bans.paste`, `bans.send_message`, `bans.view_community`, `comms.add`, and `comms.paste` are removed; the JSON API delivers raw UTF-8 and the Smarty auto-escape layer (#1087) handles display-time escaping, so decoding again was both a no-op for the common case and actively harmful for literal `&` input (#1060). `compareSanitizedString` becomes straight `===` everywhere it was used. The panel's PDO `DB_CHARSET` default flips from `utf8` (3-byte alias) to `utf8mb4` in `init.php`, `config.php.template`, the installer, the updater, and the PHPStan bootstrap, matching the SourceMod plugin's `SET NAMES utf8mb4` (#1052) and the dev/CI stack's schema charset — eliminating the `Incorrect string value` insert failures on supplementary-plane characters (#765). BanFlowTest gains a multi-byte + angle-bracket round-trip (four data sets: CJK, angle brackets, Cyrillic+emoji, literal `&`), a comms round-trip, and a JSON-encode-substitute guard. AGENTS.md and ARCHITECTURE.md pick up the new anti-patterns / legacy-pattern rows. Closes #1108. Closes #1060. Closes #971. Closes #765. Co-authored-by: Cursor <cursoragent@cursor.com>
rumblefrog
force-pushed
the
fix/issue-1108-utf8mb4-names
branch
from
May 4, 2026 04:13
071a77c to
838d2d7
Compare
- Extract Api::encodeEnvelope() so the U+FFFD substitution is observable in-process; dispatch() delegates to it. Replace the weak testDispatcherSubstitutesInvalidUtf8InResponse (which only re-asserted that json_encode accepts the flag combo) with a focused testEncodeEnvelopeSubstitutesInvalidUtf8 that decodes the encoder output and pins the U+FFFD byte sequence end-to-end, plus a sibling testHandleEmptyBodyProducesValidErrorEnvelope for the dispatcher's outer error path. - Swap BanFlowTest's @dataProvider doc-comment for #[DataProvider] attribute (PHPUnit 11 deprecation; suite's only remaining one). - web/init.php: comment now points at web/updater/data/600.php (the table-charset migrator + config.php rewriter), not upgrade.php (which writes SB_SECRET_KEY and has nothing to do with charsets). - groups.edit handler: drop the xajax-era html_entity_decode from the JSON-decode of overrides / new_override. The JS still JSON.stringify's both, so json_decode stays for now. - servers.send_rcon handler: document the intentional html_entity_decode as a deliberate carve-out from AGENTS.md's no-entity-decode-on-JSON-params rule -- the decoded value is only substring-matched against \`rcon_password\` and never stored or rendered, so the literal-typed-entity bypass attack still gets caught.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #1108.
Closes #1060. Closes #971. Closes #765.
Summary
DB_CHARSET→utf8mb4inweb/init.php,web/config.php.template,web/phpstan-bootstrap.php, and the installer/updater templates so the panel's PDO handle issuesSET NAMES utf8mb4on every connection, matching the SourceMod plugin (sbpp_main.sp,sbpp_comms.sp) and the dev/CI schema charset. The 3-byteutf8alias rejected supplementary-plane bytes and surfaced asQuery_AddBlockInsert failed: Incorrect string valuefor the comms insert.htmlspecialchars_decode/html_entity_decodedance from every API handler that reads a name / reason / chat message from a JSON body (api_bans_add,api_bans_paste,api_bans_kick_player,api_bans_send_message,api_bans_view_community,api_comms_add,api_comms_paste). xajax HTML-encoded payloads in transit; the JSON dispatcher does not. Decoding again collapsed a user-typed literal&into&and, combined with Smarty's auto-escape default (Enable automatic HTML escaping in Smarty templates #1087), produced the reportedCan't get player info for <name>failure path.compareSanitizedString($a, $b)with plain$a === $bat the five call sites and delete the helper fromsystem-functions.php. Both sides of the comparison are raw UTF-8; wrapping them inhtmlspecialcharsfirst was only ever a no-op (or — for invalid UTF-8 — a source of false positives).Api::dispatch()to passJSON_INVALID_UTF8_SUBSTITUTEtojson_encode. A single bad byte in axpaw/php-source-queryhostname/player name no longer collapses the whole response tofalse(the root cause of 1.8.0 Server webpage breaks if players have certain special characters. #971's "bad response" on the per-server tile).testBanRoundTripPreservesUnicodeAndAngleBrackets(four fixtures: CJK, angle brackets, Cyrillic+emoji, literal&), atestCommsAddPreservesUnicodeAndAngleBrackets, and a dispatcher-leveltestDispatcherSubstitutesInvalidUtf8InResponseguard.AGENTS.mdanti-patterns list +ARCHITECTURE.mdlegacy-patterns table pick up the two new rows (no-decode-on-JSON-params, utf8mb4-end-to-end); the Database section inARCHITECTURE.mdcalls out the charset convention explicitly.Acceptance criteria
叮叮当当succeeds via the panel. —testBanRoundTripPreservesUnicodeAndAngleBracketsdata setcjk nickname(web/tests/integration/BanFlowTest.php:124) drivesapi_bans_addand asserts the raw string survives intosb_bans.name.=[BSID]= ethzero <Msg>succeeds and the<Msg>survives in the DB and renders escaped (not literal) in the ban list page. — Same dataprovider,angle bracketsset. Round-trip assertsban['name'] === "=[BSID]= ethzero <Msg>"(web/tests/integration/BanFlowTest.php:129).page_bans.tplrenders{$ban.player|smarty_stripslashes}undersetEscapeHtml(true), so the stored<Msg>comes out as<Msg>in HTML (nonofilterwas involved; see web/themes/default/page_bans.tpl:90,131).api_servers_host_playerspasses player names verbatim into the JSON envelope;Api::dispatchnow encodes withJSON_INVALID_UTF8_SUBSTITUTE(web/includes/Api.php:192), so a malformed or legacy-encoded byte sequence no longer produces an empty body.testDispatcherSubstitutesInvalidUtf8InResponse(web/tests/integration/BanFlowTest.php:177) guards the encoder behaviour.SET NAMES utf8mb4(game/addons/sourcemod/scripting/sbpp_main.sp:1098, sbpp_comms.sp:1384). The panel side now aligns:DB_CHARSETdefaults toutf8mb4inweb/init.php:110,web/config.php.template:39, andweb/phpstan-bootstrap.php:32;sb_bans.name,sb_bans.reason,sb_comms.name, andsb_comms.reasonpick upcharacter set utf8mb4from the{charset}placeholder inweb/install/includes/sql/struc.sql.testCommsAddPreservesUnicodeAndAngleBrackets(web/tests/integration/BanFlowTest.php:148) asserts叮叮当当 <foo>round-trips throughapi_comms_addinto both gag and mute rows.web/tests/integration/BanFlowTest.php(or a sibling) asserts the round-trip and lands green in CI. — See the three new tests above (web/tests/integration/BanFlowTest.php:67–183). All 43 PHPUnit cases pass locally.Test plan
./sbpp.sh phpstan—[OK] No errors(level 5, 168 files analysed, dba enabled against the live MariaDB)../sbpp.sh test— 43 tests / 124 assertions, all green. New cases:Ban round trip preserves unicode and angle brackets×4,Comms add preserves unicode and angle brackets,Dispatcher substitutes invalid utf 8 in response../sbpp.sh ts-check— silent pass (tsc --noEmit --checkJsonweb/scripts/)../sbpp.sh composer api-contract— regeneratesweb/scripts/api-contract.js;git diffclean (no handler signatures, perm masks, or registry entries changed).Made with Cursor