Skip to content

Reject 4-byte emoji and stop them causing SQL errors - #441

Open
iMattPro wants to merge 11 commits into
phpbb:3.3.xfrom
iMattPro:issue/389
Open

Reject 4-byte emoji and stop them causing SQL errors#441
iMattPro wants to merge 11 commits into
phpbb:3.3.xfrom
iMattPro:issue/389

Conversation

@iMattPro

@iMattPro iMattPro commented Jul 27, 2026

Copy link
Copy Markdown
Member

This may be a more complete solution to the emoji limitations in titania.

  • Blocked unsupported four-byte Unicode characters from contribution metadata, categories, revisions, package names, authors, links, and filenames.
  • Kept four-byte character support in descriptions, posts, topics, comments, reports, and validation output using safe database encoding.
  • Enforced ASCII-only new or changed permalinks while grandfathering unchanged legacy Unicode permalinks.
  • Safely escaped four-byte characters in Composer JSON without altering its structure or formatting.
  • Protected package-derived writes, including Composer names, style filenames, revision fields, demo URLs, and ColorizeIt metadata.
  • Found and fixed a string-truncation bug.

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.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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\emoji helper to detect/strip unsupported characters and to JSON-escape non‑BMP characters as surrogate pairs.
  • Adds encode_ucr support 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.

Comment thread entity/database_base.php Outdated
Comment thread entity/database_base.php Outdated
Comment thread emoji.php Outdated
Comment thread emoji.php Outdated
Comment thread posting.php Outdated
Comment thread language/en/contributions.php Outdated
Comment thread language/en/common.php Outdated
@ECYaz

ECYaz commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

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 fix/441-completion, based on this PR's head).

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 titania_<type>_queue forums and robot accounts configured, the final step dies with the exact error from the original report:

Incorrect string value: '\xF0\x9F\x91\x8D t...' for column 'post_text' at row 1 [1366]
UPDATE customisation_posts SET post_text = '[url=...]Queue Discussion Topic[/url] ... user: tester 👍 pass: secret[/quote] ...'

Two things combine here:

  • message::decode() runs html_entity_decode() while rebuilding the first queue post, so the utf8_encode_ucr() applied to the test account text is undone and the raw emoji comes back. Encoding the inputs cannot protect this path.
  • forum_queue_update_first_queue_post() calls $post_object->submit() (queue.php line 829, it needs a post id for get_url()) while the rebuilt text is still raw source. The generate_text_for_storage() call only runs after the mirror. That early submit is only reached when the mirror forums are configured and the queue status flips from hide to new at the final step.

The fix is to parse before the mirror runs, and to have the mirror decode the stored post on its own before concatenating, because decode_message() only unparses text that starts with the parser's markup (the same pattern the function already uses for the discussion topic copy):

 		// 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, queue_notes and the queue post store parsed text, and the copied phpBB forum post comes out correctly, quote structure and emoji included. Without the reorder the same flow still produces the crash above. This reordering is also the root fix for #389 on current 3.3.x.

2. Emoji in the composer.json name field still crashes

validate_ext_name() matches bytes ([a-zA-Z0-9\x7f-\xff]), so "name": "acme😀/demo" passes it, and contrib_package_name is written through a direct $contrib->submit() that never runs validate():

Incorrect string value: '\xF0\x9F\x98\x80/d...' for column 'contrib_package_name' at row 1 [1366]

The commit guards validate_ext_name() with emoji::contains(), which reuses the existing INVALID_EXT_NAME error path:

 	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:

  • An emoji typed into the permalink field is not rejected as the new error message promises. generate_slug() strips it before validate_permalink() sees the original, so rename_🆕_target silently saves as rename___target. A contains() check on the raw request value in the create and manage controllers would make behaviour match the message.
  • Since contains() intentionally covers BMP emoji, contributions that already exist with characters like ™ or ⭐ in their names can no longer be saved from the manage form at all, including saves that change nothing, which also blocks status and co-author changes. Production data may be worth a sweep before this merges, or the check could be limited to \x{10000}-\x{10FFFF}, which is the exact set the utf8 schema cannot store.

With the two fixes above this works end to end here and supersedes #439, #440 and #390.

@iMattPro

Copy link
Copy Markdown
Member Author

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 fix/441-completion, based on this PR's head).

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 titania_<type>_queue forums and robot accounts configured, the final step dies with the exact error from the original report:

Incorrect string value: '\xF0\x9F\x91\x8D t...' for column 'post_text' at row 1 [1366]
UPDATE customisation_posts SET post_text = '[url=...]Queue Discussion Topic[/url] ... user: tester 👍 pass: secret[/quote] ...'

Two things combine here:

  • message::decode() runs html_entity_decode() while rebuilding the first queue post, so the utf8_encode_ucr() applied to the test account text is undone and the raw emoji comes back. Encoding the inputs cannot protect this path.
  • forum_queue_update_first_queue_post() calls $post_object->submit() (queue.php line 829, it needs a post id for get_url()) while the rebuilt text is still raw source. The generate_text_for_storage() call only runs after the mirror. That early submit is only reached when the mirror forums are configured and the queue status flips from hide to new at the final step.

The fix is to parse before the mirror runs, and to have the mirror decode the stored post on its own before concatenating, because decode_message() only unparses text that starts with the parser's markup (the same pattern the function already uses for the discussion topic copy):

 		// 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, queue_notes and the queue post store parsed text, and the copied phpBB forum post comes out correctly, quote structure and emoji included. Without the reorder the same flow still produces the crash above. This reordering is also the root fix for #389 on current 3.3.x.

2. Emoji in the composer.json name field still crashes

validate_ext_name() matches bytes ([a-zA-Z0-9\x7f-\xff]), so "name": "acme😀/demo" passes it, and contrib_package_name is written through a direct $contrib->submit() that never runs validate():

Incorrect string value: '\xF0\x9F\x98\x80/d...' for column 'contrib_package_name' at row 1 [1366]

The commit guards validate_ext_name() with emoji::contains(), which reuses the existing INVALID_EXT_NAME error path:

 	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:

  • An emoji typed into the permalink field is not rejected as the new error message promises. generate_slug() strips it before validate_permalink() sees the original, so rename_🆕_target silently saves as rename___target. A contains() check on the raw request value in the create and manage controllers would make behaviour match the message.
  • Since contains() intentionally covers BMP emoji, contributions that already exist with characters like ™ or ⭐ in their names can no longer be saved from the manage form at all, including saves that change nothing, which also blocks status and co-author changes. Production data may be worth a sweep before this merges, or the check could be limited to \x{10000}-\x{10FFFF}, which is the exact set the utf8 schema cannot store.

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.

@iMattPro iMattPro changed the title Encode or reject emoji where applicable Encode or reject 4-byte emoji where applicable Jul 28, 2026
@ECYaz

ECYaz commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Nice, that worked. Retested e47f059 with the mirror configured and everything passes here.

One remaining suggestion on the permalink slug. The ASCII flattening in generate_permalink_slug() goes further than the database limitation and costs non-Latin contributions their permalinks when one is auto-generated from the name:

  • "Русский язык" produces pycc___i_
  • "中文語言包" produces just _, and further CJK-named contributions collide into _2, _3, and so on

The database limitation is only the four byte characters, and url::generate_slug() already strips those through unicode::strip_unsupported(). Using it as the canonical slug restores readable permalinks for non-Latin names, in the same way a08d574 matched the detection scope to the database limitation:

 	protected function generate_permalink_slug($value)
 	{
-		return preg_replace('/[^a-z0-9_]+/', '_', url::generate_slug($value));
+		return url::generate_slug($value);
 	}

@ECYaz

ECYaz commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

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

Parameter "contrib" for route "phpbb.titania.contrib.revision" must match "[^/]++" ("" given)

and the row is saved with an empty contrib_name_clean, so the contribution exists but has no working URL. The generated permalink now stays local inside validate(); the manage controller reassigns it on submit, but the create path in author.php never does. Mirroring what manage does:

			if (empty($error))
			{
+				if ($contrib->contrib_name_clean === '')
+				{
+					$contrib->generate_permalink();
+				}
				$contrib->set_type($contrib->contrib_type);
				$contrib->set_custom_fields($settings['custom']);

@iMattPro

Copy link
Copy Markdown
Member Author

Thanks for the reviews @ECYaz
I verified your finding and fixed it, along with additional issues that were found related to cleaning these perma-links.

@iMattPro iMattPro changed the title Encode or reject 4-byte emoji where applicable Reject 4-byte emoji and stop them causing SQL errors Jul 29, 2026
@ECYaz

ECYaz commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

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.

@iMattPro
iMattPro requested a review from DavidIQ July 30, 2026 02:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Emojis in revision instructions cause general error

3 participants