Skip to content

fix: Use mb_ string operations in planet captcha#213

Merged
grandeljay merged 3 commits intowishthis:developfrom
PrivateCoffee:fix-multibyte
Mar 10, 2025
Merged

fix: Use mb_ string operations in planet captcha#213
grandeljay merged 3 commits intowishthis:developfrom
PrivateCoffee:fix-multibyte

Conversation

@thekumi
Copy link
Contributor

@thekumi thekumi commented Mar 7, 2025

Hi!

A user using their browser in Ukrainian reported issues registering an account, with the site not accepting "Марс" (Mars) as an answer to the question for a planet in our solar system.

The issue seems to consist of two parts – first, the Ukrainian translations of some planet names included periods at the end. I have fixed that on Transifex, see #211.

But also the comparison itself had some problems with multi-byte characters from non-Latin scripts. I have replaced the strtolower and strtoupper functions with their mb_-prefixed versions to ensure they work properly.

I also removed a Sanitiser::getTitle call on the input – since we are only comparing strings and not trusting the input for anything else, sanitization shouldn't be necessary here, and if we run it on the input, we would also have to run it on the accepted responses, so throwing it out altogether was the easier option. I re-added sanitization for the variable used in the error message when you enter an incorrect answer. I suppose that's what is was meant for.

thekumi added 2 commits March 7, 2025 12:16
Updates register.php to use multibyte versions of strtolower/strtoupper for compatibility with non-Latin alphabets.
Simplifies the comparison by removing a Sanitise call.
@thekumi
Copy link
Contributor Author

thekumi commented Mar 7, 2025

Me again – added a second commit. The substring handling in $planetName = mb_strtoupper($planet[0]) . substr($planet, 1); actually also tore multi-byte characters apart, which caused display issues in the error message if you give an incorrect answer. I now use mb_substr to handle that. There is also now mb_ucfirst which would solve this, but that is only available in PHP 8.4.

Copy link
Member

@grandeljay grandeljay left a comment

Choose a reason for hiding this comment

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

LGTM! Thank you for your service!

@grandeljay grandeljay merged commit ddcb65d into wishthis:develop Mar 10, 2025
grandeljay added a commit that referenced this pull request Mar 10, 2025
@thekumi thekumi deleted the fix-multibyte branch March 10, 2025 14:52
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.

2 participants