Chad wrote:
On Fri, Sep 5, 2008 at 6:11 AM, Tim Starling
<tstarling(a)wikimedia.org> wrote:
[snip]
The technical details were as follows: I fixed a bug in File.php, and
without checking what other changes were made to it, deployed the most
recent version of the file on the Wikimedia servers, without also updating
the rest of MediaWiki. Because FileRepo::$thumbDir was unset,
LocalFile::migrateThumbFile() had the effect of deleting the source image
for any thumbnail request which reached the backend. I reverted the change
after about 20 minutes, following a report on IRC.
My sincere apologies.
And mine as well. I introduced the $thumbDir code in r40385. I thought
I had set a sane default of 'thumb/' in the constructor (which would work
per current behavior of using hardcoded 'thumb/'. Is there a code-path in
which $thumbDir isn't being set? If so, that needs fixing asap. Would a
revert be in order, or is everything ok as-is?
If you had followed my example and used an accessor function, instead of
having the File class access member variables of the repo directly, then
there would have been no problem. Adding an accessor is good style in any
case, and you should make that change. But it wasn't your fault.
I patched two files in quick succession: GlobalFunctions.php and then
File.php. With GlobalFunctions.php, I checked the diff carefully for any
dependencies before I updated it on Wikimedia. There were no changes other
than my own. With File.php, I assumed it would be OK and didn't check. I
didn't think about it at the time, I was working quickly. Call it
cognitive bias, loss of concentration, laziness, whatever. Not your fault.
There was a second programming error here, and that was the fact that I
put an unlink() call in the code in the first place. It didn't seem
dangerous at the time, but obviously migrateThumbFile() is a recipe for
disaster if there's a potential for adverse input coming from getThumbPath().
However, the thumb directory is inherently temporary, and lots of things
delete from it. I think I'd be most comfortable not having the thumbDir
feature at all. Is there some reason for it?
-- Tim Starling