Opened 8 months ago

Last modified 8 months ago

#21989 new defect (bug)

update_option() calls sanitize_option() twice when option does not exist

Reported by: MikeSchinkel Owned by:
Priority: normal Milestone: Awaiting Review
Component: Administration Version:
Severity: normal Keywords: needs-patch dev-feedback 2nd-opinion
Cc: mike@…

Description

I just spent several hours tracking down an issue when using the Settings API where sanitize_option() is called twice which is unnecessary execution especially if sanitization includes calling an external API for username/password authorization (this was how another developer set it up for a plugin I was debugging.)

What happens is that a call to update_option() will call sanitize_option() and then if the option wasn't previously in the options table update_option() will delegate to add_option() which calls santize_option() a second time. This would normally be easy to workaround by first calling get_option() and testing for false and calling add_option() instead of update_option() if false, but not when the Settings API chooses how to call it.

I've looked at the problem and can envision several different ways to solve it such but don't know which the core developers would choose (or if they'd choose yet another option I haven't envisioned) so I didn't submit a patch:

  • Adding a 3rd parameter $mode to sanitize_option() that identifies the mode ('add', 'edit', default = 'unknown') and thus allow the hook to ignore one of the options (this would be more backward compatible but would put the onus on the developer to know to do this),
  • Adding a 5th parameter $bypass_sanitize to add_option() defaulted to false that is only passed as true in update_option() allowing update_option() to disable the call to sanitize_option() found in add_option() (this would be less backward compatible and hacky, but seemless to the developer)
  • Adding a 3rd parameter $bypass_sanitize to update_option() defaulted to false that is only passed as true from /wp-admin/options.php allowing update_option() to bypass the call to sanitize_option() if an add_option() will happen (this would be less backward compatible and hacky, but seemless to the developer)
  • Have /wp-admin/options.php test for no pre-existing option and then call add_option() or update_option(), respectively (this would be seemless to the developer but less backward compatible and not hacky, and it wouldn't handle the general case.)

So is this worth fixing to save other users and developers debugging time, and to reduce the chance of subtle errors in new code? If yes, how best to approach?

Change History (3)

  • Summary changed from update_option('example') calls sanitize_option('example') twice when false===get_option('example') to update_option() calls sanitize_option() twice when option does not exist

As a quick update about how I worked around what was causing the problem, I moved the API authentication code into admin_init() and read and updated $_POST directly making sure to only do so on an update for this one plugin's admin settings page.

Just FYI for anyone else who might have the same issue prior to this getting fixed in core or if it does not get fixed in core.

I imagine the issue here is specifically related to the saving of a settings screen, and not usage of update_option() at the API level. In that case, it seems like the onus is on the developer to ensure such an option is initialized in the database. There are two schools of thought, but an option like this should certainly be initialized if that kind of side effect could occur.

If there were any possible route to an elegant solution here, I probably wouldn't be suggesting essentially a wontfix. But there isn't one, alas.

Note: See TracTickets for help on using tickets.