Page MenuHomePhabricator

UploadStash should use a valid regex
Closed, ResolvedPublic


(From the Support Desk.)

I'm confused by the regex in UploadStash.php:

# When this task was originally filed the const was set to '/^[\w\-\.]+\.\w*$/'.  See comment below.
const KEY_FORMAT_REGEX = '/^[\w-\.]+\.\w*$/';

What is [\w-\.] supposed to mean?

In any case, while the regex works in php 5.x and 7.2, in 7.3, it returns different results. The following one-liner can be used to test this:

php -r '$file="a.bcd"; $key="/^[\w-\.]+\.\w*$/"; var_dump(preg_match($key, $file));'

That works for me on php 5.6 and php 7.2 (it prints int(1)), but fails on php 7.3 where it returns:

PHP Warning:  preg_match(): Compilation failed: invalid range in character class at offset 4 in Command line code on line 1

Event Timeline

Your copy of KEY_FORMAT_REGEX includes an escaped -, your php example does not use a escaped -

I see no error on using escaped \- in regex

It seems the fix already backported to REL1_31

Are you working with 1.31.2?

MarkAHershberger added a comment.EditedFri, Feb 8, 7:00 PM

It isn't me. Original user (see support desk link above) was on 1.31.1.

Also, thanks for pointing out the escape in what is on master. I created the test based on what posted on the support desk, but copied the KEY_FORMAT_REGEX above from master without noticing the added escape.

MarkAHershberger updated the task description. (Show Details)
Reedy added a comment.Fri, Feb 8, 7:21 PM

So this is basically fixed, but fix still to be released?

Change 489304 had a related patch set uploaded (by Jforrester; owner: Jforrester):
[mediawiki/core@REL1_31] Follow-up I41cc21708: Add to RELEASE-NOTES as it's now a pre-release patch

Change 489304 merged by jenkins-bot:
[mediawiki/core@REL1_31] Follow-up I41cc21708: Add to RELEASE-NOTES as it's now a pre-release patch