AdminRequestSanitizer Error Log

Another question: How do array variables get registered with the sanitizer? Do I "register" each sub-variable name? Is there a sanitizer group to define an array variable?

lat9:

Since the ***id ***parameter is already sanitized, is there no hope?

Hi

So yes there is hope :)

I would forget about changing id -> attr_info, especially if it is likely to break other code.

The new version of the adminSanitizer class i'm currently working on. will allow you to override sanitization on a per page basis. So even if we give a
general sanitizer for the id parameter, you will be able to override that for edit orders.

lat9:

Another question: How do array variables get registered with the sanitizer? Do I "register" each sub-variable name? Is there a sanitizer group to define an array variable?

So at the moment there is no way of defining sanitization except at a top level basis,

so I can't add sanitizers at a sub level at the moment

However again, I'm currently working on a MUTLI_DIMENSIONAL sanitizer that allows you to define something like

   ```php

$group = array(
'id' => array('sanitizerType' => 'MULTI_DIMENSIONAL',
'method' => 'post', 'pages' => array('edit_orders'), 'params' => array('id'=>'CONVERT_INT', 'name'=>'WORDS_AND_SYMBOLS_REGEX')));

at the moment this still doesn't let you recurse even deeper.

so if you look at edit orders it creates a post array 

 ```php
[update_products] => Array
        (
            [13] => Array
                (
                    [qty] => 1
                    [name] => Microsoft IntelliMouse Explorer
                    [onetime_charges] => 0.0000
                    [attr] => Array
                        (
                            [3] => Array
                                (
                                    [value] => 11
                                    [type] => 0
                                )

                        )

                    [model] => MSIMEXP
                    [tax] => 0
                    [final_price] => 70.95

and MULTI_DIMENSIONAL doesn't allow you to define a deep sanitizer for [attr]

what I want to happen is to allow you to define MUTLI_DIMENSIONAL within an outer MUTLI_DIMENSIONAL, but as you can imagine, that involves
some wonderful recursive structures/code.

Oh boy! Recursion! That's always fun ... especially to debug.

It also reminds me of one of my favorites (seen in a tongue-in-cheek document index):

Recursion: See recursion.

lat9:

Recursion: See recursion.

:D

Hoping to finish the code for this tonight.

The data structure for defining the sanitization would be

        $group = array(
            'update_products' => array(
                'sanitizerType' => 'MULTI_DIMENSIONAL',
                'method' => 'post',
                'pages' => array('edit_orders'),
                'params' => array(
                    'update_products' => array('sanitizerType' => 'CONVERT_INT'),
                    'qty' => array('sanitizerType' => 'CONVERT_INT'),
                    'name' => array('sanitizerType' => 'WORDS_AND_SYMBOLS_REGEX'),
                    'onetime_charges' => array('sanitizerType' => 'SIMPLE_ALPHANUM_PLUS'),
                    'attr' => array(
                        'sanitizerType' => 'MULTI_DIMENSIONAL',
                        'params' => array(
                            'attr' => array('sanitizerType' => 'CONVERT_INT'),
                            'value' => array('sanitizerType' => 'CONVERT_INT'),
                            'type' => array('sanitizerType' => 'CONVERT_INT')
                        )
                    ),
                    'model' => array('sanitizerType' => 'WORDS_AND_SYMBOLS_REGEX'),
                    'tax' => array('sanitizerType' => 'SIMPLE_ALPHANUM_PLUS'),
                    'final_price' => array('sanitizerType' => 'SIMPLE_ALPHANUM_PLUS'),
                )
            )
        );

It should be noted that you don't necessarily have to go to this level of sanitizing, but I feel it should be available.

My most recent changes are here
https://github.com/zcwilt/zc-v1-series/commit/8440accd75aa73fe5433b586c49ff41070bab641
however that doesn't yet have code to do the MULTI_DIMENSIONAL recursion.

Looks good, wilt; that's how I was envisioning the configuration. I'll watch for your updates.

lat9:

Oh boy! Recursion! That's always fun ... especially to debug.

It also reminds me of one of my favorites (seen in a tongue-in-cheek document index):

Recursion: See recursion.

Infinite Loop: See Loop, Infinite

Loop, Infinite: See Infinite Loop

So I think I am close to a finished solution now

https://github.com/zcwilt/zc-v1-series/commit/8b1d302b0ea3c17f2526f4b1e6aee0cdd95199cb

Just a couple of things left to do

Add some more unit tests
and update the Documentation for the Admin Sanitizer

It's looking good, so far! I'll keep at it and let you know if I come across anything.

I'm working on updated documentation, and probably some expanded tests.

Will post here once done.

@wilt, is there a way to stop the unwanted htmlentities' conversions? If I order the product **A Bug's Life "Multi Pak" Special 2003 Collectors Edition
** and then edit that order (simply pressing the "Update" button), the name changes to A Bug's Life "Multi Pak" Special 2003 Collectors Editi ... with the double-quotes converted to " and the name getting truncated due to the additional characters.

The same thing happens if I enter a text attribute that uses special characters, e.g. Here's some text … gets converted to Here's some text … -- and it just gets worse each time that order is updated since each & is converted to &.

Have you defined any extra sanitizers.

I'm about to update the docs regarding this.

lat9:

@wilt, is there a way to stop the unwanted htmlentities' conversions? If I order the product **A Bug's Life "Multi Pak" Special 2003 Collectors Edition
** and then edit that order (simply pressing the "Update" button), the name changes to A Bug's Life "Multi Pak" Special 2003 Collectors Editi ... with the double-quotes converted to " and the name getting truncated due to the additional characters.

The same thing happens if I enter a text attribute that uses special characters, e.g. Here's some text … gets converted to Here's some text … -- and it just gets worse each time that order is updated since each & is converted to &.

Just to be clear here.

If you want to test latest code, you need to pull in the changes from my https://github.com/zcwilt/zc-v1-series/tree/adminsanitizer-updates branch

and there are 3 files

admin/includes/auto_loaders/config.adminSanitize.php
admin/includes/classes/AdminRequestSanitizer.php
admin/includes/init_includes/init_sanitize.php

Even pulling these in won't fix edit_orders as you then need to add your own sanitizers
as a simple test I created

/admin/includes/extra_datafiles/edit_orders_sanitize.php that contained

<?php
/**
 * Created by PhpStorm.
 * User: wilt
 * Date: 07/04/16
 * Time: 20:45
 */
$sanitizer = AdminRequestSanitizer::getInstance();
$group = array(
    'id' => array('sanitizerType' => 'NULL_ACTION', 'method' => 'both', 'pages' => array('edit_orders'), 'params' => array()));
$sanitizer->addComplexSanitization($group);
$group = array(
    'update_products' => array('sanitizerType' => 'NULL_ACTION', 'method' => 'both', 'pages' => array('edit_orders'), 'params' => array()));
$sanitizer->addComplexSanitization($group);

and that fixed htmlentities problems

Now of course, those should only be considered temporary fixes as they basically ignore sanitization for id and update_products, whereas what should be added is
a MULTI_DIMENSIONAL sanitizer

I'll pull your most recent updates down for my test setup. It seems like the product's name and attribute name/value pairs will all need the NULL_ACTION sanitization -- is that correct?

Should there be a general sanitizer group for floats? The **qty **value that is included in the EO POST variables is a floating-point value, not an int.

Here's what I've come up with (so far) for the EO sanitizer; please let me know if there's a better way. This approach gets the product's name and text attributes to not get &'d to death:

    $eo_sanitizer = AdminRequestSanitizer::getInstance();
    $eo_group = array(
        'update_products' => array(
            'sanitizerType' => 'MULTI_DIMENSIONAL',
            'method' => 'post',
            'pages' => array('edit_orders'),
            'params' => array(
                'update_products' => array('sanitizerType' => 'CONVERT_INT'),
                'qty' => array('sanitizerType' => 'CONVERT_INT'),  //-This one should really be a float
                'name' => array('sanitizerType' => 'PRODUCT_DESC_REGEX'),
                'onetime_charges' => array('sanitizerType' => 'CURRENCY_VALUE_REGEX'),
                'attr' => array(
                    'sanitizerType' => 'MULTI_DIMENSIONAL',
                    'params' => array(
                        'attr' => array('sanitizerType' => 'CONVERT_INT'),
                        'value' => array('sanitizerType' => 'PRODUCT_DESC_REGEX'),
                        'type' => array('sanitizerType' => 'CONVERT_INT')
                    )
                ),
                'model' => array('sanitizerType' => 'WORDS_AND_SYMBOLS_REGEX'),
                'tax' => array('sanitizerType' => 'CURRENCY_VALUE_REGEX'),
                'final_price' => array('sanitizerType' => 'CURRENCY_VALUE_REGEX'),
            )
        )
    );
    $eo_sanitizer->addComplexSanitization ($eo_group);

lat9:

Should there be a general sanitizer group for floats? The **qty **value that is included in the EO POST variables is a floating-point value, not an int.

Will add this :)

lat9:

Here's what I've come up with (so far) for the EO sanitizer; please let me know if there's a better way. This approach gets the product's name and text attributes to not get &'d to death:
[

So I've pushed up a change to my testing branch to add a FLOAT_VALUE_REGEX

I guess that probably 'tax' and 'final_price' should strictly speaking be floats.

In fact I think the CURRENCY_VALUE_REGEX may be unnecessary.

I'm not quite ready to push my testing branch to a full blown PR against core code as I want to make sure the unit tests cover some edge cases first.

Just checking in to see where the Zen Cart base code is on this issue. So far, my tests with the eo_sanitizer supplied above and the April 7 version of the Zen Cart changes looks sound.

One request: When Zen Cart is updated with these sanitizer changes, pretty-please make it a formal release for either Zen Cart 1.5.5a or 1.5.6 so that EO can determine the environment in which it's loading/installing and guide the installer to the proper version if plain-old-original Zen Cart 1.5.5 is currently being used.

lat9:

Just checking in to see where the Zen Cart base code is on this issue. So far, my tests with the eo_sanitizer supplied above and the April 7 version of the Zen Cart changes looks sound.

One request: When Zen Cart is updated with these sanitizer changes, pretty-please make it a formal release for either Zen Cart 1.5.5a or 1.5.6 so that EO can determine the environment in which it's loading/installing and guide the installer to the proper version if plain-old-original Zen Cart 1.5.5 is currently being used.

Hi
I just pushed some final changes to the code, this shouldn't affect anything you have been testing as it mainly revolves around custom sanitizers and unit testing.
Will do the PR against core tonight.
Your comment regarding release numbering is noted and this will be what happens.