Actions will not be executed when post_max_size is exceeded.

[aszepeshazi]

When a http post request was made with a total size > php post_max_size, the called action doesn't have a chance to handle this exception gracefully. Instead the core action handler will redirect the user and display the message "Form is missing token or ts fields", which is rather confusing for the end user.

The reason for this is, quoting from php manual: "If the size of post data is greater than post_max_size, the $_POST and $_FILES superglobals are empty." Meaning also no elgg_token and elgg_ts values.

Plugins should have the possibility to gracefully handle this event.

[brettp]

Rels #2602 - Add a plugin hook when action tokens fail.

[ewinslow]

#2237 takes care of this?

[cash]

We still need to add code to handle this condition and display a useful message to the user

[mrclay]

I suggest this check occur in validate_action_token on ​this line. Something like:

if ($_SERVER['REQUEST_METHOD'] === 'POST' && empty($_POST) && $_SERVER['CONTENT_LENGTH'] > 500) {
    // @todo trigger a plugin hook here?
    register_error(elgg_echo('actiongatekeeper:exceededpostlimit'));
} else {
    register_error(elgg_echo('actiongatekeeper:missingfields'));
}

[brettp]

Why not use ini_get('post_max_size') instead of 500?

[mrclay]

Replying to brettp:

Why not use ini_get('post_max_size') instead of 500?

True, although if CONTENT_LENGTH > 0, there should be something in POST/FILES, so maybe a limit check isn't required.
Does Elgg already have a function to turn "15M"/etc into ints?

Should we assume all uploads will go through the action system? I.e. should this check be done during bootup and set some kind of global flag?

[brettp]

True, although if CONTENT_LENGTH > 0, there should be something in POST/FILES, so maybe a limit check isn't required.

AFAIK, CONTENT_LENGTH is only sent with POST, so that's true. Will need to confirm that.

Does Elgg already have a function to turn "15M"/etc into ints?

elgg_get_ini_setting_in_bytes()

Should we assume all uploads will go through the action system? I.e. should this check be done during bootup and set some kind of global flag?

If uploads aren't going through the action system I think the plugin should be expected to handle errors itself.

[brettp]

@mrclay - Were you working on this as a PR or just the code you have in the comments?

[mrclay]

I haven't started work on it. I should have time to send a PR this weekend.

[brettp]

np and thanks. Throw it back over to unassigned if you don't have time.

[mrclay]

PR: ​https://github.com/Elgg/Elgg/pull/222
Let me know if I need to re-apply against master.

[ewinslow]

No, this counts as a bug fix and so can go into 1.8.x, as the milestone reflects.

[Steve Clay]

Fixes #2411: Show message when upload exceeds PHP limits, and show limits in Server Info

Changeset: 7c0215018897bfb578e6116ef9d36e5edc1fe5cf

[Cash Costello]

Merge pull request #222 from mrclay/issue2411

Fixes #2411: Show message when upload exceeds PHP limits

Changeset: 4abe9e6727108efd1f834a80a7942ed92cae2a52

[Cash Costello]

Refs #2411 removed <code> in warning as it was causing a display issue in server info table

Changeset: b43e6c7d5550aef3d319148de37fa49cadc33855

[Steve Clay]

Fixes #2411: Show message when upload exceeds PHP limits, and show limits in Server Info

Changeset: 7c0215018897bfb578e6116ef9d36e5edc1fe5cf

[Cash Costello]

Merge pull request #222 from mrclay/issue2411

Fixes #2411: Show message when upload exceeds PHP limits

Changeset: 4abe9e6727108efd1f834a80a7942ed92cae2a52

[Cash Costello]

Refs #2411 removed <code> in warning as it was causing a display issue in server info table

Changeset: b43e6c7d5550aef3d319148de37fa49cadc33855