Last modified: 2014-02-12 23:35:54 UTC

Wikimedia Bugzilla is closed!

Wikimedia migrated from Bugzilla to Phabricator. Bug reports are handled in Wikimedia Phabricator.
This static website is read-only and for historical purposes. It is not possible to log in and except for displaying bug reports and their history, links might be broken. See T46066, the corresponding Phabricator task for complete and up-to-date bug report information.
Bug 44066 - Files cannot be uploaded when database name contains certain characters due to regex check in FileBackendStore.php
Files cannot be uploaded when database name contains certain characters due t...
Status: UNCONFIRMED
Product: MediaWiki
Classification: Unclassified
Uploading (Other open bugs)
1.19.1
All All
: Low normal (vote)
: ---
Assigned To: Nobody - You can work on this!
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2013-01-17 16:28 UTC by grotlek
Modified: 2014-02-12 23:35 UTC (History)
3 users (show)

See Also:
Web browser: ---
Mobile Platform: ---
Assignee Huggle Beta Tester: ---


Attachments

Description grotlek 2013-01-17 16:28:48 UTC
This problem took me a LONG time debugging using var_dump to get to the bottom of, but here goes..

My database backend (pgsql) allows database names containing . and case sensitivity, and so I have gotten in the habit of naming my databases sensibly; using a form such as "www.example.com-WikiDB".

Unfortunately doing something like this leaves it impossible to upload files, despite the upload directory being writeable by the web-server. A very obscure error message ("Could not create directory "mwstore://local-backend/local-public/(etc)") is shown.

I traced the "fault" to a regular expression match in the function isValidContainerName() in mediawiki/includes/filerepo/backend/FileBackend.php.

Modifying the regular expression so it allowed uppercase and the dot character fixed my problem, but I don't know whether this'd have implications. I changed the part inside [] to be [a-zA-Z0-9][a-zA-Z0-9-_.] (rest of regex left unchanged).

I recommend someone consider the reason for certain characters (uppercase, period - possibly others) which are valid in database names being invalid in this regexp, and update if considered and found appropriate.
Comment 1 Andre Klapper 2013-01-17 18:27:54 UTC
Hi grotlek, thanks for reporting and tracking this down!

In current git master (1.21wmf8) this function is located in the file
./includes/filebackend/FileBackendStore.php and you refer to this regex:

return preg_match( '/^[a-z0-9][a-z0-9-_]{0,199}$/i', $container );
Comment 2 grotlek 2013-01-17 18:55:49 UTC
Hi Andre,

Yep, that's the one. It looks like it must have moved since 1.19. Sorry I don't have the code near me -- it's in a cold, dark and noisy room -- but from that, it looks like it's doing a case insensitive match anyways, so the only addition I'd suggest is a . character in the second square brackets. (I'm not sure whether I missed spotting that previously -- I was on a "just get it working" mission :-).

Of course, while this fixes my problem, I could not tell you whether:
a) it would have any unwanted side effects or implications elsewhere.
b) it would then make the regex cover all valid scenarios (i.e., do people use other characters?)

I was a bit hesitant from posting this at all, as I couldn't even figure out why it was fiddling with with the database name at all during file upload (it isn't using it in any obvious way -- the uploaded files don't go to a directory containing the DB name, for example). Given this, I'll back out at this point and let you more-knowledgeable folks handle it (if any change is warranted).

- Grotty.
Comment 3 Elliott Eggleston 2013-09-11 04:08:49 UTC
44949 is another report of this bug.  The regex is so strict to catch any container names that would break cloud storage backends that may use a '.' to separate namespaces.  I just submitted a patch that leaves the strict regex in FileBackendStore but overrides it in FSFileBackend with a much more relaxed one that allows anything but '/'.
Comment 4 Elliott Eggleston 2013-09-11 04:55:48 UTC
I suppose it would be appropriate to link to that commit: https://gerrit.wikimedia.org/r/83774/

Note You need to log in before you can comment on or make changes to this bug.


Navigation
Links