Skip to content

Prevent HasSlug::getUtf8Slug() from guessing encoding by default - #2771

Merged
ifox merged 3 commits into
area17:3.xfrom
zachgarwood:fix/slug-encoding
Aug 20, 2025
Merged

Prevent HasSlug::getUtf8Slug() from guessing encoding by default#2771
ifox merged 3 commits into
area17:3.xfrom
zachgarwood:fix/slug-encoding

Conversation

@zachgarwood

Copy link
Copy Markdown
Contributor

Description

By default, the method was guessing from a list of all possible encodings what the encoding of the given string was. This change allows the method caller to supply the list of possible encodings. If no encodings are provided, it will use the default_charset setting. See https://www.php.net/manual/en/function.mb-convert-encoding.php.

This is an extremely narrow edge case, but it is possible to encounter an encoding clash, where the given string just so happens to be valid in two different encodings. We currently only have one example, but we have encountered this issue a couple other times in the past, but did not record them.

The string in question is "Tusk". Below, we see that the string was converted to "畔歳" because mb_convert_encoding() guessed it was encoded in UCS-2LE, an obsolete encoding that was superceded by UTF-16:

vagrant@homestead:~/website$ php artisan tinker
Psy Shell v0.12.8 (PHP 8.3.23 — cli) by Justin Hileman
> $str = mb_convert_encoding((string)'Tusk', 'UTF-8', mb_list_encodings());
= "畔歳"
> mb_detect_encoding('Tusk', mb_list_encodings());
= "UCS-2LE"

Under the hood, mb_convert_encoding() uses mb_detect_encoding() to guess the encoding, and from the documentation, we can see that this isn't always reliable:
Screenshot 2025-07-17 at 11 31 14 AM
https://www.php.net/manual/en/function.mb-detect-encoding.php

By default, the method was guessing from a list of all possible encodings what the encoding of the given string was. This change allows the method caller to supply the list of possible encodings. If no encodings are provided, it will use the `default_charset` setting. See https://www.php.net/manual/en/function.mb-convert-encoding.php.
@ifox
ifox merged commit 6d622a2 into area17:3.x Aug 20, 2025
14 of 16 checks passed
public function getUtf8Slug(string $str, array $options = []): string
{
// Make sure string is in UTF-8 and strip invalid UTF-8 characters
$str = mb_convert_encoding((string) $str, 'UTF-8', mb_list_encodings());

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Shouldn't this mb_convert_encoding have been removed then?

Why is it running mb_convert_encoding twice?

@Tofandel Tofandel Dec 2, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I also don't see any behavior change because the options still pass a non configurable mb_list_encodings in the end. I think this PR could have used some tests because right now the only thing it does is run twice the problematic mb_convert_encoding instead of not running it at all

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure what happened during the merge, but this line was removed in my original commit:
d58f575#diff-522aa2fdc5f1c037470707e1aa775f40445c9bc9dbf802f02e267975a63281d6L444-L446

It should not be there.

@ifox ifox mentioned this pull request Feb 14, 2026
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.

3 participants