Page MenuHomePhabricator

GWToolset XML upload fails with “The file that was uploaded exceeds the upload_max_filesize and/or the post_max_size directive in php.ini” on hhvm 3.6
Closed, ResolvedPublic

Description

This bug was initially reported to me by folks of United Archives GmbH.

GWToolset upload an XML fails with “The file that was uploaded exceeds the upload_max_filesize and/or the post_max_size directive in php.ini.”, even though the XML (joined) is a tiny test with 3 records and 6KB.

I also tried with another XML that I used on the Beta cluster a couple of weeks ago, which fails too.

On the other hand, the same XML passes on Wikimedia Commons.

Steps to reproduce :

Event Timeline

JeanFred created this task.Apr 28 2015, 1:58 PM
JeanFred updated the task description. (Show Details)
JeanFred raised the priority of this task from to Needs Triage.
JeanFred added a subscriber: JeanFred.
Restricted Application added a project: Multimedia. · View Herald TranscriptApr 28 2015, 1:58 PM
Restricted Application added a subscriber: Aklapper. · View Herald Transcript

Relavent check that its failing:

public static function checkContentLength() {
        if ( isset( $_SERVER["CONTENT_LENGTH"] )
                && ( (int)$_SERVER["CONTENT_LENGTH"] > Utils::getBytes( ini_get('post_max_size') )
                        || (int)$_SERVER["CONTENT_LENGTH"] > Utils::getBytes( ini_get('upload_max_filesize') ) )
        ) {
                throw new GWTException( 'gwtoolset-over-max-ini' );
        }
}

I'm not sure that's such a good check. If the file was too big it would probably be better to look at what php says about the uploaded file. Perhaps this check should be removed.

Also, on beta (and on normal commons) Special:Upload is reporting file size limit of 1000 MB. It should detect that max_post_size is only 100 MB and say max file size is 100 MB. Perhaps hhvm is no longer exposing ini_get('post_max_size').

Also, on beta (and on normal commons) Special:Upload is reporting file size limit of 1000 MB. It should detect that max_post_size is only 100 MB and say max file size is 100 MB. Perhaps hhvm is no longer exposing ini_get('post_max_size').

Ah, appears to be https://github.com/facebook/hhvm/issues/4993 . Anyways, gwtoolset should probably be checking this differently.

Change 207329 had a related patch set uploaded (by Brian Wolff):
Check filesize limit directly from PHP $_FILES

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

[I should note that my patch is based on a guess of what the problem is. I didn't reproduce the bug locally [too lazy to go to the trouble of installing that particular version of hhvm locally], but I feel pretty confident that this is what the issue is]

Bawolff renamed this task from GWToolset XML upload fails with “The file that was uploaded exceeds the upload_max_filesize and/or the post_max_size directive in php.ini” on Commons Beta to GWToolset XML upload fails with “The file that was uploaded exceeds the upload_max_filesize and/or the post_max_size directive in php.ini” on beta cluster (due to hhvm 3.6).Apr 29 2015, 12:18 AM
Bawolff added a project: HHVM.
Bawolff set Security to None.

Also note, the reason gwtoolset still works on real commons, is real commons is only on hhvm 3.3.1

greg added subscribers: Joe, greg.Apr 29 2015, 5:07 AM

Also note, the reason gwtoolset still works on real commons, is real commons is only on hhvm 3.3.1

@Joe FYI ^^

greg renamed this task from GWToolset XML upload fails with “The file that was uploaded exceeds the upload_max_filesize and/or the post_max_size directive in php.ini” on beta cluster (due to hhvm 3.6) to GWToolset XML upload fails with “The file that was uploaded exceeds the upload_max_filesize and/or the post_max_size directive in php.ini” on hhvm 3.6.Apr 29 2015, 5:08 AM
hashar added a subscriber: hashar.Apr 29 2015, 7:59 AM

[I should note that my patch is based on a guess of what the problem is. I didn't reproduce the bug locally [too lazy to go to the trouble of installing that particular version of hhvm locally], but I feel pretty confident that this is what the issue is]

For what it is worth, the Jenkins slaves have HHVM from apt.wikimedia.org which should be what is deployed in production. Currently that is 3.3.1+dfsg1-1+wm3.1.

The extensions do not have hhvm tests yet, but they can be manually triggered by commenting on a change check experimental. So in theory a test would let you reproduce it.

In T97415#1244401, @hashar wrote:

! In T97415#1243891, @Bawolff wrote:

[I should note that my patch is based on a guess of what the problem is. I didn't reproduce the bug locally [too lazy to go to the trouble of installing that particular version of hhvm locally], but I feel pretty confident that this is what the issue is]

For what it is worth, the Jenkins slaves have HHVM from apt.wikimedia.org which should be what is deployed in production. Currently that is 3.3.1+dfsg1-1+wm3.1.

Affected version is 3.6 (what is on beta). Issue does not affect production. But its cool to know its possible to do that.

That said I didnt write a test, since I removed the original method, and I dont know how to write a test that would test this properly given the dependence on how php handles uploaded files.

Hi,

I just tried another test upload on Beta and i am still getting the same message. Do we know when this problem is likely to be resolved?

Thanks

Jason Evans (Wikipedian in Residence at National Library of Wales)

Once the status field of https://gerrit.wikimedia.org/r/#/c/207329/ goes to "merge" the issue will be fixed

(Note to self: removing warning when this is merged)

Change 207329 merged by Brian Wolff:
Check php max_file_size limit directly from PHP $_FILES

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

Bawolff closed this task as Resolved.May 11 2015, 4:06 PM
Bawolff claimed this task.

Should work again

Confirmed. Thanks @Bawolff!

Dzahn reopened this task as Open.May 13 2015, 12:53 AM
Dzahn added a subscriber: Dzahn.

It's being reported again.

17:51 < andrewbogott> "You can only upload files with a size of up to -1 B. You tried to upload a file that is 133 KB"
17:51 < andrewbogott> Tried with two different browsers

17:51 < legoktm> I thought that was fixed? https://phabricator.wikimedia.org/T97415
17:52 < legoktm> hmm, that wasn't backported

17:52 < bd808> I'm getting it too

17:56 < legoktm> well I think bawolff's fix needs to be backported
17:57 < legoktm> but that's only gwtoolset

Change 210629 had a related patch set uploaded (by Legoktm):
Check php max_file_size limit directly from PHP $_FILES

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

Change 210630 had a related patch set uploaded (by Legoktm):
Check php max_file_size limit directly from PHP $_FILES

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

Dzahn added a comment.May 13 2015, 1:06 AM

18:02 < legoktm> mutante: the existing bug is about GWToolset, a different uploading tool
18:03 < mutante> legoktm: but it sounds like the same root cause?
18:03 < legoktm> yeah, but the fix will only fix GWToolset
18:04 < bd808> legoktm: so we need a patch to upload/UploadFromFile.php, specials/SpecialUpload.php and WebRequest.php too apaprently
18:04 < legoktm> I'maxPhpUploadSize' => min(
18:04 < legoktm> IIwfShorthandToInteger( ini_get( 'upload_max_filesize' ) ),
18:04 < legoktm> IIwfShorthandToInteger( ini_get( 'post_max_size' ) )
18:04 < legoktm> I),
18:04 < mutante> hmm, so reopening ticket about gwtoolset is right but additionally a new one for uploadwizard?
18:04 < legoktm> probably
18:04 < legoktm> so UW needs a similar fix

Change 210630 merged by Legoktm:
Check php max_file_size limit directly from PHP $_FILES

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

Dzahn triaged this task as High priority.May 13 2015, 1:17 AM

Change 210629 merged by Legoktm:
Check php max_file_size limit directly from PHP $_FILES

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

Change 210634 had a related patch set uploaded (by Legoktm):
Check php max_file_size limit directly from PHP $_FILES

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

Change 210634 merged by Legoktm:
Check php max_file_size limit directly from PHP $_FILES

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

Legoktm closed this task as Resolved.May 13 2015, 1:24 AM
Legoktm added a subscriber: Legoktm.

Backported+deployed

TheDJ added a subscriber: TheDJ.Jun 16 2015, 9:54 AM

The root cause for this has apparently been fixed and released upstream now.