How come this doesn't work?!

[attila2452]

<?php if(isset($_GET["page"]) && !empty($_GET["page"]) && is_string($_GET["page"])) if (file_exists("" . $_GET["page"] . ".php")) { require_once($_GET["page"] . ".php"); } else { echo " <br /> <h1>Page Not Found!</h1> <br /> <br /> <p>Please make sure that the url like was corrected!</p> "; } else { require_once "welcome.php"; } ?>

[justsomeguy]

It's ambiguous, PHP doesn't know which if statements the elses go to. Use brackets to make it unambiguous.

[thescientist]

Have you tried indenting your code? If you do something like this:

<?phpif (isset($_GET["page"]) && !empty($_GET["page"]) && is_string($_GET["page"]))  if (file_exists("" . $_GET["page"] . ".php")) {	require_once($_GET["page"] . ".php");  }else {	echo "<br /><h1>Page Not Found!</h1><br /><br /><p>Please make sure that the url like was corrected!</p>";  }else {	require_once "welcome.php";  }?>

you'll see that you have quite a few structural issues with your control statements. missing opening/closing curly braces and parenthesis, improper nesting, etc. Sort those out and see what happens.

[attila2452]

Have you tried indenting your code? If you do something like this:

<?phpif (isset($_GET["page"]) && !empty($_GET["page"]) && is_string($_GET["page"]))  if (file_exists("" . $_GET["page"] . ".php")) {	require_once($_GET["page"] . ".php");  }else {	echo "<br /><h1>Page Not Found!</h1><br /><br /><p>Please make sure that the url like was corrected!</p>";  }else {	require_once "welcome.php";  }?>

you'll see that you have quite a few structural issues with your control statements. missing opening/closing curly braces and parenthesis, improper nesting, etc. Sort those out and see what happens.

I can't find anything wrong with it. im using the exact same one on another website and it works fine for some reason, its not working on this design.

[jeffman]

It is legal in PHP for an if(condition) to be followed by a single statement without using curly braces. But when you follow it with multiple additional if(condition)s and else clauses, you can confuse the logic. Everyone is strongly recommending that you follow your first line with a curly brace and add a closing brace in the appropriate location.

[attila2452]

It is legal in PHP for an if(condition) to be followed by a single statement without using curly braces. But when you follow it with multiple additional if(condition)s and else clauses, you can confuse the logic. Everyone is strongly recommending that you follow your first line with a curly brace and add a closing brace in the appropriate location.

where would that be in mine?

[justsomeguy]

You should be able to answer that yourself. Where do you want this if statement to end?if(isset($_GET["page"]) && !empty($_GET["page"]) && is_string($_GET["page"]))

[attila2452]

You should be able to answer that yourself. Where do you want this if statement to end?if(isset($_GET["page"]) && !empty($_GET["page"]) && is_string($_GET["page"]))

im not sure, im not sure what it means. im just learning PHP.

[justsomeguy]

OK, then let's go through it. What does each line do. What about this line:if(isset($_GET["page"]) && !empty($_GET["page"]) && is_string($_GET["page"]))That uses isset, empty, and is_string. What is that line checking for?http://www.php.net/manual/en/function.isset.phphttp://www.php.net/manual/en/function.empty.phphttp://www.php.net/manual/en/function.is-string.phpWhat about this, what is this doing:

if (file_exists("" . $_GET["page"] . ".php")) {  require_once($_GET["page"] . ".php");}

That uses file_exists and require_once, what is it trying to do?Your code isn't very complex, it shouldn't be very hard to understand. But in order for you to be able to find out where the problem is and fix it, you need to understand at least what the code is trying to do.

[jeffman]

please ignore.

[attila2452]

OK, then let's go through it. What does each line do. What about this line:if(isset($_GET["page"]) && !empty($_GET["page"]) && is_string($_GET["page"]))That uses isset, empty, and is_string. What is that line checking for?http://www.php.net/manual/en/function.isset.phphttp://www.php.net/manual/en/function.empty.phphttp://www.php.net/manual/en/function.is-string.phpWhat about this, what is this doing:

if (file_exists("" . $_GET["page"] . ".php")) {  require_once($_GET["page"] . ".php");}

That uses file_exists and require_once, what is it trying to do?Your code isn't very complex, it shouldn't be very hard to understand. But in order for you to be able to find out where the problem is and fix it, you need to understand at least what the code is trying to do.

its checking if the file its looking for exists to clear the prefix, get the page and add.phpand ten to require page once.am i right?

[thescientist]

It is legal in PHP for an if(condition) to be followed by a single statement without using curly braces. But when you follow it with multiple additional if(condition)s and else clauses, you can confuse the logic. Everyone is strongly recommending that you follow your first line with a curly brace and add a closing brace in the appropriate location.

also, shouldn't that entire first if statement be wrapped in matched parens? I see one missing.

[justsomeguy]

I don't see a missing paren, those look fine to me.

its checking if the file its looking for exists to clear the prefix, get the page and add.phpand ten to require page once.

Basically, yeah. It's using $_GET['page'], which goes in the URL (e.g. file.php?page=xxx), and it's checking if there is a PHP file with that name. If there is, it includes it. A require_once statement tells it to include the code only once (i.e., if you have 2 require_once statements both including the same file, it will only do it once), and since it's require instead of include then it tells PHP to quit if the file could not be included.So, one part of the script tells it to include a file based on $_GET['page']. So then what does the first if statement check for:if(isset($_GET["page"]) && !empty($_GET["page"]) && is_string($_GET["page"]))What is that doing? The ! before empty means "not", you can read "!empty" like "not empty".

[trinistorm]

Sorry if I'm off topic but isn't

if (file_exists("" . $_GET["page"] . ".php")) {  require_once($_GET["page"] . ".php");}

a security risk? Your letting the script load a file based on values passed in the address bar. "http://yourscript.php?page=C:\Program Files\apache\conf\httpd.conf"

[thescientist]

woops, I must have lost it when formatting it.

[attila2452]

I don't see a missing paren, those look fine to me.Basically, yeah. It's using $_GET['page'], which goes in the URL (e.g. file.php?page=xxx), and it's checking if there is a PHP file with that name. If there is, it includes it. A require_once statement tells it to include the code only once (i.e., if you have 2 require_once statements both including the same file, it will only do it once), and since it's require instead of include then it tells PHP to quit if the file could not be included.So, one part of the script tells it to include a file based on $_GET['page']. So then what does the first if statement check for:if(isset($_GET["page"]) && !empty($_GET["page"]) && is_string($_GET["page"]))What is that doing? The ! before empty means "not", you can read "!empty" like "not empty".

if(isset($_GET["page"]) && !empty($_GET["page"]) && is_string($_GET["page"]))if the file is there, and is not empty and if the url is a stringso its a check for if it exists, if iti s not empty and if its a string.

[boen_robot]

Sorry if I'm off topic but isn't

if (file_exists("" . $_GET["page"] . ".php")) {  require_once($_GET["page"] . ".php");}

a security risk? Your letting the script load a file based on values passed in the address bar. "http://yourscript.php?page=C:\Program Files\apache\conf\httpd.conf"

It's a security risk only if there's a "dangerous" or "secret" PHP file that must not be executed by end users in this fashion. The ".php" part ensures that a PHP file will be executed... whether it's a known or appropriate PHP file is another question.That's why to make the possible PHP files be known, a usual practice is to get the realpath() of the file, and see if it starts with the expted base path (in which you're certain all files are appropriate for inclusion). If it doesn't, or realpath() returns false, then this is an attack attempt, and according action (like, loading a default page for example) should be taken.

[justsomeguy]

Your letting the script load a file based on values passed in the address bar. "http://yourscript.php?page=C:\Program Files\apache\conf\httpd.conf"

That would try to include a file called C:\Program Files\apache\conf\httpd.conf.php

so its a check for if it exists, if iti s not empty and if its a string.

Right, that's basically checking if the URL includes the page variable. But, that can actually be easier. It's redundant to use both isset and empty, because empty also checks if it's set. So, you can actually just use this:if(!empty($_GET["page"]) && is_string($_GET["page"]))That will work the same, it's just checking if the URL includes a value for "page" and that it's a string. The reason for is_string is because you could make page an array if you did this:page.php?page[]=1&page[]=2That would still make a variable called $_GET['page'], but it would be an array with 2 items instead of a string.So, the first if statement checks if $_GET['page'] is a non-empty string. Then, if a file exists with that name, include it. So far, that looks like this:

if(!empty($_GET["page"]) && is_string($_GET["page"])){  if (file_exists($_GET["page"] . ".php")) {	require_once($_GET["page"] . ".php");  }}

Next, you have an else statement to show a file not found message. It sounds like it should show file not found if $_GET['page'] is valid, but the file does not exist. So that else should go with the inner if statement, like this:

if(!empty($_GET["page"]) && is_string($_GET["page"])){  if (file_exists($_GET["page"] . ".php")) {	require_once($_GET["page"] . ".php");  }  else {	echo "	<br />	<h1>Page Not Found!</h1>	<br />	<br />	<p>Please make sure that the url like was corrected!</p>	";  }}

The other else statement always loads welcome.php, so it sounds like you want to load that file if $_GET['page'] was not given. So, that else belongs with the outer if statement which checks for $_GET['page']:

if(!empty($_GET["page"]) && is_string($_GET["page"])){  if (file_exists($_GET["page"] . ".php"))   {	require_once($_GET["page"] . ".php");  }  else   {	echo "	<br />	<h1>Page Not Found!</h1>	<br />	<br />	<p>Please make sure that the url like was corrected!</p>	";  }}else {  require_once "welcome.php";}

Does that make sense to you? If not, what doesn't make sense?

[trinistorm]

Oops my bad I misread the statement.