[Done v155c] $product_type incorrectly set in admin/product.php

[apogeerockets]

Using 1.5.5a. Pretty sure this bug has been around since 1.5.4, since I located it in a pre-upgrade file, too. I didn't see any obvious references to this, nor addressed in 1.5.5b changelogs, so hopefully it's not a repeat.

This bug only becomes evident when expanding product types. The only place $product_type is defined is within the case of "delete_product_confirm" action. I have several custom product types with custom delete_product_confirm and copy_to_confirm files. I couldn't figure out why it kept using the default copy_to file and thus not correctly copying my extra tables, but then after far-too-long, I realized that $product_type wasn't defined for the copy_to_confirm action.

product.php code:
Beginning Ln19:

  if (zen_not_null($action)) {
    switch ($action) {
      case 'setflag':
        if ( ($_GET['flag'] == '0') || ($_GET['flag'] == '1') ) {
          if (isset($_GET['pID'])) {
            zen_set_product_status($_GET['pID'], $_GET['flag']);
          }
        }
        zen_redirect(zen_href_link(FILENAME_CATEGORIES, 'cPath=' . $_GET['cPath'] . '&pID=' . $_GET['pID'] . (isset($_GET['page']) ? '&page=' . $_GET['page'] : '') . ((isset($_GET['search']) && !empty($_GET['search'])) ? '&search=' . $_GET['search'] : '')));
        break;

      case 'delete_product_confirm':
      $delete_linked = 'true';
      if ($_POST['delete_linked'] == 'delete_linked_no') {
        $delete_linked = 'false';
      } else {
        $delete_linked = 'true';
      }
      $product_type = zen_get_products_type($_POST['products_id']);
        if (file_exists(DIR_WS_MODULES . $zc_products->get_handler($product_type) . '/delete_product_confirm.php')) {
          require(DIR_WS_MODULES . $zc_products->get_handler($product_type) . '/delete_product_confirm.php');
         } else {
          require(DIR_WS_MODULES . 'delete_product_confirm.php');
         }
        break;
      case 'move_product_confirm':
        if (file_exists(DIR_WS_MODULES . $zc_products->get_handler($product_type) . '/move_product_confirm.php')) {
          require(DIR_WS_MODULES . $zc_products->get_handler($product_type) . '/move_product_confirm.php');
         } else {
          require(DIR_WS_MODULES . 'move_product_confirm.php');
         }
        break; 

I resolved the bug by moving the $product_type definition to right after the

if (zen_not_null($action)) { 

and changing it to....

$product_type = (isset($_POST['products_id']) ? zen_get_products_type($_POST['products_id']) : $_GET['product_type']); 

Result:

  if (zen_not_null($action)) {

    $product_type = (isset($_POST['products_id']) ? zen_get_products_type($_POST['products_id']) : $_GET['product_type']); // Changed

    switch ($action) {
      case 'setflag':
        if ( ($_GET['flag'] == '0') || ($_GET['flag'] == '1') ) {
          if (isset($_GET['pID'])) {
            zen_set_product_status($_GET['pID'], $_GET['flag']);
          }
        }
        zen_redirect(zen_href_link(FILENAME_CATEGORIES, 'cPath=' . $_GET['cPath'] . '&pID=' . $_GET['pID'] . (isset($_GET['page']) ? '&page=' . $_GET['page'] : '') . ((isset($_GET['search']) && !empty($_GET['search'])) ? '&search=' . $_GET['search'] : '')));
        break;

      case 'delete_product_confirm': 

Not 100% if that's the best redefinition of the code, but it works in the instances I need it to. Mostly, $_GET['product_type'] seems to cover most instances. This will apply to all the product_HANDLER.php pages, afaik.

[DrByte]

Thanks for the detective work and the proposed solution.

While digging further, and with a view to helping resolve it for all product types, I opted to move that line out of the products.php file, into the prod-cat-header.php file, as shown here:
https://github.com/zencart/zencart/pull/1352/files