WordPress.org

Make WordPress Core

Opened 17 months ago

Closed 17 months ago

Last modified 17 months ago

#22757 closed defect (bug) (fixed)

load-scripts.php is receiving too long of a GET parameter

Reported by: nacin Owned by: nacin
Milestone: 3.5 Priority: high
Severity: blocker Version: 3.5
Component: Administration Keywords: has-patch needs-testing
Focuses: Cc:

Description

This was originally reported here: http://wordpress.org/support/topic/flyout-menus-35-rc12.

This works:

http://lamandrewsdevelopment2.com/la-wp-testing/wp-admin/load-scripts.php?c=1&load=admin-bar,hoverIntent,common,wp-ajax-response,jquery-color,wp-lists,quicktags,jquery-query,admin-comments,jquery-ui-core,jquery-ui-widget,jquery-ui-mouse,jquery-ui-sortable,postbox,dashboard,customize-base,customize-loader,thickbox,plugin-install,underscore,shortcode,media-upload,backbone,media-models,plupload,plupload-html5,plupload-flash,plupload-silverlight,plupload-html4,wp-plupload,media-views,media-editor,word-count,jquery-ui-resizable,jquery-ui-draggable,jquery-ui-button,jquery-ui-position,jquery-ui-

This does not:

http://lamandrewsdevelopment2.com/la-wp-testing/wp-admin/load-scripts.php?c=1&load=admin-bar,hoverIntent,common,wp-ajax-response,jquery-color,wp-lists,quicktags,jquery-query,admin-comments,jquery-ui-core,jquery-ui-widget,jquery-ui-mouse,jquery-ui-sortable,postbox,dashboard,customize-base,customize-loader,thickbox,plugin-install,underscore,shortcode,media-upload,backbone,media-models,plupload,plupload-html5,plupload-flash,plupload-silverlight,plupload-html4,wp-plupload,media-views,media-editor,word-count,jquery-ui-resizable,jquery-ui-draggable,jquery-ui-button,jquery-ui-position,jquery-ui-dialog,wpdialogs,wplink,wpdialogs-popup&ver=3.5-RC2-22928

Obviously, error reporting would normally be suppressed in load-scripts.php, but that'd only end up with a blank page and thus most of the JavaScript on the screen breaking. $_GET['load'] is being unset for length reasons. Why? I don't know yet. I can reproduce this exact length issue on my nacin.com server too, so going to dig there next.

Add one more character to the first URL and it will fail.

Attachments (4)

22757.diff (1.8 KB) - added by nacin 17 months ago.
22757.2.diff (1.5 KB) - added by nacin 17 months ago.
22757.2.2.diff (1.6 KB) - added by georgestephanis 17 months ago.
22757.3.diff (978 bytes) - added by georgestephanis 17 months ago.

Download all attachments as: .zip

Change History (12)

comment:1 nacin17 months ago

suhosin.get.max_value_length = 512 is the default. We need to chunk this.

nacin17 months ago

comment:2 nacin17 months ago

  • Keywords has-patch needs-testing added

22757.diff is a very ugly (and not at all efficient) patch that works.

comment:3 georgestephanis17 months ago

Shouldn't

if ( strlen( $concat ) + strlen( $script ) > 513 ) {

be > 511 as the comma isn't included between the two?

nacin17 months ago

georgestephanis17 months ago

georgestephanis17 months ago

comment:4 georgestephanis17 months ago

22753.3.diff is functionally the same as 22753.2.2.diff, but I didn't correct whitespace style around it to simplify the patch.

comment:5 ryan17 months ago

22757.2.diff looks good.

  • Turn off SCRIPT_DEBUG
  • Load an admin page
  • Look at load-scripts.php requests
  • Verify proper chunking. No off by one, no lists out of order.
  • Console is error free
  • All the js goodness seems to be present and working

comment:6 devesine17 months ago

22757.2.diff looks good to me; no missing or out-of-order scripts with some of our script-heavier plugins.

comment:7 follow-up: nacin17 months ago

  • Owner set to nacin
  • Resolution set to fixed
  • Status changed from new to closed

In 23074:

Script loader: Chunk the script names as passed to load-scripts.php into 128-character pieces. Avoids hitting a limit for the length of a single variable, such as suhosin.get.max_value_length which defaults to 512. fixes #22757.

comment:8 in reply to: ↑ 7 westi17 months ago

Replying to nacin:

In 23074:

Script loader: Chunk the script names as passed to load-scripts.php into 128-character pieces. Avoids hitting a limit for the length of a single variable, such as suhosin.get.max_value_length which defaults to 512. fixes #22757.

Looks good to me too.

Note: See TracTickets for help on using tickets.