WordPress.org

Make WordPress Core

Opened 4 years ago

Last modified 8 months ago

#13575 new defect (bug)

wp_max_upload_size() is not taking memory limit into account

Reported by: hakre Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version: 3.0
Component: Upload Keywords: has-patch
Focuses: Cc:

Description

wp_max_upload_size() is not taking a memory limit into account while checking for the maximum upload size.

according to the official php documentation the memory limit is limiting file uploads as well (and post form operations).

post_max_size - integer

Sets max size of post data allowed. This setting also affects file upload. To upload large files, this value must be larger than upload_max_filesize. If memory limit is enabled by your configure script, memory_limit also affects file uploading. Generally speaking, memory_limit should be larger than post_max_size.

Source

Attachments (5)

13575.patch (734 bytes) - added by hakre 4 years ago.
13575.php (558 bytes) - added by hakre 3 years ago.
ini_set(), ini_get() and ini_restore() php example
13575.2.patch (871 bytes) - added by hakre 3 years ago.
Updated patch reflecting original php.ini memory_limit value and only if memory_limit is active. Containing a filter.
13575.3.patch (839 bytes) - added by hakre 3 years ago.
Updated patch reflecting original php.ini memory_limit value and only if memory_limit is active. W/o a filter.
13575.4.patch (842 bytes) - added by hakre 3 years ago.
Better compare against -1 for no memory limit.

Download all attachments as: .zip

Change History (13)

hakre4 years ago

comment:2 hakre4 years ago

  • Keywords has-patch added
  • Milestone changed from Unassigned to Future Release

It might be better to name the filter upload_memory_limit, so take it as suggestion only - if filtering that is even considered.

comment:3 hakre4 years ago

Related: #13418

comment:4 follow-up: dd324 years ago

A filter is not needed here, However, the ini_get() may return incorrect information, It may have been upped already by something such as WP_MEMORY_LIMIT.

hakre3 years ago

ini_set(), ini_get() and ini_restore() php example

comment:5 hakre3 years ago

Replying to dd32:

I added a demo script which outputs the following (Fedora 13):

initial value: "128M"
new value: "42M"
old value: "128M"
current value: "42M"
original value: "128M"

So the original value should be taken into account as that's the one that comes into play at times of the upload procedure.

hakre3 years ago

Updated patch reflecting original php.ini memory_limit value and only if memory_limit is active. Containing a filter.

hakre3 years ago

Updated patch reflecting original php.ini memory_limit value and only if memory_limit is active. W/o a filter.

hakre3 years ago

Better compare against -1 for no memory limit.

comment:6 in reply to: ↑ 4 hakre3 years ago

Replying to dd32:

A filter is not needed here, However, the ini_get() may return incorrect information, It may have been upped already by something such as WP_MEMORY_LIMIT.

I've uploaded some new patches that reflect both concerns, the filter and the original value. This still needs some tests (e.g. what happens if 64 MB are consumed but setting get's restored to 16 MB original value - would that do a fatal error?) but tries to address accessing the original settings value.

comment:7 hakre3 years ago

Did run some tests now, lowering the memory_limit configuration value while having more memory in use than the set value will not fatal error. Method in patch looks save.

comment:8 avryl8 months ago

  • Component changed from General to Upload
Note: See TracTickets for help on using tickets.