Opened 12 months ago

Last modified 7 months ago

#20712 new defect (bug)

Wrong data type returned from get_option for "page_on_front" after update_option

Reported by: tszming Owned by:
Priority: normal Milestone: Awaiting Review
Component: General Version: 3.3.2
Severity: normal Keywords: has-patch
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)

20712.patch (529 bytes) - added by SergeyBiryukov 12 months ago.

Download all attachments as: .zip

Change History (6)

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?

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;
  

comment:3 follow-up: ↓ 4   dd3212 months 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.

comment:4 in reply to: ↑ 3   SergeyBiryukov12 months 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.

Note: See TracTickets for help on using tickets.