Delete query can potentially remove unintended variables [#2831476]

The structure of the query in the hook_uninstall function could delete unintended variables from the variable table.

function fb_uninstall() {
  // Variables prefixed with double underbar to avoid conflicts with other modules named fb_something.
  db_query("DELETE FROM {variable} WHERE name LIKE 'fb__%'");
}

The underscore is a wildcard character in this scenario, so the query will delete the following variables:

fb_var_to_delete
fbconnect_key
fbtoken

The underscore character should be escaped, or the db_like() function should be used.

Documentation on db_like details the underscore as a wildcard issue https://api.drupal.org/api/drupal/includes%21database%21database.inc/fun...

Comment	File	Size	Author
#3	2831476-escape-wildcard-character-in-delete-query.patch	492 bytes	bfodeke

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

29 November 2016 at 15:44

bfodeke created an issue. See original summary.

Comment #2

bfodeke CreditAttribution: bfodeke at Mediacurrent commented 29 November 2016 at 15:47

Comment #3

bfodeke CreditAttribution: bfodeke at Mediacurrent commented 29 November 2016 at 16:04

Status:

Active

» Needs review

File	Size
2831476-escape-wildcard-character-in-delete-query.patch	492 bytes

Comment #4

shrop CreditAttribution: shrop at Mediacurrent commented 15 December 2016 at 19:33

Description

I did some testing and I think the db_like() statement is not working. It appears to still delete any vars with a single underscore, which isn't right (ex: fb_test would be deleted). I wonder if just using db_query() with \_ to escape the underscores in the WHERE clause would work.

Testing Instructions

Install patch in fb 7.x-4.x
Enable the fb module
Check for fb__ vars in the variables table (fb__json_bigint should exist on install)
Add a new row to the variables table with the "name" "fb_test"
Disable the module
Uninstall the module
Check the variables table
Expected results: fb__json_bigint should be removed and fb_test should remain
Actual results: fb__json_bigint and fb_test are both deleted

Next Steps

Consider using db_query() with escaped underscores in the WHERE clause

Comment #5

bfodeke CreditAttribution: bfodeke at Mediacurrent commented 15 December 2016 at 20:04

Hey @shrop, I wasn't too clear on what my patch is fixing. It's not going to prevent removal of any variables with the fb_ prefix. It *will* however prevent variables that begin with fb from being deleted.

So my patch will ensure:

fb_keys <- will get deleted
fbconnect <- will NOT get deleted

Testing Instructions

Install patch in fb 7.x-4.x
Enable the fb module
Check for fb_ vars in the variables table (fb__json_bigint should exist on install)
Add a new row to the variables table with the "name" "fbtest"
Disable the module
Uninstall the module
Check the variables table
Expected results: fb__json_bigint should be removed and fbtest should remain

Comment #6

shrop CreditAttribution: shrop at Mediacurrent commented 17 December 2016 at 02:32

@bayo, that make sense!

I ran the test and confirm that your patch resolves the issue and works as expected following the instructions below.

Description

Testing Instructions

Install patch in fb 7.x-4.x
Enable the fb module
Check for fb__ vars in the variables table (fb__json_bigint should exist on install)
Add a new row to the variables table with the "name" "fbtest"
Disable the module
Uninstall the module
Check the variables table
Expected results: fb__json_bigint should be removed and fbtest should remain
Actual results: fb__json_bigint should be removed and fbtest should remain