Page MenuHomePhabricator

The upload form only checks upload_max_filesize, not post_max_size
Closed, ResolvedPublic

Description

The upload form only checks upload_max_filesize, not post_max_size; it should use the smaller of the two.


Version: 1.15.x
Severity: minor

Details

Reference
bz18411

Event Timeline

bzimport raised the priority of this task from to Low.Nov 21 2014, 10:33 PM
bzimport set Reference to bz18411.
bzimport added a subscriber: Unknown Object (MLST).

A fix

attachment uploadsize.patch ignored as obsolete

Patch looks okay at a glance. I'd have to test it and switch branches to commit it, though.

ste.coda wrote:

A second fix

I've corrected the Patch by adding two trim functions before ini_get (line 26-27 of the patch), removing an useless parenthesis on line 29 and removing a double semicolon on line 39.

Attached:

Applied in r50706. Thanks.

brion added a comment.Apr 4 2011, 8:43 PM

This has regressed somewhere in the meantime; the current code in SpecialUpload.php only checks upload_max_filesize.

brion added a comment.Apr 4 2011, 8:45 PM

Note that UploadFromFile::verifyUpload() does check both upload_max_filesize and post_max_size, but it doesn't seem to trigger properly -- I see no error message instead of seeing a verification failure error.

The limits output for the special page UI and the API should check both settings and return the minimum (actually the max file size would be a bit _smaller_ than the max post size, since a little space will be used for other form variables, but that's a start!)

sumanah wrote:

Adding "reviewed" since Stefano's patch was reviewed and, indeed, committed. cc'ing NeilK since he was recently talking/asking about size limits in uploading.

Jdforrester-WMF moved this task from Untriaged to Backlog on the Multimedia board.Sep 4 2015, 5:58 PM
Restricted Application added subscribers: Steinsplitter, Matanya, Aklapper. · View Herald TranscriptSep 4 2015, 5:58 PM
matmarex closed this task as Resolved.Apr 24 2017, 11:51 AM
matmarex added a subscriber: matmarex.

This was fixed again in 05c27c3af122b57f896927e5092bd25a9d8808d3 (in 2012). And since ed6648427a86d6893deb219081a96e337a1141cb, the logic is centralized in UploadBase.