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

[budaned]

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.

[budaned]

Patch for fix #14134

[swissspidy]

Hey there,

Thanks for your report!

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

[ericlewis]

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​.

[huzaifaalmesbah]

I attempted to apply the attached patch for testing, but ran into issues while applying it against current trunk. During the patch process, backup files such as the following were created:

• nav-menu.php.orig

This suggests the patch does not apply cleanly and is likely outdated.

Given the age of this ticket and the amount of changes in core since the patch was created, the patch likely needs to be refreshed against current trunk before meaningful testing can be performed.

Since the patch cannot currently be tested against trunk, I have removed the needs-testing keyword. Without a patch that applies cleanly to current core, testing is not meaningful.

Marking as needs-refresh.

Also adding reporter-feedback to confirm whether this issue is still reproducible in current WordPress or if it has already been resolved.