undefined index help

PHP programming forum. Ask questions or help people concerning PHP code. Don't understand a function? Need help implementing a class? Don't understand a class? Here is where to ask. Remember to do your homework!

Moderator: General Moderators

andylyon87
Forum Contributor
Posts: 168
Joined: Sat Jan 31, 2004 5:31 am
Location: Dundee

undefined index help

Post 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
User avatar
feyd
Neighborhood Spidermoddy
Posts: 31559
Joined: Mon Mar 29, 2004 3:24 pm
Location: Bothell, Washington, USA

Post by feyd »

isset(), empty(), array_key_exists() should all be of interest.
andylyon87
Forum Contributor
Posts: 168
Joined: Sat Jan 31, 2004 5:31 am
Location: Dundee

Post 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
User avatar
stereofrog
Forum Contributor
Posts: 386
Joined: Mon Dec 04, 2006 6:10 am

Re: undefined index help

Post 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...
User avatar
feyd
Neighborhood Spidermoddy
Posts: 31559
Joined: Mon Mar 29, 2004 3:24 pm
Location: Bothell, Washington, USA

Post by feyd »

Error suppression is not a good way to handle the existence of elements in an array.
User avatar
stereofrog
Forum Contributor
Posts: 386
Joined: Mon Dec 04, 2006 6:10 am

Post 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).
User avatar
RobertGonzalez
Site Administrator
Posts: 14293
Joined: Tue Sep 09, 2003 6:04 pm
Location: Fremont, CA, USA

Post 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';
}
?>
User avatar
feyd
Neighborhood Spidermoddy
Posts: 31559
Joined: Mon Mar 29, 2004 3:24 pm
Location: Bothell, Washington, USA

Post by feyd »

It may also be a good idea to read one of the threads referenced from Useful Posts about including... ;)
User avatar
stereofrog
Forum Contributor
Posts: 386
Joined: Mon Dec 04, 2006 6:10 am

Post 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.
User avatar
RobertGonzalez
Site Administrator
Posts: 14293
Joined: Tue Sep 09, 2003 6:04 pm
Location: Fremont, CA, USA

Post 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.
andylyon87
Forum Contributor
Posts: 168
Joined: Sat Jan 31, 2004 5:31 am
Location: Dundee

Post 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
User avatar
RobertGonzalez
Site Administrator
Posts: 14293
Joined: Tue Sep 09, 2003 6:04 pm
Location: Fremont, CA, USA

Post 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.
andylyon87
Forum Contributor
Posts: 168
Joined: Sat Jan 31, 2004 5:31 am
Location: Dundee

Post 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?
User avatar
RobertGonzalez
Site Administrator
Posts: 14293
Joined: Tue Sep 09, 2003 6:04 pm
Location: Fremont, CA, USA

Post 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';
    }
}
?>
User avatar
harrisonad
Forum Contributor
Posts: 288
Joined: Fri Oct 15, 2004 4:58 am
Location: Philippines
Contact:

Post 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;
Post Reply