Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #20487 +/- ##
============================================
+ Coverage 63.49% 64.20% +0.71%
- Complexity 16060 16099 +39
============================================
Files 676 678 +2
Lines 59891 57852 -2039
============================================
- Hits 38028 37145 -883
+ Misses 21863 20707 -1156
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Fixes phpmyadmin#20128 Signed-off-by: Matt Van Horn <455140+mvanhorn@users.noreply.github.com>
8ab2e30 to
e5e883a
Compare
|
The PHP 8.5 job wasn't failing an assertion — it was hitting Fixed in 1e73b67:
Two things worth a reviewer's eye. Dropping |
The SJIS-win export path normalised to CP932 and passed it to iconv with the caller's //TRANSLIT or //IGNORE suffix. On glibc that combination can fail to consume any input for an unconvertible sequence, so the export loop span until PHP hit max_execution_time -- the CI job died at the 300s limit rather than on an assertion. Prefer mbstring when the build lists SJIS-win natively, since mbstring implements Windows CP932 directly. When falling back to iconv for CP932, drop the //TRANSLIT and //IGNORE suffixes so a failed conversion returns false instead of spinning. Either way a false result now yields the original string rather than propagating false. The mbstring capability lookup is cached because convertString runs per exported line. Signed-off-by: Matt Van Horn <455140+mvanhorn@users.noreply.github.com>
1e73b67 to
86ff447
Compare
PHPStan reported the ignore pattern for Encoding::convertString() returning string|false as unmatched, since this branch no longer produces that error. Verified locally: phpstan analyse reports no errors. Signed-off-by: Matt Van Horn <455140+mvanhorn@users.noreply.github.com>
0c8eb6e to
9ffc95a
Compare
1 participant
Description
First confirm the alias diagnosis on an affected iconv backend by comparing conversion to
SJIS-win,SJIS-win//TRANSLIT,CP932, andCP932//TRANSLIT, recording PHP and iconv implementation versions; the expected distinction is support for the charset name, not a blanket incompatibility with transliteration. Insrc/Encoding.php, normalize the exactSJIS-winname case-insensitively toCP932for both source and destination only when the selected engine is iconv, immediately before the existing conversion dispatch. Preserve the equality fast path, configured iconv suffix, mbstring behavior, and engine-none behavior, and do not substitute plainSJISor suppress conversion errors.Custom SQL export with
SJIS-winselected fails with an iconv wrong-encoding error on the reporter's PHP 8.5.3 installation, affecting both 5.2.4-dev and 6.0.0-dev. The issue supplies a MyISAM table and a Japaneseテストrow as a concrete reproduction. On the current master checkout,Encoding::convertString()passes the configured charset straight to iconv and appends the default//TRANSLITsuffix;SJIS-winis offered by the default charset configuration. The thread suggests removing the suffix, but does not establish that transliteration is the cause; acceptance of charset aliases varies between iconv implementations.Fixes #20128