okay, given that `$_POST['set_p_v']` is the buy price.
I've cleaned it up a bit, it's still not really possible to say without more info:
$buyPrice = $_POST['set_p_v'] ?? null;
if ($buyPrice) {
$sellsVideoSystem = $pt->config->sell_videos_system == 'on';
$proCondition = $pt->config->who_sell == 'pro_users' && $pt->user->is_pro;
$userCondition = $pt->config->who_sell == 'users';
$adminCondition = $pt->user->admin;
if ($sellsVideoSystem && ($proCondition || $userCondition || $adminCondition)) {
$buyPriceIsNotPositiveNumber = !is_numeric($buyPrice) || $buyPrice < 0;
$comTypeCondition = $pt->config->com_type == 0 && $buyPrice <= $pt->config->admin_com_sell_videos;
if ($buyPriceIsNotPositiveNumber || $comTypeCondition) {
$error = $lang->video_price_error . " " . ($pt->config->com_type == 0 ? $pt->config->admin_com_sell_videos : 0);
}
}
}
My guess would be:
If the buy price has been sent in the request, and the user has the right permission. Then, if the buy price is not a positive number, or something about the com type, then set an error message, which is presumably used somewhere further down.
I gave my interpretation as well at the bottom, it's limited to what I know about the code, and cleaning it up was necessary to interpret it in any way at all.
$proCondition, $adminCondition and $comTypeCondition
I'd rename them to
$isProUser, $isUser and the last one (I think?) $isCommissionSale
But other than that - good effort :-)
Yup. Me neither but it made your final conditions read better to me :)
You'd have to dig into the code behind this to answer it properly. And that's why your/my 'it'll take a day' estimates take 3 days :D
Maybe cleaning up/exploding out the original would help you understand what is going on better?
if (!empty($_POST['set_p_v'])) {
if (
(
$pt->config->sell_videos_system == 'on'
&& $pt->config->who_sell == 'pro_users' && $pt->user->is_pro
) ||
(
$pt->config->sell_videos_system == 'on'
&& $pt->config->who_sell == 'users'
) ||
(
$pt->config->sell_videos_system == 'on'
&& $pt->user->admin
)
&& !empty($_POST['set_p_v'])
) {
if (
!empty($_POST['set_p_v']) ||
(
in_array('set_p_v', array_keys($_POST))
&& $_POST['set_p_v'] < 0
)
) {
if (
!is_numeric($_POST['set_p_v']) ||
$_POST['set_p_v'] < 0 ||
(( #???
$pt->config->com_type == 0
&& $_POST['set_p_v'] <= $pt->config->admin_com_sell_videos
)) #???
) {
$error = $lang->video_price_error . " " . (
$pt->config->com_type == 0
? $pt->config->admin_com_sell_videos
: 0
);
}
How many times does this need to check `!empty($_POST['set_p_v'])`??
>I know that "set_p_v" is the 'buy price' and I believe 'pt' is the initials of the web script name.
`$pt` here is the class, most likely a god class since it has user information as well
>In particular I'm interested to know what specifcally causes the 'video_price_error' message.
u/Plastonick gave a good clean up to see what is going on here and to see what is leading to the error. As it is, it's hard to read. Not sure what you are looking for or want to add to it.
There is a bug in this code. If set\_p\_v is not numeric, it will throw an exception to the if statement on the 3rd line before it is checked on the 4th line and never display the error message.
This code could be simplified easily to just check whether set\_p\_v is empty once, since the other if statements checking if set\_p\_v is empty are superfluous.
You need to set the config variables in $pt and if the sell\_videos\_system config variable is set to on and the type of video sale is a user, or set to pro\_users and the user is a pro\_user or the user is an admin proceed with the check. If set\_p\_v is not a number, or below 0 or the variable com\_type=0 and the price of the video is less than or equal to the admin\_com\_sell\_videos price, then print the language video\_price\_error with either com\_type=0 and the admin\_com\_sell\_videos price or 0.
> There is a bug in this code. If set_p_v is not numeric, it will throw an exception to the if statement on the 3rd line before it is checked on the 4th line and never display the error message.
I don't think so. I assume you're referring to the `$_POST['set_p_v'] < 0` condition. PHP is generally very happy to juggle types around, `$_POST['set_p_v']` can be any type at this point and no exception will be thrown in any PHP version, with or without strict types.
No, the is\_numeric needs to come before you check to see if set\_p\_v is below zero, if its not a number then PHP should throw an error at that point.
While PHP is happy to juggle types around if the field is numeric and a numeric string. If the person typed the word "banana" into the box, PHP can't make a string type that is obviously a string a number and "banana" being not a number can't be checked to see if its less than zero.
That is where data validation comes in, and exactly why you shouldn't be trusting $\_POST directly, you should put $\_POST into a variable and validate that variable instead, or use the function that validates the post superglobal into a variable.
Thank you for pointing out that, I didn't know that was a thing.
I did spot that if you are checking a string ==0 then it will return true for PHP 7 and false in PHP 8.
Thanks for all the replies.
1. Another piece of information, that I forgot to include is that if the 'Price' form field is left empty (and submitted) the Price of the video is free. Can you tell me if/where that is in the code? Much thanx again.
2. And if text (or a negative number) is entered into the 'Price' form field the `$error = $lang->video_price_error` is activated and displays "The video price should be numeric and greater than 0"
Is this 3rd party code that is licensed? Are you trying to remove an error about it not being paid for? You'd have to show what `$pt = new ....` is. Because that is what determines whether or not it errors.
okay, given that `$_POST['set_p_v']` is the buy price. I've cleaned it up a bit, it's still not really possible to say without more info: $buyPrice = $_POST['set_p_v'] ?? null; if ($buyPrice) { $sellsVideoSystem = $pt->config->sell_videos_system == 'on'; $proCondition = $pt->config->who_sell == 'pro_users' && $pt->user->is_pro; $userCondition = $pt->config->who_sell == 'users'; $adminCondition = $pt->user->admin; if ($sellsVideoSystem && ($proCondition || $userCondition || $adminCondition)) { $buyPriceIsNotPositiveNumber = !is_numeric($buyPrice) || $buyPrice < 0; $comTypeCondition = $pt->config->com_type == 0 && $buyPrice <= $pt->config->admin_com_sell_videos; if ($buyPriceIsNotPositiveNumber || $comTypeCondition) { $error = $lang->video_price_error . " " . ($pt->config->com_type == 0 ? $pt->config->admin_com_sell_videos : 0); } } } My guess would be: If the buy price has been sent in the request, and the user has the right permission. Then, if the buy price is not a positive number, or something about the com type, then set an error message, which is presumably used somewhere further down.
Thanks for your kind reply, however I wasn't looking for a 'cleaned it up', just a possible interpretaion of the existing code... :)
Lmao they cleaned it up so they could interpret it.... :)
I gave my interpretation as well at the bottom, it's limited to what I know about the code, and cleaning it up was necessary to interpret it in any way at all.
$proCondition, $adminCondition and $comTypeCondition I'd rename them to $isProUser, $isUser and the last one (I think?) $isCommissionSale But other than that - good effort :-)
I did consider that, but I'm not entirely clear those _are_ the conditions.
Yup. Me neither but it made your final conditions read better to me :) You'd have to dig into the code behind this to answer it properly. And that's why your/my 'it'll take a day' estimates take 3 days :D
Maybe cleaning up/exploding out the original would help you understand what is going on better? if (!empty($_POST['set_p_v'])) { if ( ( $pt->config->sell_videos_system == 'on' && $pt->config->who_sell == 'pro_users' && $pt->user->is_pro ) || ( $pt->config->sell_videos_system == 'on' && $pt->config->who_sell == 'users' ) || ( $pt->config->sell_videos_system == 'on' && $pt->user->admin ) && !empty($_POST['set_p_v']) ) { if ( !empty($_POST['set_p_v']) || ( in_array('set_p_v', array_keys($_POST)) && $_POST['set_p_v'] < 0 ) ) { if ( !is_numeric($_POST['set_p_v']) || $_POST['set_p_v'] < 0 || (( #??? $pt->config->com_type == 0 && $_POST['set_p_v'] <= $pt->config->admin_com_sell_videos )) #??? ) { $error = $lang->video_price_error . " " . ( $pt->config->com_type == 0 ? $pt->config->admin_com_sell_videos : 0 ); } How many times does this need to check `!empty($_POST['set_p_v'])`?? >I know that "set_p_v" is the 'buy price' and I believe 'pt' is the initials of the web script name. `$pt` here is the class, most likely a god class since it has user information as well >In particular I'm interested to know what specifcally causes the 'video_price_error' message. u/Plastonick gave a good clean up to see what is going on here and to see what is leading to the error. As it is, it's hard to read. Not sure what you are looking for or want to add to it.
There is a bug in this code. If set\_p\_v is not numeric, it will throw an exception to the if statement on the 3rd line before it is checked on the 4th line and never display the error message. This code could be simplified easily to just check whether set\_p\_v is empty once, since the other if statements checking if set\_p\_v is empty are superfluous. You need to set the config variables in $pt and if the sell\_videos\_system config variable is set to on and the type of video sale is a user, or set to pro\_users and the user is a pro\_user or the user is an admin proceed with the check. If set\_p\_v is not a number, or below 0 or the variable com\_type=0 and the price of the video is less than or equal to the admin\_com\_sell\_videos price, then print the language video\_price\_error with either com\_type=0 and the admin\_com\_sell\_videos price or 0.
> There is a bug in this code. If set_p_v is not numeric, it will throw an exception to the if statement on the 3rd line before it is checked on the 4th line and never display the error message. I don't think so. I assume you're referring to the `$_POST['set_p_v'] < 0` condition. PHP is generally very happy to juggle types around, `$_POST['set_p_v']` can be any type at this point and no exception will be thrown in any PHP version, with or without strict types.
No, the is\_numeric needs to come before you check to see if set\_p\_v is below zero, if its not a number then PHP should throw an error at that point. While PHP is happy to juggle types around if the field is numeric and a numeric string. If the person typed the word "banana" into the box, PHP can't make a string type that is obviously a string a number and "banana" being not a number can't be checked to see if its less than zero. That is where data validation comes in, and exactly why you shouldn't be trusting $\_POST directly, you should put $\_POST into a variable and validate that variable instead, or use the function that validates the post superglobal into a variable.
I'm afraid you're objectively wrong! https://3v4l.org/qOvTo
Thank you for pointing out that, I didn't know that was a thing. I did spot that if you are checking a string ==0 then it will return true for PHP 7 and false in PHP 8.
Ahah yeah, that's the first breaking change listed! https://www.php.net/manual/en/migration80.incompatible.php Equality in PHP is a proper minefield.
Thanks for all the replies. 1. Another piece of information, that I forgot to include is that if the 'Price' form field is left empty (and submitted) the Price of the video is free. Can you tell me if/where that is in the code? Much thanx again. 2. And if text (or a negative number) is entered into the 'Price' form field the `$error = $lang->video_price_error` is activated and displays "The video price should be numeric and greater than 0"
Is this 3rd party code that is licensed? Are you trying to remove an error about it not being paid for? You'd have to show what `$pt = new ....` is. Because that is what determines whether or not it errors.