php variable via link

[benanamen]

However, do you trust it just to be a number from user input?

Absolutely not! As you wisely said, NEVER trust user input

BTW, If you know it's just going to be a number then some other kind of check probably should be perform instead of the urlencode, for example: [php]// Validate the ID: if (!isset($id) || !filter_var($id, FILTER_VALIDATE_INT, array('min_range' => 1))) { throw new Exception('An invalid page ID was provided to this page.'); }[/php]

Must definitely check the data. If a number is expected, test for it. There is also is_numeric but that is not strictly numbers. A better option aside from the filter example you cite is using preg_match

[php]if(!preg_match("/^[0-9]+$/", $_GET[‘ID’]) { // not valid } else { // valid } [/php]

[m11]

Personally, I would just typecast the input, since an ID of 0 should never exist…

[php]
$id = (int) $_GET[‘ID’];
if (!$id) {
// not valid
}
[/php]

[sturobinson81]

Hi guys,

thanks for all your posts, perhaps if I include more of my code you’ll get a better picture of what I’m trying to achieve.

[php]

CHOOSE PRODUCT BELOW TO EDIT

 

<?php include 'connect.php';

	$result = mysqli_query($con,"SELECT Title, ID FROM pdfs ORDER by Title ASC");
	$num_rows = mysql_num_rows($result);
	$count = 0;
	
	echo "<br />";
	echo "<br />";
 
    
    while($row = $result->fetch_assoc()){
      $count++;
      //echo "$count." . $row['ID'] . "<h3>" . $row['Title'] . "</h3>&nbsp;&nbsp;<a href='updateproduct.php'>Edit Product</a> &nbsp; | &nbsp;  <a href="deleteproducts.php?<?=$row['ID'];"Delete Product</a>  <br /> <br />";
	 echo "$count{$row['ID']}<h3>{$row['Title']}</h3>&nbsp;&nbsp;<a href=\"updateproduct.php\">Edit Product</a> &nbsp; | &nbsp;  <a href=\"deleteproducts.php?{$row['ID']}\">Delete Product</a>  <br /> <br />";

	 
   }

?>

 

[/php]

[sturobinson81]

this is the php file that is supposed to get the ID value sent from the hyperlink in my previous post, then delete a record related to that ID.

[php]<?php

//connect to the database server
include ‘connect.php’;

//get the data from the form
$id = $_GET[‘ID’];
echo $id;

//formulate the query
$result=mysqli_query(“DELETE FROM pdfs WHERE ID=’” . $id . “’;”);

//check if there is an error
if (!$result)
{
die('Error: ’ . mysqli_error());
}
else
{
echo “1 record deleted…”;

}

//close the connection
mysqli_close();
?> [/php]

[sturobinson81]

Problem is the echo ID line is not displaying anything.

$id = $_GET[‘ID’];
echo $id;

Not sure why?

[benanamen]

Personally, I would just typecast the input,

This is a bad idea. All it will do is give you a value of zero for non numeric and you will never know it failed.

Here is an example:

[php]<?php
echo $var=(int)‘a’;// ‘a’ is clearly not an integer but we now get a value of zero which is. Php is now lying to you saying ‘a’ is an integer

if (is_int($var))
{
echo “$var is Integer
” ; //Evaluates to true because ‘a’ is now zero
}
?>[/php]

The value is zero. It will not error or do anything but give you a zero.

[benanamen]

i assume it's just this part that needs attention

Dont assume anything.

if I include more of my code you'll get a better picture of what I'm trying to achieve.

If you post the complete code you will get better and proper answers than posting pieces. As you can see from my first posts with you, it changed how the code should be when you provided the entire line.

Since you didnt post your form we cannot see why there is no $_GET[‘ID’] being set.

[sturobinson81]

Hi Kevin,

I don’t have a form…

The ID value is initially pulled from a database with the following code in

[php]
$result = mysqli_query($con,“SELECT Title, ID FROM pdfs ORDER by Title ASC”);
$num_rows = mysql_num_rows($result);
$count = 0;
[/php]

This is getting the value fine, as I echo the ID out with each row and it shows fine on the page… it’s just not getting to my second php page/file which is the file that will do the deleting.

[benanamen]

<a href=“deleteproducts.php?ID={$row[‘ID’]}”>Delete Product

Now this will work:

[php] $id = $_GET[‘ID’];
echo $id;[/php]

[sturobinson81]

Thanks very much… that works just fine I have much to learn

[m11]

This is a bad idea. All it will do is give you a value of zero for non numeric and you will never know it failed.

Here is an example:

[php]<?php
echo $var=(int)‘a’;// ‘a’ is clearly not an integer but we now get a value of zero which is. Php is now lying to you saying ‘a’ is an integer

if (is_int($var))
{
echo “$var is Integer
” ; //Evaluates to true because ‘a’ is now zero
}
?>[/php]

The value is zero. It will not error or do anything but give you a zero.

That’s exactly the point. The zero value is the failure.

[benanamen]

The zero value is the failure.

No, the failure is that you dont know it failed and send a bad query to the database.

Are you saying it is better for bad data to pass through the app, and make bad query calls to the DB putting unnecessary load on it instead of catching it first?

Consider an app with as many users as facebook or twitter. Would it be a good idea to allow millions of bad calls to the DB and put that load on it? It would be an extreme unnecessary load on the DB simply because you didn’t catch the bad data before making the DB call.

[m11]

No, the failure is that you dont know it failed and send a bad query to the database.

Are you saying it is better for bad data to pass through the app, and make bad query calls to the DB putting unnecessary load on it instead of catching it first?

Consider an app with as many users as facebook or twitter. Would it be a good idea to allow millions of bad calls to the DB and put that load on it? It would be an extreme unnecessary load on the DB simply because you didn’t catch the bad data before making the DB call.

What part of my code does not detect the failure?

[php]
$id = (int) $_GET[‘ID’];
if (!$id) {
// not valid
}
[/php]

Typecasting is one of the most important things you can do. If you aren’t doing it, you’re doing it wrong.

[benanamen]

Typecasting is one of the most important things you can do. If you aren’t doing it, you’re doing it wrong.

For an ID? really? Lets go with your code. To be clear, we are only talking about a record ID that is most usually a number ONLY.

Now lets go with your code:
[php]<?php
$_GET[‘ID’]="-8-10-5-6";// VALID INTEGER
$id = (int) $_GET[‘ID’];

if (!$id) {
echo “Not Valid”;
}
else
{
echo “Valid”;// yayyy! -8-10-5-6 is valid integer.
// I get to sneak by and make a bogus call the database and ask for ID -8
}

?>[/php]

“-8-10-5-6” is a valid integer, so according to your way, we make a bad call to the database for a record with a value of -8.

So , making bad calls to the database instead of catching and handling the error is the right way to do it according to you?

Now, lets go with the code I gave as an example.

[php]if(!preg_match("/^[0-9]+$/", $_GET[‘ID’])) {
echo “NOT Valid $id”;// Uh oh! -8 is not a valid ID we want to allow.
//We never even get to think about making a query to the database and now we can decide how to handle this error.
}
else {
echo “Valid $id”;
}[/php]

-8 will never get past the validation check and the database will not get hit with a bogus query.

So, do still say I am doing it wrong? This is not even about our personal coding opinions. The code speaks for itself.

Summary:
If I use your code, bogus query’s could be sent to my database potentially in the millions. I dont get to trap and handle the error.

If I use the code I posted, the query is NEVER sent to the DB and the database isn’t aware that anything has happened. I DO get to trap and handle the error.

[astonecipher]

I’ll throw my hat into the ring…

[php]
if (is_numeric($id) && ($id >= 0)) {
//db function call
} else {
Echo “{$id} is not valid.”;
}[/php]

If anything if fundamentally wrong with it let me know, but unless you enjoy regex it removes the need.

[benanamen]

I’ll throw my hat into the ring…

[php]
if (is_numeric($id) && ($id >= 0)) {
//db function call
} else {
Echo “{$id} is not valid.”;
}[/php]

If anything if fundamentally wrong with it let me know, but unless you enjoy regex it removes the need.

unless you enjoy regex it removes the need

Not so, it is actually worse.

Per the docs:

Finds whether the given variable is numeric. Numeric strings consist of optional sign, any number of digits, optional decimal part and optional exponential part. Thus +0123.45e6 is a valid numeric value. Hexadecimal (e.g. 0xf4c3b00c), Binary (e.g. 0b10100111001), Octal (e.g. 0777) notation is allowed too but only without sign, decimal and exponential part.

All of these return true for is_numeric. Do these look like values you would want to pass to the DB as an ID?

[php] 0x539,
02471,
0b10100111001,
1337e0,
9.1,
+0123.45e6,
0xf4c3b00c,
0b10100111001[/php]

is_numeric allows even more non-numeral characters. I was going to mention about is_numeric, but I wanted to see if anyone would suggest it.

• As I think about it, PHP should invent a function called is_numeral that only allows whole numbers (0-9)

[m11]

For an ID? really? Lets go with your code. To be clear, we are only talking about a record ID that is most usually a number ONLY.

Now lets go with your code:
[php]<?php
$_GET[‘ID’]="-8-10-5-6";// VALID INTEGER
$id = (int) $_GET[‘ID’];

if (!$id) {
echo “Not Valid”;
}
else
{
echo “Valid”;// yayyy! -8-10-5-6 is valid integer.
// I get to sneak by and make a bogus call the database and ask for ID -8
}

?>[/php]

“-8-10-5-6” is a valid integer, so according to your way, we make a bad call to the database for a record with a value of -8.

So , making bad calls to the database instead of catching and handling the error is the right way to do it according to you?

Now, lets go with the code I gave as an example.

[php]if(!preg_match("/^[0-9]+$/", $_GET[‘ID’])) {
echo “NOT Valid $id”;// Uh oh! -8 is not a valid ID we want to allow.
//We never even get to think about making a query to the database and now we can decide how to handle this error.
}
else {
echo “Valid $id”;
}[/php]

-8 will never get past the validation check and the database will not get hit with a bogus query.

So, do still say I am doing it wrong? This is not even about our personal coding opinions. The code speaks for itself.

Summary:
If I use your code, bogus query’s could be sent to my database potentially in the millions. I dont get to trap and handle the error.

If I use the code I posted, the query is NEVER sent to the DB and the database isn’t aware that anything has happened. I DO get to trap and handle the error.

You’re not making any sense here. Yes, -8 is a valid integer.

So is 929392923293293239. The same would be a “bad call to the database” in your eyes.

If you think evaluating a string as an integer is the way to go, by all means go for it.

[m11]

Not so, it is actually worse.

Per the docs:

All of these return true for is_numeric. Do these look like values you would want to pass to the DB as an ID?

[php] 0x539,
02471,
0b10100111001,
1337e0,
9.1,
+0123.45e6,
0xf4c3b00c,
0b10100111001[/php]

is_numeric allows even more non-numeral characters. I was going to mention about is_numeric, but I wanted to see if anyone would suggest it.

• As I think about it, PHP should invent a function called is_numeral that only allows whole numbers (0-9)

Even the laravel source code (which I have great respect for) shows is_numeric being used for validation. If you think you can avoid injection of invalid values to your database, you are wrong. All you can do is ensure they are not harmful to you.

[benanamen]

The whole point is, if your expecting a number ONLY, check for a number ONLY.

If you think evaluating a string as an integer is the way to go, by all means go for it.

THIS:
[php]<?php
$_GET[‘ID’]=‘clearlyAstring’;
$var=(preg_match("/^[0-9]+$/", $_GET[‘ID’]));
var_dump($var);
?>[/php]

EVALUATES TO EXACTLY THE SAME THING AS THIS:
[php]<?php
var_dump((int)$_GET[‘ID’]=‘clearlyAstring’);
?>[/php]

So your point quoted above is meaningless.

The biggest difference is the first example will allow ONLY numbers to get past which is exactly what an ID should be normally be.

So is 929392923293293239. The same would be a "bad call to the database" in your eyes.

If it does not exist as an ID then of course it is a "bad call", meaning a useless hit to the database. A useless database hit has nothing to do with "My eyes". It is just useless period. You could easily limit the max number you want to accept as an ID at this point.

With the preg function you have at least eliminated EVERYTHING that is not numbers ONLY which is what you are expecting.

[benanamen]

After a bit of research and testing it turns out that Strider provided the best answer for checking for a numeral only using FILTER_VALIDATE_INT that satisfies all the points I made about is_numeric, is_integer, preg_match, and (int) as well as the points M@tt made. See the code test below.

Cheers Strider

[php]<?php
$options = array(
‘options’ => array(
‘min_range’ => 1,
‘max_range’ => 5000000000
)
);

$var = filter_var(‘25’, FILTER_VALIDATE_INT, $options);
echo “

”;

var_dump($var); //Valid AND is integer

echo “

”;

$var = filter_var(‘0’, FILTER_VALIDATE_INT, $options);
echo “

”;

var_dump($var); //FAIL

echo “

”;

$var = filter_var(‘clearlyAstring’, FILTER_VALIDATE_INT, $options);
echo “

”;

var_dump($var); //FAIL

echo “

”;

$var = filter_var(’-8’, FILTER_VALIDATE_INT, $options);
echo “

”;

var_dump($var); //FAIL

echo “

”;

$var = filter_var(‘0x539’, FILTER_VALIDATE_INT, $options);
echo “

”;

var_dump($var); //FAIL

echo “

”;

$var = filter_var(‘02471’, FILTER_VALIDATE_INT, $options);
echo “

”;

var_dump($var); //FAIL

echo “

”;

$var = filter_var(‘0b10100111001’, FILTER_VALIDATE_INT, $options);
echo “

”;

var_dump($var); //FAIL

echo “

”;

$var = filter_var(‘1337e0’, FILTER_VALIDATE_INT, $options);
echo “

”;

var_dump($var); //FAIL

echo “

”;

$var = filter_var(‘9.1’, FILTER_VALIDATE_INT, $options);
echo “

”;

var_dump($var); //FAIL

echo “

”;

$var = filter_var(’+0123.45e6’, FILTER_VALIDATE_INT, $options);
echo “

”;

var_dump($var); //FAIL

echo “

”;

$var = filter_var(‘0xf4c3b00c’, FILTER_VALIDATE_INT, $options);
echo “

”;

var_dump($var); //FAIL

echo “

”;

$var = filter_var(‘0b10100111001’, FILTER_VALIDATE_INT, $options);
echo “

”;

var_dump($var); //FAIL

echo “

”;

$var = filter_var(‘929392923293293239’, FILTER_VALIDATE_INT, $options);
echo “

”;

var_dump($var); //FAIL - M@tt example - All numerals but exceeds max_range

echo “

”;
?>[/php]