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...

CommentFileSizeAuthor
#3 2831476-escape-wildcard-character-in-delete-query.patch492 bytesbfodeke

Comments

bfodeke created an issue. See original summary.

bfodeke’s picture

bfodeke commented
bfodeke’s picture

bfodeke commented
Status: Active » Needs review
StatusFileSize
new 2831476-escape-wildcard-character-in-delete-query.patch492 bytes
shrop’s picture

shrop commented

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

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

Next Steps

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

bfodeke’s picture

bfodeke commented

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:

  1. fb_keys <- will get deleted
  2. fbconnect <- will NOT get deleted

Testing Instructions

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

shrop commented

@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

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

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

Next Steps

Commit to the 7.x-4.x branch

shrop’s picture

shrop commented
Status: Needs review » Reviewed & tested by the community
damienmckenna’s picture

damienmckenna
Location TN, USA
commented
Assigned: bfodeke » Unassigned

Don't forget to unassign the issue after you upload a patch.