uc_file bug regarding multiple users purchasing files

Strange, I thought that could only happen if somehow they would have had the same file_key generated, but that's pretty unlikely (although I suppose, possible). I'll get this fixed very soon!

Really bad bug!

Untested fix attached.

Patch fixes the problem on our site.

After applying the patch I fixed our existing file orders by running the following php (I used batches of 100 orders and changed the WHERE statement each time):

$result = db_query_range("SELECT order_id FROM {uc_orders} WHERE order_id > 0 AND order_id < 100 ORDER BY order_id ASC ", 0, 100);
while ($id = db_result($result)) {
  $order = uc_order_load($id);
  $user_downloads = array();

  // Load user.
  if ($order->uid && ($order_user = user_load($order->uid))) {
    // Scan products for models matching downloads.
    foreach ($order->products as $product) {
      $files = db_query("SELECT * FROM {uc_file_products} AS fp ".
        "INNER JOIN {uc_product_features} AS pf ON pf.pfid = fp.pfid ".
        "INNER JOIN {uc_files} as f ON f.fid = fp.fid ".
        "WHERE nid = %d", $product->nid, $order_user->uid);

      while ($file = db_fetch_object($files)) {

        // Either they match, or the file was set to any SKU.
        if (!empty($file->model) && $file->model != $product->model) {
          continue;
        }

        // Grab any existing privilege so we can calculate the new expiration time
        // as an offset of the previous.
        $file_user = _uc_file_user_get($order_user, $file->fid);

        // Get the limit info from the product feature
        $file_modification = array(
          'download_limit' => uc_file_get_download_limit($file),
          'address_limit' => uc_file_get_address_limit($file),
          'expiration' => _uc_file_expiration_date(uc_file_get_time_limit($file), $file_user->expiration),
        );

        // Add file_user(s) for this file/directory. (No overwrite)
        $new_files = uc_file_user_renew($file->fid, $order_user, $file->pfid, $file_modification, FALSE);

        // Save for notification.
        $user_downloads = array_merge($user_downloads, $new_files);

      }
    }
  }
}

Thanks a lot for that, Jody Lynn. The fix is correct, I can't believe that wasn't found until now...

I see now, the reason I didn't see a problem in my local is because I had the patch applied that I posted here: http://www.ubercart.org/files/uc_file.2.x.patch

I have attached a patch that is more appropriate.

I just, just, got the latest 2.x dev from bazar and it does not have this patch.

line 341 still reads:

  // Delete the old entry for this file.
  db_query("DELETE FROM {uc_file_users} WHERE fid = %d", $file_user->fid);

  drupal_write_record('uc_file_users', $file_user);

Did this patch still need to be committed?

Yeah, hence the status. When it's committed, it'll be moved to fixed. ; )

I know why this issue wasn't found for a while. It's really hard to think about testing how multiple users will affect things when you're only one person. ;) And that probably explains why I've taken so long to review this patch.

Committed now.