Make WordPress Core

Opened 10 years ago

Last modified 13 months ago

#25939 reviewing defect (bug)

add_options_page(..., 'options.php') and 1000 vars limit

Reported by: tivnet's profile tivnet Owned by: sergeybiryukov's profile SergeyBiryukov
Milestone: Awaiting Review Priority: normal
Severity: normal Version:
Component: Administration Keywords: has-patch 2nd-opinion dev-feedback
Focuses: Cc:

Description

(Related to the discussion in #14134)

There is a "common knowledge" in the wilderness, suggesting having this for seeing all options at once, and I guess many admins have that enabled:

add_options_page('All Settings', 'All Settings', 'administrator', 'options.php');

At some point, the list grows to more than 1,000 variables, and:

PHP Warning: Unknown: Input variables exceeded 1000.
To increase the limit change max_input_vars in php.ini.
in Unknown on line 0

The option table is corrupted then. In the best scenario, the tail of the table is occupied with transients. The worst - real settings.

(having a separate table for transients sounds like a good idea to me, BTW)

Now, to the point: the "All Settings" panel should NOT have the [Save] button. It must be read-only.

Alternatively - AJAX save, similar to phpMyAdmin - for each option separately.

Attachments (1)

core.25939.0.patch (6.2 KB) - added by bordoni 8 years ago.

Download all attachments as: .zip

Change History (7)

#1 @kovshenin
10 years ago

  • Cc kovshenin added

#2 @dd32
9 years ago

In the best scenario, the tail of the table is occupied with transients.

This angle is mostly mitigated by the fact that the fields which are marked as disabled/readonly (as fields with serialised data in them are) are not submitted to a server in a POST request.

Since the all-options page is mostly a hidden power-tool, an option may be to simply disable the submit process when the number of active fields exceeds that of max_input_vars.
Another option would be to simply use JS to only send changed fields to the server.

#3 @bordoni
8 years ago

  • Keywords has-patch 2nd-opinion added

I've worked on this as a possible solution using a JavaScript to only send the value of the field in case it was changed.

It was the first patch I changed that much.

#4 @SergeyBiryukov
4 years ago

  • Milestone set to Awaiting Review
  • Owner set to SergeyBiryukov
  • Status changed from new to reviewing

#5 @ramon fincken
15 months ago

  • Keywords dev-feedback added
  • Version set to trunk

Ok so I found this ticket in search of something that appears quite similar.

To be precise =>
In the current setup op options.php

A) It is possible to modify the DOM for serialized data. The POST server check is non-present upon processing of a POST request

B) It is possible that you modify a single value whilst the site has updated another option due to normal site ( or plugin ) behavior.
Note that the patch provided here would fix this.

C) In the current setup the POST server will update ALL options it is presented by the form. Why not check what is changed in the form ? Yes it is a minor performance improvement and yes only on the admin option pages, but I like this.
Note that the patch provided here would fix this.

D) It might be worth commenting in code that the nonce is actually verified using check_admin_referer

So, now what? @SergeyBiryukov , @bordoni (thanks for your patch!)

#6 @hellofromTonya
13 months ago

  • Version trunk deleted

Removing trunk as the Version as trunk is currently 6.2.0 at the time of this comment.

Note: See TracTickets for help on using tickets.