Page MenuHomePhabricator

mw.ustring.gsub replacement string handling differs between PHP and pure Lua
Closed, ResolvedPublicBUG REPORT

Description

The pure Lua version of mw.ustring.gsub handles replacement strings containing invalid '%' sequences differently than the PHP version.

There are two issues:

Issue 1: If a replacement string has '%' followed by an unrecognized character, the pure Lua implementation removes the '%', but PHP keeps it as-is:

mw.ustring.gsub("test", "test", "x%yz")

PHP: x%yz
Lua: xyz

Issue 2: A '%' at the end of a replacement string causes Lua to insert a null byte:

mmw.ustring.gsub("test", "test", "bar%")

PHP: bar%
Lua: bar\000

Both implementations should handle '%' characters the same way.

https://gerrit.wikimedia.org/r/c/mediawiki/extensions/Scribunto/+/1232409 (not for submission) demonstrates the bug: if you update the expected value to match the actual value cited in the test failure, the test will fail again, this time due to a mismatch with the PHP implementation.

You can run the test by installing the Scribunto extension and running composer phpunit -- extensions/Scribunto/tests/phpunit from the MediaWiki installation root.

Here is where the Lua implementation of gsub is located: includes/Engines/LuaCommon/lualib/ustring/ustring.lua, lines 945-1055

Event Timeline

AtharvaKathe subscribed.
This comment was removed by AtharvaKathe.

This is a good first task for developers familiar with Lua (or willing to learn). Here is where the Lua implementation of gsub is located: includes/Engines/LuaCommon/lualib/ustring/ustring.lua, lines 945-1055

This change has unit tests that fail against the current implementation: I7d2574f5e2.

You can run the test by installing the Scribunto extension and running composer phpunit -- extensions/Scribunto/tests/phpunit from the MediaWiki installation root. If the tests pass, you probably fixed it.

Thank you for tagging this task with good first task for Wikimedia newcomers!

Newcomers often may not be aware of things that may seem obvious to seasoned contributors, so please take a moment to reflect on how this task might look to somebody who has never contributed to Wikimedia projects.

A good first task is a self-contained, non-controversial task with a clear approach. It should be well-described with pointers to help a completely new contributor, for example it should clearly point to the codebase URL and provide clear steps to help a contributor get set up for success. We've included some guidelines at https://phabricator.wikimedia.org/tag/good_first_task/ !

Thank you for helping us drive new contributions to our projects <3

Hi @ori , I’m interested in working on this task.
I’m learning Lua and would like to try fixing the ustring.gsub replacement behavior.
Please let me know if that’s okay.

Change #1235487 had a related patch set uploaded (by Suraj Seth; author: Suraj Seth):

[mediawiki/extensions/Scribunto@master] scribunto: Fix mw.ustring.gsub handling of invalid % sequences

https://gerrit.wikimedia.org/r/1235487

Change #1235487 merged by jenkins-bot:

[mediawiki/extensions/Scribunto@master] scribunto: Fix mw.ustring.gsub handling of invalid % sequences

https://gerrit.wikimedia.org/r/1235487