Fix encoded names on the renewal page

[guerricv]

Description

The renewal page displays stored HTML entities literally: an item named Arrivée appears as Arrivée instead of Arrivée. Folder names have the same double-escaping problem.

Decode one storage entity layer before escaping item labels and folder breadcrumbs for HTML output. Search and text sorting also use decoded labels and folder names, so users can search for the text they see and entity fragments such as amp or quot do not produce false matches. Filtering and text sorting happen before pagination, within the existing server-side authorization scope. The default chronological view retains SQL pagination.

Published commit: 0b323c677 — Fix encoded names on the renewal page.

Related issue

No linked issue.

Type of change

• Bug fix (non-breaking change that fixes an issue)
• New feature (non-breaking change that adds functionality)
• Breaking change (existing behaviour changes)
• Documentation
• Translation
• Refactor / maintenance

How has this been tested?

• PHP 8.2.34: all renewal-related PHPUnit tests pass (66 tests, 4,838 assertions), including the real handler executed against SQLite authorization fixtures.
• Added regressions for named and numeric accents, raw UTF-8, R&D, O'Brien, quotes, angle brackets and encoded HTML payloads. They cover safe output, item/folder search, false entity-fragment matches, text sorting and pagination.
• Automated authorization coverage includes standard, manager and admin page permissions, read-only folder grants, item restrictions, personal folders enabled/disabled, and grant revocation. No manual role testing on a live installation was performed.
• The renewal page JavaScript test passes (1 test), covering initial date ordering and clearing the date filter.
• PHP lint, the table-prefix guard and git diff --check pass.
• PHPStan level 4 passes for the modified controller using the standalone PHAR. Full local analysis reports five missing-file errors because this checkout has no app/config/settings.php; the analysis bootstrap uses the shipped sample configuration. Full PHPStan analysis passes in CI.
• All checks pass on commit 0b323c677: Quality (PHP 8.2/8.3 test suites, JavaScript tests, PHPStan level 4, table-prefix and production-autoloader guards, concurrent recipient requests with MariaDB), CodeQL, and Scrutinizer.

Checklist

• PHPStan level 4 passes (php app/vendor/bin/phpstan analyse --memory-limit=2G)
• The test suite passes (php _tools/phpunit.phar — see CONTRIBUTING.md for the one-time setup)
• Every new public function has a docblock
• Variable names and comments are in English
• No var_dump() or console.log() left in the code
• New app/sources/*.queries.php files have a matching public/sources/ proxy shim
• Changes to teampassclasses were applied to both copies (app/includes/libraries/ and app/vendor/)
• app/vendor/composer/ is in its production form (git checkout -- app/vendor/composer/)

No new queries controller or teampassclasses change is included. All CI checks passed on the published commit; the PR is ready for review.

Impact on install / upgrade

• No schema change
• Schema change — an install/upgrade_run_X.X.X.php script is included and the fresh install path was tested

No installation or migration step is required.

Screenshots

No screenshot from a live installation. Regression tests verify the HTML returned for table cells; visual verification on an installed instance is still pending.

[nilsteampassnet]

Hi @guerricv, thanks for this one.
The fix is right: decoding one entity layer before escaping is the exact inverse of how labels are stored.

Moving search and text sorting to PHP has two side effects I would like fixed before merging, plus a few smaller points.

1. Text sorting no longer follows the database collation

strcmp(mb_strtolower(...)) compares bytes, so an accented initial sorts after Z:

eclair | Zebra | Écran | Été

The columns are utf8mb4_unicode_ci, where Écran sorts with E. Labels stored as raw UTF-8 (API, imports) were sorted correctly before this PR and are not any more.

2. Search is no longer accent-insensitive

With the SQL LIKE, typing arrivee matched a label stored as Arrivée. With mb_stripos it does not.

Suggested fix for 1 and 2: build one folded key per row and use it for both matching and sorting. ext-intl is not a required extension, so it needs a guard:

$fold = static function (string $text): string {
    if (class_exists('Normalizer')) {
        $text = preg_replace('/\p{Mn}+/u', '', (string) Normalizer::normalize($text, Normalizer::FORM_D)) ?? $text;
    }
    return mb_strtolower($text, 'UTF-8');
};

Fold the search term the same way. Without intl this falls back to what you have now, which is fine.

3. Compute the sort key once per row

The comparator currently lowercases both sides on every comparison. On synthetic data the PHP side takes about 0.2 s for 20,000 rows and about 1.9 s for 100,000, almost all of it in usort. Storing the folded key in the foreach that already builds label_display / folder_display removes that.

4. Skip the SQL ORDER BY when PHP sorts afterwards

When $sortColumn !== 1, the query still sorts every row on i.label or n.title, and usort then discards that order. Keep the SQL ordering only for the date column.

5. Reuse the existing helpers

app/sources/search.functions.php already has searchDecodeStoredText(), searchEscapeStoredText() and searchDecodeFolderTitle(). Please require_once it and use them instead of the four inline html_entity_decode(..., ENT_QUOTES | ENT_HTML5, 'UTF-8') calls, the same way search.queries.php renders labels and folder names. searchEscapeStoredText() also adds ENT_SUBSTITUTE, so a label holding invalid UTF-8 no longer renders as an empty link.

6. Tests

Please add cases for the two behaviours above:

• an accented initial, stored both as Écran and as raw Écran, sorts between eclair and Zebra;
• searching arrivee finds Arrivée in both stored forms.

Guard them with extension_loaded('intl'), since the CI extension list does not name it explicitly.

7. Live check

You mention that visual verification on an installed instance is still pending. Could you confirm once it is done? A screenshot of the table with an accented item and folder name is enough.

No schema, setting or language change is needed, and nothing else in the PR needs to move. Thanks again.

[guerricv]

Hi Nils, thanks for the review. I've addressed the code points in d39c69faa:

• Item labels, folder names and the search term now use the same folded key. With intl, Unicode decomposition removes combining accents before lowercasing, so Écran sorts between eclair and Zebra, and arrivee matches Arrivée. Without intl, the existing case-insensitive fallback remains available.
• Each row's keys are computed once and reused for filtering and sorting. Text sorts no longer request a redundant SQL ORDER BY; date sorting keeps its SQL ordering and pagination.
• The controller now reuses searchDecodeStoredText(), searchDecodeFolderTitle() and searchEscapeStoredText(). Folder output also uses ENT_SUBSTITUTE, so malformed UTF-8 is replaced rather than producing an empty label.
• Added regressions for entity-encoded/raw accented initials, ascending/descending order and pagination, accent-free searches of item/folder names, decomposed Unicode, SQL ordering and invalid UTF-8. The accent-specific cases are guarded with extension_loaded('intl') as requested.

Local validation on PHP 8.2.34: 155 tests / 5,476 assertions pass with intl (renewal and shared search suites). The renewal handler also passes without intl: 50 tests pass, 5 accent-specific cases skipped. PHP lint, the 15 renewal JavaScript tests, the table-prefix guard and PHPStan for the changed controller pass. Full local PHPStan still reports the five missing app/config/settings.php errors noted earlier. Quality and CodeQL have started on the new commit; results, including Scrutinizer, are pending.

I'm keeping the PR in draft while the new checks run and the lab verification is completed. No schema, setting or language change is included.

Lab screenshots

Before the fix

After the fix

After screenshot to be added here.