Harden language catalog repair and syntax validation - #1114
Conversation
|
I think we'd be better off doing a one off fix on those files with invalid syntax, and just aborting/breaking within update-languages.sh if there is invalid syntax in the future (as in, stop trying to be clever fixing up invalid syntax). |
|
Thanks, agreed. I simplified languages/language-update.sh so it only validates catalog PHP syntax and aborts the scan on the first invalid file; the 24 affected catalogs remain a one-off correction. The optional repair logic is now isolated in ADDITIONS/fix-language-syntax.php and is never called by the main script or CI. The updated GitHub CI run passed lint, PHP 8.2 through 8.5, MySQL, and PostgreSQL. |
| function next_backup_filename(string $file): string | ||
| { | ||
| $backup = "$file.bak"; | ||
| for ($suffix = 2; file_exists($backup); $suffix++) { |
There was a problem hiding this comment.
file_exists comes with some funny details:
- it returns false on dead symlinks (so you can have symlinks attack even without any race involved)
- it caches the result
This is probably not very problematic for this script, but maybe you should at least add a comment (in the file header) like "don't run this script with files in directories that are writeable by others (like /tmp/)"
There was a problem hiding this comment.
the cache only lasts as long as the php process, so probably not a problem here.
I just don't see any need to have the 'fix-language-syntax.php' script in git, that's all.
What we do probably need is to just run 'php -l' on each language file (or include languages/ in what psalm or parallel-lint checks).
I introduced these syntax errors when I updated the *.lang files two days ago. The most obvious (and most boring) issue was that some strings were missing in most language files. There were also some texts that were changed, and not yet translated in most languages. Instead of just adding a comment, I replaced the not-yet-translated texts with the new text using some script magic, and accidentally killed the semicolon. Sorry for that! (I'm not sure if my translation updates + fixes from this PR are also needed in the 4.0 branch - feel free to review and backport them.) |
|
Thanks. I removed the automatic repair helper from the repository and added an explicit |
- add the missing Spanish translation for `pException_ip_error`, which is used by the 4.x TOTP exception flow - remove the master-only `pDkim_field_domain_and_selector` key from the 4.x Spanish catalog - restore exact key parity between `languages/en.lang` and `languages/es.lang` related to #1114 thanks @TrapoSAMA
3 participants
Summary
php -lchecks in CIRoot cause
The tokenizer-based parser accepted token streams from syntactically invalid catalogs. Missing semicolons caused adjacent
$PALANGassignments to be treated as one statement, so--patchcould insert duplicate keys.Behavior
languages/language-update.shdoes not attempt automatic repair. It validates the complete PHP source before comparing translation keys and aborts the scan on the first syntax error.Read-only catalog checks return status 1 when any catalog has missing or obsolete keys.
--patchcan return 0 after adding every missing key, but still returns 1 when obsolete keys remain. The GitHub Actions lint job runs the complete catalog check and explicitly executesphp -lfor every language file.Validation
git diff --checkpassedThe full PHPUnit suite was not run locally because dependencies are unavailable in this checkout. Regression tests are included for CI.
Fixes #1112.
Fixes #1113.