Opened 13 years ago
Closed 9 years ago
#20712 closed defect (bug) (wontfix)
Wrong data type returned from get_option for "page_on_front" after update_option
Reported by: | tszming | Owned by: | |
---|---|---|---|
Milestone: | Priority: | normal | |
Severity: | normal | Version: | 3.3.2 |
Component: | Options, Meta APIs | Keywords: | has-patch 2nd-opinion |
Focuses: | Cc: |
Description
echo gettype(get_option("page_on_front")); // return string update_option("page_on_front", "123"); echo gettype(get_option("page_on_front")); // return integer
You can repeat the steps and it give consistent result, so get_option
return the wrong data type ONLY when it is after update_option
Attachments (1)
Change History (8)
#2
@
13 years ago
The funniest thing is when you replace page_on_front
with another option such as blog_public
then it would be okay. (both return strings in my code above)
Anyway, since the datatype is changed, so when I execute the update_option
twice, e.g.
echo gettype(get_option("page_on_front")); // return string update_option("page_on_front", "123"); echo gettype(get_option("page_on_front")); // return integer update_option("page_on_front", "123");
Need to be careful as the following lines of code does not work
function update_option( $option, $newvalue ) { ... // If the new and old values are the same, no need to update. if ( $newvalue === $oldvalue ) return false;
#3
follow-up:
↓ 4
@
13 years ago
Some core options have pre-save sanitization hooks, most numeric core options have intval() as their sanitizer.
In general, WordPress treats all Identifiers as numeric strings, most functions will return either a numeric string, or a literal integer.
As a result, === shouldn't be used for comparison of core numeric ID's.
The ticket I linked to, is the opposite of this.. If you save an option with an Int value, it'll return as the numeric string representation.
#4
in reply to:
↑ 3
@
13 years ago
- Keywords has-patch added
Replying to tszming:
Need to be careful as the following lines of code does not work
Confirmed. In update_option()
, $newvalue
is passed through sanitize_option
:
http://core.trac.wordpress.org/browser/trunk/wp-includes/option.php?rev=20784#L227
When comparing $newvalue
with $oldvalue
, we apparently assume that $oldvalue
was also sanitized previously, but that doesn't work for numeric values, so we end up comparing a number with a string in line 232.
The funniest thing is when you replace page_on_front with another option such as blog_public then it would be okay. (both return strings in my code above)
blog_public
doesn't get sanitized by sanitize_option()
(which may be a separate bug), so the check is passed in that case.
Replying to dd32:
As a result, === shouldn't be used for comparison of core numeric ID's
Perhaps we should also check for sanitized $oldvalue
(20712.patch)? This doesn't seem to break any of our current unit tests.
#6
@
11 years ago
- Component changed from General to Options and Meta
- Keywords 2nd-opinion added
Sanitizing for $oldvalue seems pretty dangerous. Especially if the routine changed, this has the potential of comparing what end up being identical strings but really are not, thus preventing the DB from being updated and poisoning future get_option calls. I think we should probably wontfix this.
Duplicate of #7383
I do agree with the final resolution there, PHP doesn't care if it's an int or a string, both are interprated as a scalar and will be handled the same in any calculation.
Perhaps you can explain about the actual issue that you've run into? As a way of explaining to others what the real problem is?