how is this beta addon idea?

[bumba000]

okay so I've changed to the following, but I don't understand why dropping a ' # ' or a ' % ' into one of the $_GETs still breaks stuff. Isn't zen checking for this?

if(isset($_GET)){
    $cleanJpManus =  array();
    foreach($_GET as $key => $value){
        if(preg_match('/^jparam_manu/i', $key)){
        $clean = preg_replace('/[^0-9_]/', '', $value);
            $cleanJpManus[] = $clean;
        }
        
    }
    if(sizeof($cleanJpManus) == 0){unset($cleanJpManus);}
}

if(isset($_GET['jpricelohi'])){
    $cleanJpLoHi = preg_replace('/[^0-9_]/', '', $_GET['jpricelohi']);
} 

[mc12345678]

Both of those characters in the uri string itself have their own definition. They would need to be escaped in some manner if they are to mean something other than their original definition. See RFC 3986 for some more information about uri characters. Yet also one more reason not to pass the data via GET: http://www.ietf.org/rfc/rfc3986.txt

[bumba000]

no, I don't want them there. If they're present ( hack attempt ) they need to be removed. The regex/preg_replace does work to remove the chars but apparently not before zen breaks....

[DrByte]

okay so I've changed to the following, but I don't understand why dropping a ' # ' or a ' % ' into one of the $_GETs still breaks stuff.

What exactly do you mean by "breaks stuff"? Some sort of error message perhaps? Without knowing what's breaking and where, it's kinda tough to tell you what to change.

Isn't zen checking for this?

Not for parameters it doesn't know about, no. There are some legitimate other scenarios where those characters are valid. So, since you're adding completely new functionality, it's up to you to sanitize your own stuff.


BTW, in a regex, the /i means "case insensitive", but everything you're doing doesn't need to be case-insensitive, so you can leave off the "i" and make the parser run faster.

[bumba000]

ok. I moved the sanitize stuff into functions/safety_functions.php and overridden it. The trouble is that

$_GET['jpricelohi'] = preg_replace('/[^_0-9]/', '', $_GET['jpricelohi']); 

doesn't remove ( % # * ) and so on and it breaks stuff, which we know isn't a good sign. I can not seem to find a regex combination that works and leaves 0-9 and _ . What the heck?!

I'm still working on it though. If you have a solution please, do tell.

Thank You, John

[DrByte]

ok. I moved the sanitize stuff into functions/safety_functions.php and overridden it.

That's not really necessary.
Simply sanitizing the inputs where you use them is preferable.

and it breaks stuff

Again, you've not yet explained what you mean by "breaks stuff".

The code $foo = preg_replace('/[^0-9_]/', '', $foo) does remove anything that's not 0-9 or underscore. I'm guessing you're testing that in the wrong place if you're getting unexpected results.

Handy: http://regexpal.com/

[bumba000]

uhhh, safety_functions.php is from another site, completely custom. So, sorry bout that!

The file is init_includes/overrides/init_sanitize.php. This is where I have the preg_replace...

Now. Ok, so load up the site. hit the price range filter (not sure why i called it loHi) anyways, changing the data in the url parameter to something like jpricelohi=96_1%89 or jpricelohi=96_1#89 after the preg_replace has run on $_GET['jpricelohi'] will either whitescreen with warning: refresh... or the hashtag makes it through but stops the functionality of the module and an echo of $_GET['jpricelohi'] then says: 96_1 .

If I do an all in php test like

$dirty = '99_434%34';
$dirty = preg_replace('/[^_0-9]/', '', $dirty);  
echo $dirty; 

I get ( 99_43434 )

if I do the exact same preg_replace on the url parameter $_GET['jpricelohi'] I can't tell if it works or not because if I put an % symbol into the url like ( 99_434%34 ) , I get a "page isn't redirecting properly". Replace the % with the $ symbol and I get "WARNING: An Error occurred, please refresh the page and try again."

But any of these funky characters in the $_GET['sort'] seem to be cleaned right up and the system moves along.

I have moved the preg_replace's to where the code is being executed on (as suggested). Still the same though when entering unwanted chars into the URL.

[DrByte]

The feedback I've given so far is from inspecting the code.

I just installed it to test on a demo site, and the sidebox doesn't even display, despite being enabled in the admin. Tried on home page, category page, product page, pretty much everywhere.
So, I hacked $show_product_filter_sidebox = true; and got it to display, but then it only displays Lo-to-Hi and Hi-to-Lo choices. Nothing about prices or manufacturers.

So I checked, and this line:

if(!$listing->EOF && $_GET['main_page'] == 'index'){

... when it checks $listing it finds that it's already EOF because the product-listing has already cycled thru all its content to reach the EOF marker.
Given that this is seemingly intended to simply check whether there are any listing results shown, there are some other options:
- if ($listing->RecordCount() > 0 && $current_page_base == 'index') {
- if ($listing_split->number_of_rows > 0 && $current_page_base == 'index') {
I'm inclined to use the latter.


I also discovered I'd made a couple typos in my earlier code suggestions: I used "explode" instead of "implode" in the manufacturer SQL, and forgot a closing parenthesis. I've corrected the post above.
This would lead to db errors (the "WARNING" you mentioned) if you're testing any manufacturer option at all.

The % is an odd duck, since using %34 gets treated as a single character, and thus has the effect of potentially invalidating the next parameters you've passed.

Also, any time I click on the Filter submit button it just takes me to the home page. I'm assuming that's because there's no <form> "action" specified in the sidebox template?

[bumba000]

That's surprising that the box doesn't show for you. Im on 1.5.1 (if it makes any difference). The idea is that the box will only display on a page which uses $listing and is not search, new, specials...

I tested the two conditions you've supplied and neither let the box show
if ($listing->RecordCount() > 0 && $current_page_base == 'index')
if ($listing_split->number_of_rows > 0 && $current_page_base == 'index')

I did notice that you had left off that closing round but completely overlooked the explode vs implode!

Wonder why we're having two way different experiences with this incredibly simple plugin. Really it's just the 1.3.8 blank_sidebox with a modification to a single core file (default_filter.php which is now overridden).

I've been trying to figure out where the mistake was made now with that implode / explode, you edited the post!! Funny. Well it works now.

Any luck getting the mod to work on your dev site?

[bumba000]

Also, the form submit url being blank obviously has been working for me. Perhaps it's browser related...
I've fixed it though.

in includes/templates/YOTEMP/sideboxes/tpl_product_filter_sidebox.php
add the lines

$uri_parts = explode('?', $_SERVER['REQUEST_URI'], 2);
$actionURL = $uri_parts[0]; 

and of course, change the line

  $content .= zen_draw_form('cust_product_filter', '', $method = 'get', $parameters = ''); 

to

  $content .= zen_draw_form('cust_product_filter', $actionURL, $method = 'get', $parameters = ''); 

No problems.