Commit d25e8ea
committed
THRIFT-5759: Remove mbstring.func_overload workaround from PHP library
Client: php
The mbstring.func_overload PHP ini setting was deprecated in PHP 7.2
and removed in PHP 8.0. Since THRIFT-5956 raised the project's minimum
PHP version to 8.1, the TStringFunc abstraction guarding against this
setting has been functionally unreachable in every supported runtime
(ini_get('mbstring.func_overload') always returns false). This removes
the dead code; native strlen()/substr() already return byte counts on
PHP 8.1+, which is what Thrift's binary protocols need.
- Delete Thrift\Factory\TStringFuncFactory, Thrift\StringFunc\{Core,
Mbstring, TStringFunc}, and legacy lib/php/src/TStringUtils.php.
- Replace 30+ TStringFuncFactory::create()->strlen/substr call sites
in lib/Transport/* and lib/Protocol/* with native strlen()/substr().
- Initialize TFramedTransport::$rBuf_ and $wBuf_ to '' (instead of the
default null) and reset $rBuf_ to '' after a frame is consumed, so
the new direct strlen()/substr() calls never receive null and never
trigger the PHP 8.1 "passing null to parameter of type string is
deprecated" warning that the deleted Core wrapper used to mask via
(string)$str casts.
- Drop the corresponding unit tests (TStringFuncFactoryTest, CoreTest,
MbStringTest) and remove the obsolete mbstring.func_overload=0 ini
override from src/ext/thrift_protocol/run-tests.php.
- Add a regression test (StringByteCountTest) that pins the byte-count
contract: TBinaryProtocol/TCompactProtocol round-trip non-ASCII UTF-8
payloads (Cyrillic, Arabic, CJK, Malayalam, emoji, etc.) intact, and
the on-the-wire i32 length prefix matches the byte length, not the
character count. The test also covers TFramedTransport on top of
TMemoryBuffer to exercise the rBuf_ exhaustion path.
- No mb_* function calls remain anywhere in lib/php, test/php, the
tutorial, or the C extension, so drop mbstring from build.yml,
sca.yml, and the focal/jammy/noble Dockerfiles.
- Update lib/php/Makefile.am to drop the phpstringfuncdir block and
the src/TStringUtils.php distribution entry; src/Thrift.php no
longer includes the deleted TStringUtils.php.
The Thrift code generator does not emit any TStringFunc* references,
so generated user code is unaffected.
Generated-by: Claude Opus 4.7 <noreply@anthropic.com>1 parent d7e4bf2 commit d25e8ea
27 files changed
Lines changed: 171 additions & 873 deletions
File tree
- .github/workflows
- build/docker
- ubuntu-focal
- ubuntu-jammy
- ubuntu-noble
- lib/php
- lib
- Factory
- Protocol
- StringFunc
- Transport
- src
- test/Unit/Lib
- Factory
- Protocol
- StringFunc
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
162 | 162 | | |
163 | 163 | | |
164 | 164 | | |
165 | | - | |
| 165 | + | |
166 | 166 | | |
167 | 167 | | |
168 | 168 | | |
| |||
928 | 928 | | |
929 | 929 | | |
930 | 930 | | |
931 | | - | |
| 931 | + | |
932 | 932 | | |
933 | 933 | | |
934 | 934 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
52 | 52 | | |
53 | 53 | | |
54 | 54 | | |
55 | | - | |
| 55 | + | |
56 | 56 | | |
57 | 57 | | |
58 | 58 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
244 | 244 | | |
245 | 245 | | |
246 | 246 | | |
247 | | - | |
248 | 247 | | |
249 | 248 | | |
250 | 249 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
233 | 233 | | |
234 | 234 | | |
235 | 235 | | |
236 | | - | |
237 | 236 | | |
238 | 237 | | |
239 | 238 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
232 | 232 | | |
233 | 233 | | |
234 | 234 | | |
235 | | - | |
236 | 235 | | |
237 | 236 | | |
238 | 237 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
63 | 63 | | |
64 | 64 | | |
65 | 65 | | |
66 | | - | |
67 | 66 | | |
68 | 67 | | |
69 | 68 | | |
| |||
106 | 105 | | |
107 | 106 | | |
108 | 107 | | |
109 | | - | |
110 | | - | |
111 | | - | |
112 | | - | |
113 | | - | |
114 | | - | |
115 | 108 | | |
116 | 109 | | |
117 | 110 | | |
| |||
147 | 140 | | |
148 | 141 | | |
149 | 142 | | |
150 | | - | |
151 | 143 | | |
152 | 144 | | |
153 | 145 | | |
| |||
This file was deleted.
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
24 | 24 | | |
25 | 25 | | |
26 | 26 | | |
27 | | - | |
28 | 27 | | |
29 | 28 | | |
30 | 29 | | |
| |||
211 | 210 | | |
212 | 211 | | |
213 | 212 | | |
214 | | - | |
| 213 | + | |
215 | 214 | | |
216 | 215 | | |
217 | 216 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
24 | 24 | | |
25 | 25 | | |
26 | 26 | | |
27 | | - | |
28 | 27 | | |
29 | 28 | | |
30 | 29 | | |
| |||
136 | 135 | | |
137 | 136 | | |
138 | 137 | | |
139 | | - | |
| 138 | + | |
140 | 139 | | |
141 | 140 | | |
142 | 141 | | |
| |||
364 | 363 | | |
365 | 364 | | |
366 | 365 | | |
367 | | - | |
| 366 | + | |
368 | 367 | | |
369 | 368 | | |
370 | 369 | | |
| |||
761 | 760 | | |
762 | 761 | | |
763 | 762 | | |
764 | | - | |
| 763 | + | |
765 | 764 | | |
766 | 765 | | |
767 | 766 | | |
| |||
This file was deleted.
0 commit comments