Page 1 of 2
undefined index help
Posted: Tue Mar 20, 2007 5:20 pm
by andylyon87
hey I keep getting this when I launch the home page.
Notice: Undefined index: id in c:\program files\easyphp1-7\www\portfolio site\index.php on line 18
Line 18 onwards is as follows:
Code: Select all
$id = $_GET['id'];
if($id){
if(is_file("php/$id.php")){
include("php/$id.php");
}else{
include("php/error.php");
echo "$id";
}
}else{
include("php/home.php");
$id = "home";
}
Its used to insert some content but that doesn't really matter, line 18 as said above is the first line. This code works fine once the id has been stated but I need an amendment to make it work when no id is specified.
thanks
Andylyon87
Posted: Tue Mar 20, 2007 5:32 pm
by feyd
Posted: Tue Mar 20, 2007 5:37 pm
by andylyon87
ta mate fixed with empty(), had tried isset but couldn't implement it properly.
Code: Select all
if(!empty($_GET['id'])){
$id = $_GET['id'];
if(is_file("php/$id.php")){
include("php/$id.php");
}else{
include("php/error.php");
echo "$id";
}
}else{
include("php/home.php");
}
There is the revised code if anyone is interested
Andylyon87
Re: undefined index help
Posted: Tue Mar 20, 2007 5:52 pm
by stereofrog
andylyon87 wrote:
include("php/$id.php");
Don't do that. This code can be used to view arbitrary files from your server. Always make sure request variable is what you expect it to be:
Code: Select all
$id = @$_GET['id'];
if(ctype_alnum($id) && is_readable("php/$id.php"))
etc...
Posted: Tue Mar 20, 2007 6:04 pm
by feyd
Error suppression is not a good way to handle the existence of elements in an array.
Posted: Tue Mar 20, 2007 6:43 pm
by stereofrog
I'm not a big fan of the "@" operator myself, but in this case using it seems to make sense. Accessing undefined index is not an error in a strict sense (compare to other scripting languages).
Posted: Tue Mar 20, 2007 7:02 pm
by RobertGonzalez
In this case,
isset() would work fine without suppressing the error.
All this stuff
Code: Select all
$id = @$_GET['id'];
if(ctype_alnum($id) && is_readable("php/$id.php"))
etc...
Is just quieting down the bad that is happening. Perhaps using a little expanded logic could help in this case...
Code: Select all
<?php
if(isset($_GET['id'])) {
// This really needs to be looked at a lot more closely
$id = $_GET['id'];
$inc_file = 'php/' . $id . '.php';
if(file_exists($inc_file)) {
include $inc_file;
} else {
include 'php/error.php';
}
echo $id;
} else {
include 'php/home.php';
}
?>
Posted: Tue Mar 20, 2007 7:05 pm
by feyd
It may also be a good idea to read one of the threads referenced from Useful Posts about including...

Posted: Tue Mar 20, 2007 7:10 pm
by stereofrog
@ or isset, is a matter of taste, but this
Everah wrote:
Code: Select all
$inc_file = 'php/' . $id . '.php';
if(file_exists($inc_file)) {
include $inc_file;
should not be used under any circumstances. ctype_ (or equivalent regexp) check is mandatory.
Posted: Wed Mar 21, 2007 10:40 am
by RobertGonzalez
stereofrog wrote:@ or isset, is a matter of taste, but this
Everah wrote:
Code: Select all
$inc_file = 'php/' . $id . '.php';
if(file_exists($inc_file)) {
include $inc_file;
should not be used under any circumstances. ctype_ (or equivalent regexp) check is mandatory.
I agree with the checking. I would not include like that myself. However, suppressing an error/warning/notice is not the same as handling it. Throwing an error suppressor in front of the undefined index still results in an undefined index... you just don't see it. Checking to see if the array val is set is totally different than ignoring the fact that your code has errors.
Posted: Wed Mar 21, 2007 3:27 pm
by andylyon87
Ok guys I understand the code aint very good, but I don't understand. I changed my code to use isset now, but should I still use ctype_ and how would I implement this. My code now looks like this:
Code: Select all
if(isset($_GET['id'])){
$id = $_GET['id'];
$file = "php/$id.php";
if(is_file($file)){
include($file);
}else{
echo "<p class=\"center\"> The page: <span class=\"bold\">".$_GET['id'].".php</span> you requested is unavailable at the present time please try refreshing the page or try again later, your error has been logged and the problem will be fixed shortly.</p>";
}
}else{
include("php/home.php");
}
Any thoughts much appreciated
Andy
Posted: Wed Mar 21, 2007 3:41 pm
by RobertGonzalez
what are the page names going to look like? If they are pure alphanumeric (literally along the lines of [a-zA-Z0-9]) then you can use
ctype_alnum() to check the value.
Posted: Wed Mar 21, 2007 4:50 pm
by andylyon87
yeh they will all be a-z, but some may have ______ which can be taken out, so would I just use ctype_alnum as shown above??
I take that would go inside the isset if statement! if not where?
Posted: Wed Mar 21, 2007 5:07 pm
by RobertGonzalez
Code: Select all
<?php
if(isset($_GET['id']))
{
$id = $_GET['id'];
if (ctype_alnum($id))
{
$file = "php/$id.php";
if(file_exists($file))
{
include $file;
}
else
{
echo '<p class="center"> The page: <span class="bold">' . $id . '.php</span> you requested is unavailable at the present time please try refreshing the page or try again later, your error has been logged and the problem will be fixed shortly.</p>';
}
}
else
{
include 'php/home.php';
}
}
?>
Posted: Wed Mar 21, 2007 6:32 pm
by harrisonad
I usually set default values for my variables to ensure the no errors like that occur.
Code: Select all
$id = isset($_GET['id']) ? $_GET['id'] : 0;