AdminRequestSanitizer Error Log

[swguy]

Edit orders fabricates the order totals in a multilayered structure. It probably does other things like this too.

Array
(
    [0] => Array
        (
            [code] => ot_combination_discounts
            [title] => Combination Discounts :
            [value] => 3.0000
        )

    [1] => Array
        (
            [code] => ot_fuelsurcharge
            [title] => Fuel Surcharge:
            [value] => 8.5753
        )

    [2] => Array
        (
            [code] => ot_shipping
            [shipping_module] => flat
            [title] => Regular Shipping (Basic shipping included):
            [value] => 0.0000
        )

    [3] => Array
        (
            [code] => ot_snqd
            [title] =>
            [value] =>
            [shipping_module] =>
        )

)

[mc12345678]

A recursive approach could be taken instead of "planning" for depth... Also, it seems that since ZC 1.5.1 the use of htmlspecialchars has been expanded to include other "directives", should those not be added to that code instead of letting things go as defaulted?

[wilt]

Hi

I do have a pending commit that uses recursion.
https://github.com/zencart/zencart/pull/886/files

[DrByte]

Wilt's fix has been merged into the v155 branch on github ... and is now part of core code since the 03-29-2016 zip of v155.


Please run it with Edit Orders. I think the only potential "issue" with it is that it might mangle any HTML in product names when editing one of those in an order.

[mc12345678]

Hi

I do have a pending commit that uses recursion.
https://github.com/zencart/zencart/pull/886/files

Doesn't the foreach depend on $item being array, but it seems that there is no check either upon entry into traverseStricSanitize or before/as a part of calling traverseStricSanitize. Ie, is it possiblethat $item could be empty I guess? I can't seem to think where either $_GET or $_POST would specifically equal a value and not some formof an array... Hmmm... Now that I say that though... Use of Super Globals while watching the store front, add a product to the cart, and $_POST which at some point had data is now "empty". Wondering if the issue is similar/related... Otherwise if $item is not an array, could throw it into one?

I haven't been able to test related to this, but it has me curious...

[wilt]

The traverseStricSanitize function passes $item by reference, and for the initial entry point uses either $_POST or $_GET and these will always be arrays whether empty or not.

The problem is that something like Edit Orders passes a multi dimension array, where child entries need their own sanitization, rather than using the default strict sanitization and the current admin sanitizer class doesn't support that.

I am working on code at the moment that will allow for defining validation at a more granular level, and hope to have a PR soon

Doesn't the foreach depend on $item being array, but it seems that there is no check either upon entry into traverseStricSanitize or before/as a part of calling traverseStricSanitize. Ie, is it possiblethat $item could be empty I guess? I can't seem to think where either $_GET or $_POST would specifically equal a value and not some formof an array... Hmmm... Now that I say that though... Use of Super Globals while watching the store front, add a product to the cart, and $_POST which at some point had data is now "empty". Wondering if the issue is similar/related... Otherwise if $item is not an array, could throw it into one?

I haven't been able to test related to this, but it has me curious...

[wilt]

i have watched this thread for a bit. i find it amusing/sad that the dev team is forced to change CORE code to make it work with a plug-in; not the other way around.

This is not what is happening

We are not going to put code into core that adds sanitization for specific Edit Order code parameters
Although I might suggest how EO might work better with core.

The problem is that currently core code does not have sufficient hooks to allow some thing like EO to use our sanitizer correclty.
Thats not EO's problem

[mc12345678]

The traverseStricSanitize function passes $item by reference, and for the initial entry point uses either $_POST or $_GET and these will always be arrays whether empty or not.

The problem is that something like Edit Orders passes a multi dimension array, where child entries need their own sanitization, rather than using the default strict sanitization and the current admin sanitizer class doesn't support that.

I am working on code at the moment that will allow for defining validation at a more granular level, and hope to have a PR soon

Yeah, I saw the by reference style and when the original reason of not working was identified (multi-dimension array), I was half expecting something along the lines of the following (doesn't use pass-by-reference) and not trying to derail existing coding path:

function return_array_combinations($arrMain, $intVars, $currentLoop = array(), $currentIntVar = 0) {
  $arrNew = array();

  for ($currentLoop[$currentIntVar] = 0; $currentLoop[$currentIntVar] < sizeof($arrMain[$currentIntVar]); $currentLoop[$currentIntVar]++) {
    if ($intVars == $currentIntVar + 1) {
      $arrNew2 = array();
      for ($i = 0; $i<$intVars;$i++) {
        $arrNew2[] = $arrMain[$i][$currentLoop[$i]];  // This is a place where an evaluation could be made to do something unique with a single component that is to be assigned to a single combination as this assigment is for one of the arrays to be combined for one record. If the goal would be not to add anything having this array, then could call continue 2 to escape this for loop and move on to the next outer for loop.  If just want to not add the one array/component to the combination then place the above assignment so that it is bypassed when not to be added. 
      }
      if (zen_not_null($arrNew2) && sizeof($arrNew2) > 0) { // This is a place where an evaluation could be made to do something unique with an array or component as this assigment is one of all arrays combined for one record.  //Still something about this test doesn't seem quite right, but it's the concept that is important, as long as there is something to evaluate/assign that is not nothing, then do the assignment.
        $arrNew[] = $arrNew2;
      }
    } else {
      $arrNew = array_merge($arrNew, return_array_combinations($arrMain, $intVars, $currentLoop, $currentIntVar + 1));
    }
  }

  return $arrNew;
}

It's initiated by passing the array and the number of initial components of the main array, probably could be simplified that small amount further to not need to pass the number of array elements, but when that solution was found for the problem at hand, that was that as it worked and I didn't want to mess it up. :)

It doesn't have/use the additional feature of logging/identifying the "starting" point of the "unrecognized" key, but I thought I would offer some code that has been developed to dig into the depths of an array of an array of an array, etc... recursively.

This was developed to accomplish something similar to the array of array situation, though the conditions are/were a little different as far as the earlier build of action based on condition to perform the next/same action with the next group... The problem being addressed/resolved was:

          if ($intVars >= 1) {
          //adds array combinations
          // there are X variables / arrays
          // so, you need that many arrays
          // then, you have to loop through EACH ONE
          // if it is the LAST variable / array
          // you need to add that variable / array VALUE
          // and ALL PREVIOUS VALUES to the multi-dimensional array
          // below supports up to 7 variables / arrays
          // to add more, just copy and paste into the last for loop and go up from $n is the last one
          for ($i = 0;$i < sizeof($arrMain[0]);$i++) {
          if ($intVars >= 2) {
          for ($j = 0;$j < sizeof($arrMain[1]);$j++) {
          if ($intVars >= 3) {
          for ($k = 0;$k < sizeof($arrMain[2]);$k++) {
          if ($intVars >= 4) {
          for ($l = 0;$l < sizeof($arrMain[3]);$l++) {
          if ($intVars >= 5) {
          for ($m = 0;$m < sizeof($arrMain[4]);$m++) {
          if ($intVars >= 6) {
          for ($n = 0;$n < sizeof($arrMain[5]);$n++) {
          if ($intVars >= 7){
          for ($o = 0; $o < sizeof($arrMain[6]); $o++) {
          $arrNew[] = array($arrMain[0][$i], $arrMain[1][$j], $arrMain[2][$k], $arrMain[3][$l], $arrMain[4][$m], $arrMain[5][$n], $arrMain[6][$o]);
          }
          } else {
          $arrNew[] = array($arrMain[0][$i], $arrMain[1][$j], $arrMain[2][$k], $arrMain[3][$l], $arrMain[4][$m], $arrMain[5][$n]);
          }
          }
          } else {
          $arrNew[] = array($arrMain[0][$i], $arrMain[1][$j], $arrMain[2][$k], $arrMain[3][$l], $arrMain[4][$m]);
          }
          }
          } else {
          $arrNew[] = array($arrMain[0][$i], $arrMain[1][$j], $arrMain[2][$k], $arrMain[3][$l]);
          }
          }
          } else {
          $arrNew[] = array($arrMain[0][$i], $arrMain[1][$j], $arrMain[2][$k]);
          }
          }
          } else {
          $arrNew[] = array($arrMain[0][$i], $arrMain[1][$j]);
          }
          }
          } else {
          $arrNew[] = array($arrMain[0][$i]);
          }
          }

ultimately, every combination of the original array is addressed; however, the original variable was a single depth array each item having one set of keys with a matching value, but ultimately an array of arrays is built. Don't know if it helps at all... I'm not in a position to tie into the file being addressed here.

[wilt]

So one of the reasons Edit Orders is broken with reference to the sanitizer
is when attempting to add a new product to the order(as has been mentioned elsewhere in the thread) .

It breaks because EO uses a post parameter called 'id' to pass a multi-dimensional array.
However the sanitizer already adds the 'id' parameter to a Sanitization Group that converts the parameter to an integer, which obviously destroys the
array structure in EO.

I mentioned in the documentation for the Sanitizer the need for developers to be careful with parameter naming.

However, I also realize that forcing developers to have to rewrite code to address parameter naming could be onerous and likely to introduce bugs initially, and could delay store owners in adopting v1.5.5, if popular contributions are not updated.

As I mentioned previously, I am working on a rewrite of the sanitizer to make it more granular, and I’m also exploring some changes to the data structures that would allow contributions to override sanitizing of a parameter on a per page basis.