QT pro a logical error

[Juto]

Hi, maby someone would like to have a look at this (I have posted it in the support thread for QTpro also, but so far no fix):

 

A bug in catalog/includes/functions/general.php

 

function tep_get_products_stock($products_id, $attributes=array()) {

global $languages_id;

$products_id = tep_get_prid($products_id);

//Fix ? Otherwise $all_nonstocked is not defined if there are no attributes:

$all_nonstocked = false; <-----------------A fix?

if (sizeof($attributes)>0) {

$all_nonstocked = true;

$attr_list='';

$options_list=implode(",",array_keys($attributes));

$track_stock_query=tep_db_query("select products_options_id, products_options_track_stock from " . TABLE_PRODUCTS_OPTIONS . " where products_options_id in ($options_list) and language_id= '" . (int)$languages_id . "' order by products_options_id");

 

while($track_stock_array=tep_db_fetch_array($track_stock_query)) {

if ($track_stock_array['products_options_track_stock']) {

$attr_list.=$track_stock_array['products_options_id'] . '-' . $attributes[$track_stock_array['products_options_id']] . ',';

$all_nonstocked=false;

}

}

$attr_list=substr($attr_list,0,strlen($attr_list)-1);

}

 

// Below the $all_nonstocked is undefined if the above fix isn't applied. Pipe symbol: $a | $b means either $a or $b

// NB there is NO condition for $all_nonstocked below. Shouldn't be something like $all_nonstocked==false;?

 

if ((sizeof($attributes)==0) | ($all_nonstocked)) {

$stock_query = tep_db_query("select products_quantity as quantity from " . TABLE_PRODUCTS . " where products_id = '" . (int)$products_id . "'");

} else {

$stock_query=tep_db_query("select products_stock_quantity as quantity from " . TABLE_PRODUCTS_STOCK . " where products_id='". (int)$products_id . "' and products_stock_attributes='$attr_list'");

}

 

if (tep_db_num_rows($stock_query)>0) {

$stock=tep_db_fetch_array($stock_query);

$quantity=$stock['quantity'];

} else {

$quantity = 0;

}

return $quantity;

}

 

Have someone else found this and a correct fix?

Sara

[♥FWR Media]

You haven't given the error so it can't be debugged but ..

 

$all_nonstocked = false; <-----------------A fix?

 

Is a parse error.

 

if ((sizeof($attributes)==0) | ($all_nonstocked)) {

 

Looks like it is using a bitwise operator where it should be using the logical OR operator ||

 

The QtPro support thread is the correct place for this question though, if you are not getting an answer you'll just have to wait, the price of free support.

[Juto]

Hi Robert and thanks for your answer :)

 

KISS error handler reports the $all_nonstocked is undefined for the original code i.e without my "fix".

 

And yes, there is a support thread, but still no luck... So I really appreciate your help.

 

Kindest

Sara

[dr_lucas]

I believe the correct code should actually look like that:

 

 function tep_get_products_stock($products_id, $attributes=array()) {
global $languages_id;
$products_id = tep_get_prid($products_id);

if (sizeof($attributes)>0) {
$all_nonstocked = true;
$attr_list='';
$options_list=implode(",",array_keys($attributes));
$track_stock_query=tep_db_query("select products_options_id, products_options_track_stock from " . TABLE_PRODUCTS_OPTIONS . " where products_options_id in ($options_list) and language_id= '" . (int)$languages_id . "' order by products_options_id");

while($track_stock_array=tep_db_fetch_array($track_stock_query)) {
if ($track_stock_array['products_options_track_stock']) {
$attr_list.=$track_stock_array['products_options_id'] . '-' . $attributes[$track_stock_array['products_options_id']] . ',';
$all_nonstocked=false;
}
}
$attr_list=substr($attr_list,0,strlen($attr_list)-1);
}

// Below the $all_nonstocked is undefined if the above fix isn't applied. Pipe symbol: $a | $b means either $a or $b
// NB there is NO condition for $all_nonstocked below. Shouldn't be something like $all_nonstocked==false;?

if (sizeof($attributes)==0 || isset($all_nonstocked)) {
$stock_query = tep_db_query("select products_quantity as quantity from " . TABLE_PRODUCTS . " where products_id = '" . (int)$products_id . "'");
} else {
$stock_query=tep_db_query("select products_stock_quantity as quantity from " . TABLE_PRODUCTS_STOCK . " where products_id='". (int)$products_id . "' and products_stock_attributes='$attr_list'");
}

if (tep_db_num_rows($stock_query)>0) {
$stock=tep_db_fetch_array($stock_query);
$quantity=$stock['quantity'];
} else {
$quantity = 0;
}
return $quantity;
}

[dr_lucas]

Actually, I am not 100% sure. It may be better to make:

if (sizeof($attributes)==0 || isset($all_nonstocked)) {

 

Changed to:

if ( sizeof($attributes)==0 || (isset($all_nonstocked) && $all_nonstocked==false) ) {

 

The strange thing is that QT pro seems to be working just fine (except for the occasional E_NOTICE of undefined $all_nonstocked) the way this function was originally written...I don't get it... :mellow:

[MrPhil]

The only error was using the bitwise OR | instead of the logical OR ||. Once that's fixed, there is no need to set a default $all_nonstocked, because "short circuiting" of the logical expression will prevent PHP from seeing the second part of the test, if the first is already TRUE. If the first part is FALSE, then the variable should have been set in the block of code above and will be tested. If it makes you feel better to define the variable up top, there's no harm done.

[dr_lucas]

The problem is that many times this line throws E_NOTICE saying that $all_nonstocked is not defined.

What do you think this condition supposed to check except for the sizeof($attributes)==0? Does it check if $all_nonstocked==false or true? or just if it is set?

[MrPhil]

If you fix the | to be ||, it should work withhout error.

[dr_lucas]

@@MrPhil I tried that and it did throw E_NOTICE...do you know, by any chance, which condition would be needed? I guess a more strict way to write it to avoid the error?

[MrPhil]

Change

if ((sizeof($attributes)==0) | ($all_nonstocked)) {

to

if (sizeof($attributes)==0 || $all_nonstocked) {

 

If there are other problems, they may be causing their own errors. In other words, you're probably getting past this particular error and hitting another one. Can you give the specific error it's now throwing?

[dr_lucas]

@@MrPhil Sure:

Error Type: [E_NOTICE] Undefined variable: all_nonstocked

 

My concern is that this if condition checks the sizeof the $attributes variable, which is being set above and then it checks the $all_nonstocked variable, which basically doesn't make sense to me because $all_nonstocked should either be true or false or it's not set. How can it just check $all_nonstocked variable without being more specific?

Is the code you wrote:

if (sizeof($attributes)==0 || $all_nonstocked) {

 

equivalent to:

if (sizeof($attributes)==0 || isset($all_nonstocked)) {

 

Because if it is, then this whole IF condition doesn't make much sense if you look at the entire function.

[MrPhil]

if (sizeof($attributes)>0) {
 $all_nonstocked = ....
}
...
if (sizeof($attributes)==0 || $all_nonstocked) { ...

What's the problem? If there are attributes (sizeof > 0), $all_nonstocked will be set true or false, and will be looked at in the second test. If there are no attributes (sizeof == 0), the "if" test is 'true' without even looking at $all_nonstocked, so it doesn't matter that it's not defined.

 

isset($all_nonstocked) is not the same as $all_nonstocked. It returns true if the variable has been defined, and has nothing to do with whether it's true or false.

[dr_lucas]

@@MrPhil

Exactly. And that was one of the main points of the OP. I will try to explain.

When the function reaches this second IF section, the only situation it is actually looking at in the "|| $all_nonstocked", as you explained, is when there are attributes, ie. sizeof($attributes)>0.

So, let's look at the function again, when there are attributes.

The first IF section ("if (sizeof($attributes)>0) {" ie. there are attributes) gives $all_nonstocked a "true" value when it starts and then it may give it a "false" value in the while loop.

After that, it reaches the second IF, and since "sizeof($attributes)==0" is not true, it continues to look at "$all_nonstocked", but what value does it expect in order to return this IF to "true"? When will it perform the operation conditioned by this IF? Will it do it only if $all_nonstocked="true" or only if $all_nonstocked="false" or in either case?

[MrPhil]

sizeof($attributes) cannot be < 0. If it's > 0, $all_nonstocked will be either true or false. If it's == 0, $all_nonstocked is not set, but that doesn't matter since sizeof($attributes) == 0 will be true, and the other term in the OR will be ignored (not evaluated). Read up on "short circuiting" of boolean expression evaluation. If sizeof($attributes) > 0, the first term of the if is false, so then it will look at $all_nonstocked (which is guaranteed true or false) to set the expression's value.

[dr_lucas]

@@MrPhil

Yes, I know that and that's exactly what I wrote.

Maybe I didn't write my question clearly...I will try to rephrase:

 

Quote: "If sizeof($attributes) > 0, the first term of the if is false, so then it will look at $all_nonstocked (which is guaranteed true or false) to set the expression's value."

In which case will it then process the following part (1)?

$stock_query = tep_db_query("select products_quantity as quantity from " . TABLE_PRODUCTS . " where products_id = '" . (int)$products_id . "'");

 

Instead of processing part (2):

$stock_query=tep_db_query("select products_stock_quantity as quantity from " . TABLE_PRODUCTS_STOCK . " where products_id='". (int)$products_id . "' and products_stock_attributes='$attr_list'");

 

Will it do part (1) when $all_nonstocked is true or $all_nonstocked is false or in either case?

[MrPhil]

if (sizeof($attributes)==0 || $all_nonstocked) {
// here if no attributes (sizeof == 0). $all_nonstocked isn't even set, much less looked at: T || x ==> T
// OR there are attributes (sizeof > 0) and $all_nonstocked was set to TRUE: F || T ==> T
$stock_query = tep_db_query("select products_quantity as quantity from " . TABLE_PRODUCTS . " where products_id = '" . (int)$products_id . "'");
} else {
// here if there are attributes (sizeof > 0) and $all_nonstocked was set to FALSE: F || F ==> F
$stock_query=tep_db_query("select products_stock_quantity as quantity from " . TABLE_PRODUCTS_STOCK . " where products_id='". (int)$products_id . "' and products_stock_attributes='$attr_list'");
}

[dr_lucas]

@@MrPhil :)

Now we are getting somewhere.

 

So basically if we changed:

if (sizeof($attributes)==0 || $all_nonstocked) {

 

To:

 

if (sizeof($attributes)==0 || $all_nonstocked=="true") {

 

Or to:

 

if (sizeof($attributes)==0 || (isset($all_nonstocked) && $all_nonstocked=="true")) {

 

 

They would be practically the same, just clearer and more strict, right?

[MrPhil]

Functionally equivalent (since we know that $all_nonstocked is set if the sizeof is != 0). It's not necessary, but if it's clearer to you, go ahead and do it.

[dr_lucas]

Excellent! Thanks a lot, @@MrPhil :)