Reject 4-byte emoji and stop them causing SQL errors - #441
Conversation
There was a problem hiding this comment.
Pull request overview
This PR addresses Titania/phpBB MySQL utf8 limitations around emoji/non‑BMP characters by selectively encoding message-like fields to be DB-safe while rejecting or stripping such characters from metadata fields (names/permalinks/authors/categories/filenames) that should remain constrained.
Changes:
- Introduces a new
phpbb\titania\emojihelper to detect/strip unsupported characters and to JSON-escape non‑BMP characters as surrogate pairs. - Adds
encode_ucrsupport to entity string validation and applies it across multiple entities/fields to avoid SQL errors on direct writes and subject-like columns. - Adds validation to reject emoji/non‑BMP characters in contribution metadata, revision metadata, category names, and uploaded filenames; also strips unsupported characters from generated slugs.
Reviewed changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| url/url.php | Strips unsupported characters before slug generation to keep permalinks clean. |
| posting.php | Encodes post subject for a direct DB update path that bypasses entity validation. |
| manage/tool/composer/rebuild_repo.php | Escapes non‑BMP characters in extracted composer.json while preserving formatting/structure. |
| language/en/contributions.php | Adds user-facing validation messages for disallowed emoji/non‑BMP in contribution/revision metadata. |
| language/en/common.php | Adds user-facing validation message for disallowed emoji/non‑BMP in category names. |
| includes/objects/topic.php | Enables UCR encoding/max length constraints for topic subject fields. |
| includes/objects/post.php | Enables UCR encoding/max length constraints for post subject/edit reason fields. |
| includes/objects/faq.php | Enables UCR encoding/max length constraints for FAQ subject field. |
| includes/objects/contribution.php | Rejects emoji/non‑BMP in contribution metadata; validates demo URLs even when emoji is hidden in JSON escapes; filters emoji in author username lists. |
| includes/objects/category.php | Rejects emoji/non‑BMP in category names. |
| includes/objects/attention.php | Enables UCR encoding/max length constraints for attention title/description. |
| entity/database_base.php | Enhances string validation to truncate correctly and re-truncate after UCR encoding. |
| emoji.php | Adds emoji utility: JSON escaping, detection, and stripping helpers. |
| controller/contribution/revision.php | Encodes queue notes test-account text; rejects emoji/non‑BMP in revision name/version/license. |
| controller/contribution/revision_edit.php | Rejects emoji/non‑BMP in revision edit fields (name/license/custom license). |
| contribution/translation/type.php | Encodes validator output before persisting to DB. |
| contribution/extension/type.php | Ensures generated composer JSON written to disk is JSON-safe via surrogate escaping. |
| attachment/uploader.php | Rejects emoji/non‑BMP in uploaded filenames. |
| attachment/attachment.php | Enables UCR encoding for attachment comments and ensures update paths run entity validation. |
Comments suppressed due to low confidence (1)
language/en/contributions.php:188
- emoji::contains() also flags non-emoji 4-byte characters (e.g. rare CJK). Saying only "Emoji" are not allowed can be confusing when the rejection is due to a non-emoji non-BMP character. Consider wording that reflects both cases.
'REVISION_EMOJI_NOT_ALLOWED' => 'Emoji are not allowed in revision names, versions, or licenses.',
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Tested the latest head (f56ac6f) on a local 3.3.x replica with the validation queue forums and robot accounts configured, the same layout as phpbb.com. The approach holds up well: attention reports store and render emoji correctly, emoji names are rejected on create and on rename, topic subjects encode properly, the branch merges clean onto current 3.3.x, and phpcs reports no new violations. Two crashes remain, and both have a fix in a commit you can cherry-pick: ECYaz/customisation-db@cb7d33be (branch 1. The #389 scenario still crashes, but only when the queue mirror forums are configured Submit a new extension revision with 👍 in the validation notes or the test account field. On a bare install it works, which is probably why it passes local testing. With Two things combine here:
The fix is to parse before the mirror runs, and to have the mirror decode the stored post on its own before concatenating, because // Prevent errors from different configurations
phpbb::$config['min_post_chars'] = 1;
phpbb::$config['max_post_chars'] = 0;
+ // Parse the text before the forum copy runs: it stores the post early
+ // to obtain an id, and raw text may hold four byte characters the
+ // database cannot store.
+ $post->generate_text_for_storage(true, true, true);
+
$this->forum_queue_update_first_queue_post($post);
// Store the post
- $post->generate_text_for_storage(true, true, true);
$post->submit();- $post_text .= "\n\n" . $post_object->post_text;
+ // Decode the stored post on its own: decode_message() only unparses
+ // text that starts with the parser's markup.
+ $queue_post_text = $post_object->post_text;
+ handle_queue_attachments($post_object, $queue_post_text);
+ message::decode($queue_post_text, $post_object->post_text_uid);
- handle_queue_attachments($post_object, $post_text);
- message::decode($post_text, $post_object->post_text_uid);
+ $post_text .= "\n\n" . $queue_post_text;
$post_text .= "\n\n" . $path_helper->strip_url_params($post_object->get_url(), 'sid');Verified with the mirror configured: the full flow completes, 2. Emoji in the composer.json
The commit guards public function validate_ext_name($name)
{
+ // The byte ranges below would let four byte characters through, and
+ // the name is stored in a utf8 column that cannot hold them.
+ if (emoji::contains($name))
+ {
+ return false;
+ }
+
return (bool) preg_match(
'#^[a-zA-Z0-9\x7f-\xff]{2,}/[a-zA-Z0-9\x7f-\xff]{2,}$#',
$name
);
}Two smaller review notes, no crashes involved:
With the two fixes above this works end to end here and supersedes #439, #440 and #390. |
Thanks for the testing. These and additional improvements have been implemented. Also narrowed scope so now we just reject/block the BMP/4-byte emoji type characters that mirrors the database limitation. |
|
Nice, that worked. Retested e47f059 with the mirror configured and everything passes here. One remaining suggestion on the permalink slug. The ASCII flattening in
The database limitation is only the four byte characters, and protected function generate_permalink_slug($value)
{
- return preg_replace('/[^a-z0-9_]+/', '_', url::generate_slug($value));
+ return url::generate_slug($value);
} |
|
Retested the entire branch again on 2b29017 with the queue mirror configured and everything passes, the new filter is better than what I suggested. One regression though: creating a new contribution with the permalink field left blank (the default the form suggests) now fails with and the row is saved with an empty if (empty($error))
{
+ if ($contrib->contrib_name_clean === '')
+ {
+ $contrib->generate_permalink();
+ }
$contrib->set_type($contrib->contrib_type);
$contrib->set_custom_fields($settings['custom']); |
|
Thanks for the reviews @ECYaz |
|
Retested 82ca9fb with the queue mirror configured and everything passes here, including the blank permalink create and the legacy permalink cases. All good on my end. |
This may be a more complete solution to the emoji limitations in titania.
Fixes #389
Closes #440
Closes #439
Closes #390
This is an alternative to #440 and #390 and #439
But it could use additional testing and input from the authors of the above mentioned PRs.