#180 Import files forgets what it is doing

[Jonathan Dushoff]

I want to input a large file called yes.dat into a project using Import files, and put it in the Image: space (because it's large). If I click through everything in the normal way, I get to a warning (my wiki is configured to not like ".dat" files). If I tell Import files to ignore the error, it jumps back to the input screen, apparently forgetting what I told it and asking me where the file is.

Clicking the "ignore warnings" box solves the problem.

This seems to be highly repeatable.

[Lee Worden]

This might be a misconfiguration on yushan or something. Or it might be a bug in my Special:ImportProjectFiles code. If it's the latter, I'm not sure how much work on that code is justified. It's a modified version of Special:Upload, because there's no framework for extending Special:Upload in MediaWiki 1.13. So I had to take a copy of the whole thing and refactor it into several separate classes so I could subclass some of them. Later I've had to tweak it in various ways to make it play well with later versions of MediaWiki while still being compatible with 1.13. In 1.16 the MediaWiki team did a refactor of their Special:Upload allowing it to be extended in similar ways. So really I should be using their code. The only problem is I would lose backward compatibility with older versions. So at some point I need to abandon versions before 1.16, and then it'll be more straightforward to write a well-functioning, maintainable Special:ImportProjectFiles. This is one reason why I'd like to upgrade our McMaster wikis, so I can abandon support for old versions.

[Lee Worden]

Also, do you know how to configure the wikis to accept .dat files without complaining (and, if you want, to automatically assign them to Image: pages)? This is controlled by settings in LocalSettings.php - in enable-ww13.php on our system (included by LocalSettings.php to control WW settings).

to enable upload of .xyz files without a warning (whether using WW's import or the standard upload page):

$wgFileExtensions[] = 'xyz';

to import .xyz files to Image: pages rather than text pages, when using WW's import interface:

$wwImageExtensions[] = 'xyz';

[note, the latter will also cause .xyz to be displayed as images when they appear in pages. To import them as images but display them as text, use

$wwImportImageExtensions[] = 'xyz';]

[Lee Worden]

trying to reproduce, but oddly, having a hard time getting it to reject odd file extensions...

[Lee Worden]

OK, got it. For posterity: by setting

$wgCheckFileExtensions = true;

after including enable-ww13.php. When I use 'ignore warning and save file anyway' it returns to the original import form row, with nothing in the file input, but with the source filename and destination page set.

[Lee Worden]

Great, so what happens in the code?

The form row displays and "upload warning" text instead of its form fields. But there are a bunch of hidden inputs, including

<input type="hidden" value="yes.dbt" name="wpOrigFilename1">

Is that not sufficient? It loses track of where the actual file contents are stashed?

The file input is wpUploadFile1 though, so it looks like that's getting lost along the way.

Strangely (perhaps), there doesn't seem to be anywhere in SpecialImportProjectFiles that does anything with the value of wpUploadFile*. Maybe just inherited from SpecialCustomUpload?

Looks like SpecialCustomUpload's (i.e. SpecialUpload's) handling of this field is not so straightforward: it translates it immediately into some other fields:

$this->mTempPath       = $request->getFileTempName( "wpUploadFile$sfx" );
$this->mFileSize       = $request->getFileSize( "wpUploadFile$sfx" );
$this->mSrcName        = $request->getFileName( "wpUploadFile$sfx" );
$this->mCurlError      = $request->getUploadError( "wpUploadFile$sfx" );

Those values get stored in the session data, not in hidden form fields (to prevent forgery of file paths), and the session key is in the hidden fields:

<input type="hidden" value="1688515517" name="wpSessionKey1">

(though maybe I don't need to store the session key once for each row...)

UploadFormRow's constructor reads mTempPath and the rest of those from the session data, and ImportFormRow's constructor calls that constructor. So that seems like it ought to work...

[Lee Worden]

The "Ignore warning and save file anyway" checkbox is wpUpload1, which I think in the standard Upload interface poses as the regular Upload button. Here it's separate because there's only one Upload button for the page (wpUpload), while there's an ignore box per-row, hence the suffix 1. The per-row one gets its value stored in $this->mRowUploadClicked, which gets checked along with mIgnoreWarnings, at least at first glance.

I think I need to add some printfs, i.e. log some stuff...

[Lee Worden]

And... source forge has logged me out and destroyed another long comment that I accumulated over a series of explorations...

Firefox was choking on a series of JavaScript errors and not loading some of the scripts, I don't know why, and as a consequence not filling in the filename and destination page. Then it started working again. Fortunately I opened a separate ticket for most of that.

When I go through the original sequence - select a file with an "unwanted filetype", then click "Ignore warnings and upload anyway" - sometimes it imports successfully and sometimes it does what this ticket says.

I can compare the two cases in the logfile:
When it uploads successfully, at the time it receives the "Ignore warnings" selection it has a good value of $mTempPath:

mwrepo://local/temp/b/bf/20130309051045!1%21Sandbox%24yes.ddt

This is the location of the stashed file in the images directory, I think.

When it receives the ignore warnings selection but fails, it has mTempPath empty. Next is to find out why.

[Lee Worden]

I'm wondering if it might be that it does this successfully when the steps all happen in quick succession, and forgets the file when there's a significant delay between the one step and the next. This would be easy to explain, because the session data probably expires pretty quickly, and in that case, the page might act as if it has a file stashed, but not actually find it in the session data. I'll try to test for this.

[Lee Worden]

I should check whether it reports a useful message when the session data has expired. If it doesn't, and I fix that, then I can see whether the main problem here goes away.

[Lee Worden]

Hmm, SpecialCustomUpload checks

if( !empty( $this->mSessionKey ) &&
  isset( $_SESSION['wsUploadData'][$this->mSessionKey]['version'] ) &&
  $_SESSION['wsUploadData'][$this->mSessionKey]['version'] == CustomUploadForm::SESSION_VERSION ) {

and if that isn't satisfied, it uses whatever POST data it's got, which I think in the case we're talking about means it starts over as a fresh upload form. This might make sense in the regular Special:Upload case, but I think in the multiple-file case, it would be better to report an error and apologize. Also, in the ImportProjectFiles case, if the file input field is going to go blank, it should at least forget the other data as well. But obviously it should apologize instead.

[Lee Worden]

I've added a check for a session key but no session data. If this happens, it'll output an error message. So if the original file-forgetting problem happens without the error message appearing, I'll know that it's caused by something else.