Opened 4 years ago
Last modified 2 years ago
#51486 new defect (bug)
The add_option function should not be able to update existing rows in the database.
Reported by: | khag7 | Owned by: | |
---|---|---|---|
Milestone: | Awaiting Review | Priority: | normal |
Severity: | normal | Version: | 2.9 |
Component: | Database | Keywords: | needs-patch needs-unit-tests |
Focuses: | Cc: |
Description
In certain edge cases, add_option
is able to update existing option values. This should not be possible. If SQL is executed which instructs "Insert XYZ" into database, then the insert should fail if the key already exists in the database, unless the SQL specifies that an UPDATE should happen.
The add_option
function does check first to see if an option exists before attempting to add it to the database. In the overwhelming majority of cases, this works fine because when the option exists the function will return early before attempting to insert into the database.
However, consider a scenario where an object cache is being used. And further consider that the object cache may report incorrectly. If the object cache tells add_option
that the value doesn't exist in the database (but in reality it does!), then add_option
continues on and attempts to insert the supposedly new option.
In that particular circumstance, the SQL query, shown below, is executed which will insert the new row. And in the event that the option already exists, it will be overwritten.
The issue here is that add_option
shouldn't be able to update the existing value. The query should fail. Why is the ON DUPLICATE KEY UPDATE
clause included here?
<?php // Code here is from wp-includes/option.php Line 581: $result = $wpdb->query( $wpdb->prepare( "INSERT INTO `$wpdb->options` (`option_name`, `option_value`, `autoload`) VALUES (%s, %s, %s) ON DUPLICATE KEY UPDATE `option_name` = VALUES(`option_name`), `option_value` = VALUES(`option_value`), `autoload` = VALUES(`autoload`) ", $option, $serialized_value, $autoload ) );
I realize this is an edge case. If the object cache being used gives bad information to add_option
then really its the object caching plugin at fault. However, I can't understand a possible circumstance where add_option
should ever UPDATE an existing option. The SQL here should not include ON DUPLICATE KEY UPDATE
. Am I missing something? Is there a good reason for that clause? What would happen if that clause were removed?
And now for a little history...
It was 11 years ago, and
add_options
was located inwp-includes/functions.php
back then.The clause
ON DUPLICATE KEY UPDATE
was added to theadd_option
function in ticket #11437 and commit [12403]The description in that ticket does explain why this clause exists in the SQL. On a busy site, you can't know whether the option exists or not before attempting to insert it. As such, an
INSERT
query may fail.Unfortunately, they solved that ticket incorrectly. The solution should have been that whatever piece of code was attempting to add the option
site_transient_timeout_theme_roots
should have been usingupdate_option
instead. This would've given them the result they wanted. Instead, they patchedadd_option
withON DUPLICATE KEY UPDATE
and now we have a function that is intended to add and never update, but which in fact does update.