WordPress.org

Make WordPress Core

Opened 19 months ago

Last modified 12 months ago

#39196 new defect (bug)

When saving large menus the JSON encoded string was not used. related to #14134

Reported by: budaned Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version: 4.5
Component: Menus Keywords: has-patch needs-testing
Focuses: javascript, administration Cc:

Description

This is a follow-up to #14134.

The solution proposed in #14134 is not properly working. This is because the values of the JSON string are replacing $_POST but later on $_REQUEST was being used. The only reason it seemed to still work is because the post action of the form was populating all the variables, but still failed when the apache, php or suhosin were reached.

In the proposed solution only the JSON string, menu ID and action are submitted by post and then $_POST and $_REQUEST are populated with the data.

Attachments (2)

patch.txt (1.1 KB) - added by budaned 19 months ago.
Patch for fix #14134
39196.diff (396 bytes) - added by ericlewis 12 months ago.

Download all attachments as: .zip

Change History (5)

@budaned
19 months ago

Patch for fix #14134

#1 @budaned
19 months ago

  • Summary changed from When saving largmenus the JSON encoded string was not used. related to #14134 to When saving large menus the JSON encoded string was not used. related to #14134

#2 @swissspidy
19 months ago

  • Keywords has-patch needs-testing added
  • Version changed from trunk to 4.5

Hey there,

Thanks for your report!

This sounds like a regression from 4.5. Pinging @ericlewis as he worked on the original solution.

@ericlewis
12 months ago

#3 @ericlewis
12 months ago

Hey @budaned,

thanks for this bug report! A few questions and notes.

The solution proposed in #14134 is not properly working. This is because the values of the JSON string are replacing $_POST but later on $_REQUEST was being used. The only reason it seemed to still work is because the post action of the form was populating all the variables, but still failed when the apache, php or suhosin were reached.

What do you mean the solution is not working? I can reliably reproduce the problem if I revert the change: in my dev environment only the first 83 items are saved because PHP truncates after 1000 POST values, and enabling the change allows me to save many more than that. Can you share steps to reproduce what you are seeing?

I do see that in a few places we use the $_REQUEST global to access form submitted data on the navigation menu page (1, 2, there are more spots) but I don't see one that would impact the saving of the menu items. The menu item fields are the last values in order in the POST request, so they are the only ones liable to be truncated. The function which extracts the menu item data from the request wp_nav_menu_update_menu_items gets the data from $_POST, and this is the only place in our codebase that touches these variables.

We can certainly set $_REQUEST with the same data, as that's the default behavior of PHP and developers may expect those values to exist, core or otherwise which I've incorporated from your submitted patch in attachment:39196.diff.

Note: See TracTickets for help on using tickets.