Make WordPress Core

Opened 14 years ago

Closed 9 years ago

#17856 closed enhancement (duplicate)

magic_quotes_gpc future-proof enhancements

Reported by: troydavisson's profile troydavisson Owned by:
Milestone: Priority: normal
Severity: normal Version:
Component: Bootstrap/Load Keywords: has-patch dev-feedback
Focuses: Cc:

Description

As is well documented across the Internet, the magic_quotes_gpc feature is going away in future versions of PHP. WordPress has historically automatically escaped _GET, _POST, _REQUEST and _COOKIE input from users, even if the server doesn't have magic_quotes_gpc turned on. Regardless of the reasons for this, having a way to move forward seems absolutely necessary.

Current issues related to this include (among others):

  • maintaining backwards compatibility for those plugin developers who depend on WordPress handling this escaping for them
  • giving plugin developers a way to help put magic_quotes_gpc in the past
  • giving developers access to the original super globals
  • making these super global values read-only so that poorly written plugins/themes don't cause conflicts and problems for other plugins/themes

Attached is a patch which I believe handles this effectively without causing any backwards compatibility issues.

This patch introduces 5 new getter functions for wordpress:

  • wp_input_get()
  • wp_input_post()
  • wp_input_get_post()
  • wp_input_cookie()
  • wp_input_server()

When WordPress first loads, these 5 functions grab the original copies of their respective super globals, undo magic_quotes if it's turned on and then makes the values accessible in a read-only way.

Moving forward, plugin developers can be encouraged to use, for example, wp_input_get('name') rather than $_GETname? . In addition to giving developers a migration path away from the forced magic_quotes_gpc behavior, additional security filters could be done on the given values for further protection.

Attachments (1)

magic_quotes.diff (4.0 KB) - added by troydavisson 14 years ago.

Download all attachments as: .zip

Change History (9)

#1 @troydavisson
14 years ago

  • Cc troy.davisson@… added

#2 @johnbillion
14 years ago

In your patch, calling each of the wp_input_*() functions once (from wp_input_init()) just to populate their static variables seems pointless. Why not populate the static variable the first time each function is called?

There should be a way to access a deep value of a multidimensional array. I've seen CMSes and frameworks that allow a pseudo array key notation in the form of

wp_input_post( 'foo[bar][baz]' )

and others that allow for an arbitrary number of arguments in the form of

wp_input_post( 'foo', 'bar', 'baz' );

in order to access $_POST['foo']['bar']['baz'].

#3 @troydavisson
14 years ago

John, thanks for your feedback. I realize now that I didn't do a great job of explaining the my rationale for how I put together the code.

The attached patch/diff adds code to 2 primary files. The change to wp-settings.php allows for those functions to pre-load the super global data into static arrays. I jumped back and forth between using basic functions or if I should extend the WP class. In the end, using regular functions made the most sense (although it made the code in those functions a bit ugly). The new "wp_input_init()" function is called in wp-settings.php just after the formatting.php file is included (due to the dependency on the stripslashes_deep function) but before "wp_magic_quotes()" is called which is where WordPress goes through and force changes the values of these super globals.

So, populating the static variables ahead of time allows those functions to grab the original values of those variables:

  1. prior to WordPress force-adding magic quotes (the behavior this improvement is supposed to help phase out within WordPress over time)
  2. prior to plugins being loaded which have the ability to alter the values within those arrays. I've run into numerous cases in the past few months where a plugin installed on a WordPress site has either overridden a $_GET variable, completely unset $_GET or has manually overridden one of the $_SERVER values (like the User-Agent)

To summarize, this patch is intended to allow addon developers the access to a read-only version of the original values.

Great suggestion regarding the nested values. I hadn't considered that use-case but I agree that it makes sense. I'll upload an updated patch that covers that.

#6 @nacin
11 years ago

  • Component changed from General to Bootstrap/Load

#7 @chriscct7
9 years ago

  • Keywords dev-feedback added

#8 @johnbillion
9 years ago

  • Milestone Awaiting Review deleted
  • Resolution set to duplicate
  • Status changed from new to closed

This is sufficiently a dupe of #22325 which has more discussion and similar patches.

Note: See TracTickets for help on using tickets.