[GIT-PULLS] [php-src] PR #23307: Fix enchant_broker_set_dict_path() and enchant_broker_get_dict_path() returning null
[email protected] (lacatoire)
| Newsgroups | php.git-pulls |
|---|---|
| Message-ID | <[email protected]> |
Pull Request: https://github.com/php/php-src/pull/23307 Author: lacatoire `enchant_broker_set_dict_path()` and `enchant_broker_get_dict_path()` return `null` on every build linked against libenchant 2, violating the `bool` and `string|false` return types their stubs declare. In both functions the entire body after parameter parsing sits inside `#ifdef HAVE_ENCHANT_BROKER_SET_PARAM`, with nothing after the `#endif`, so when the macro is undefined the function falls off the end without setting a return value. `ext/enchant/config.m4` defines that macro only on the legacy libenchant 1.x path, and its own comment says why: `enchant_broker_set_param` was "available since 1.5.0 and removed in 2.x". Since `PKG_CHECK_MODULES` looks for `enchant-2` first and that is what distributions ship, both bodies are compiled away in practice. Reproduced from `php:8.4-cli` with `libenchant-2-dev` and `docker-php-ext-install enchant`, libenchant 2.8.2: ```php <?php $b = enchant_broker_init(); var_dump(@enchant_broker_set_dict_path($b, ENCHANT_MYSPELL, '/tmp')); // NULL, declared bool var_dump(@enchant_broker_get_dict_path($b, ENCHANT_MYSPELL)); // NULL, declared string|false ``` Internal functions are not return-type checked, so this stays silent until the value reaches typed userland code, where the `TypeError` points at the caller rather than at the source. Returning `false` on that path matches what both functions already do for every in-band failure: unknown `$type`, empty path, unset path. `enchant_broker_set_dict_path.phpt` and `bug53070.phpt` both carry `skip libenchant v1 only`, so the affected path had no coverage. A test is added for it, verified to fail without the C change and pass with it. The whole suite passes on a libenchant 2.3.3 build: 32 passed, 2 skipped, those being the v1-only tests. Both functions are deprecated since 8.0, which argues for this minimal fix rather than a redesign. An explicit diagnostic on the unsupported path would also be defensible, since they currently look like silent no-ops; left out here to keep the change minimal.