8 replies to this topic

[Duke]

Hey. I thought someone here might be able to give me a hand at this. Now. I'm by no means particularly skilled with php/mysql stuff but I have a reasonable idea of how different things work and have been getting along ok but...well obviously not ok enough.

This is a little awkward to explain but i'll try my best!

<?php
/*
+-------------------------------------------------------------------------------
|	pages.php
|	====
|	Grabs the content for and sets up our pages and navigations.
|	----
|	Written: 1 Jan 2009 ~ Caleb
|		# Page names and links gotten from DB and parsed into <li></li> tags
+-------------------------------------------------------------------------------
*/

// Load up our Global Vars, functions and connect to the database
require ('global.php');
require ('functions.php');

dbconnect();

// First job is to get the page names so we can build our navigation links
	/* We then write it into the function nav_page_names (); to call in the template later on */
	
function nav_page_names() {
$page_nav_query  = "SELECT name, link FROM pages ORDER BY id ASC";
$page_nav_result = mysql_query($page_nav_query);

while($row = mysql_fetch_array($page_nav_result, MYSQL_ASSOC))

echo "<li><a href=\".$homepage./{$row['link']}\">{$row['name']}</a></li>";
}

### Now to make some pages ###

// First, we have to figure out what page is being requested

if (isset($_GET["page"]))
{
$page_url_get = $_GET["page"];

if ($page_url_get == "") // If no page is being requested we set the $show_homepage var to true for later use
$show_homepage = "true";
}
else
{
$show_homepage = "true";
}

$page_url_query  = "SELECT link FROM pages ORDER BY id ASC"; // Get the data from the link row. We're going to compare the url we've got with the id's in 'link'
$page_url_result = mysql_query($page_url_query);
while($row = mysql_fetch_array($page_url_result, MYSQL_ASSOC))

if ($row['link'] == $page_url_result){ 
	echo "Yes"; // This will be modified to show the actual data from the sql table once other problems have been resolved
}
elseif ($show_homepage = "true"){
	$content = "This is the homepage"; // This will be modified to reflect the data for 'homepage' in the DB later
}

echo $content;

?>

In the database I have a table called 'pages', with the rows 'id, 'name', 'link', 'date' and 'content'. Name is just a name of it sort of thing like 'About Us' and link is the link version of that name 'aboutus'.

Now i'm not sure if the approach i have used with the (isset($_GET is even the right idea here and will almost definitely require tweaking later on anyway. The point i'm particularly struggling with here is "
if ($row['link'] == $page_url_result){ ". Obviously i'm saying does the isset GEt in the adress bar match the results of the query for the link row. That's fine (I think), the problem is that I have no idea how to compare it one at a time with each record in the row. Naturally it's not ever going to work this way if you compare it with all the records in the row at once. But how do you tell it to check each 'link' record one at a time to see if a suitable match is found there?

All help is appreciated, please pick apart my code and tell me how useless I really am at this

[Phil]

Okay, some general advice first:

1. This may be just me, but I find it very hard to understand code that isn't properly formatted. I usually indent everything, to make nested blocks more readable.
IMO the first is far easier on the eye than the second:

if ( isset($_GET["page"]) )
{
	$page_url_get = $_GET["page"];

	if ($page_url_get == "") // If no page is being requested we set the $show_homepage var to true for later use
	{
		$show_homepage = "true";
	}
}
else
{
	$show_homepage = "true";
}

if (isset($_GET["page"]))
{
$page_url_get = $_GET["page"];

if ($page_url_get == "") // If no page is being requested we set the $show_homepage var to true for later use
$show_homepage = "true";
}
else
{
$show_homepage = "true";
}

2. Use curly braces. I know many people omit them for single-liners, but whenever you wish to add more than one line, it bears the danger of forgetting them, which will totally screw up your logic and you probably won't notice it. Your while block at the end is such an example that makes very little sense logically.

3. PHP features a real boolean type (i.e. true and false). Writing them as strings is unnecessary (even though PHP is "smart" enough to evalute it to the right thing).

4. Watch out with assignments and comparisons. The following will always evaluate to true because you forgot the second equal sign:

elseif ( $show_homepage = "true" )

5. Never trust the user! Accepting values from the user without properly validating them is suicide. In case of the page name, you could use a regular expression to filter it. Otherwise people will be able to pull off cross-site scripting, can read files they're not supposed to read or, in your case, use SQL injection to do some nasty stuff.
Here's an example with filtering (it could still be improved a lot, but it works):

if( isset($_GET['page']) && $_GET['page'] !== '' )
{
	$page = $_GET['page'];
}
else
{
	$page = $default_page;
}

// filter out anything but alphanumeric characters, the hyphen and the underscore
if( preg_match("/^[a-zA-Z0-9_\-]*$/", $page) == 0 )
{
	$page = false;
}

if( $page )
{
	$include = $include_path.$page.".php";

	
	// check if the file we want to include exists
	if( ! file_exists($include) ) 
	{
		// file not found - reset the include file
		$include = $include_path.$error_file;
	}
	
}
else
{
	$include = $include_path.$error_file;
}

// include the file
require_once $include;

6. Save yourself some variables and increase readability by writing the SQL query directly into the function call, like so (unless, of course, you are going to reuse the exact same query many times):

$page_nav_result = mysql_query("SELECT name, link FROM pages ORDER BY id ASC");

Now, onto your problem:
I am not totally sure I have understood what exactly you want to do, but if you want to find the matching entry for `link` in your table, it would be better to let MySQL do the searching because it is far more efficient. E.g.

SELECT * FROM `pages` WHERE `link` = 'linkname';

[Mastermind]

Also, you can just use true for true, not "true", "true" is a string, true is a boolean. That way you're not doing string comparisons.

[Duke]

Thank you both. Very much. Especially to DLOTS who walked me through a bit of the confusing stuff on msn! I may be back for more questions though sometime i'm sure

[Duke]

Ok so it's been a wee while since my last question but i'm back! (not surprisingly). However i've learned a lot in the past week and i'm getting a lot better. Anyway, I've still managed to find myself quite convincingly - stuck. I have a section where I want you to be able to delete a row from the database by clicking on a link. Which then uses a js confirmation thing (i'm even less experienced with js so I may be doing this all wrong).

I've tried a few different ideas that I could come up with and then resorted to google but couldn't really find what I was after there either because it has to combine php and js.

Here's what i've got at the moment

The related section of the main file:

elseif ($type == "delete"){
	$content = "Are you sure you want to Delete the page <a href=\"index.php?act=pages&page={$page_link}\" onclick=\"deletepage()\">{$page_title}</a>?";
	page_delete($page_link);
}

The $content variable is what is shown on a certain part of a page, basically to give you a visual the links of all the pages are listed in a left column and on the right is an area where you can edit the page you've chosen or show the delete message (above).

$page_title is obviously the title for a page. $page_link is the url friendly version so the title would be 'Contact Us' where link is 'contact_us' for a contact page. These have been got from the db a few lines earlier. Anyway, at the moment i'm just calling the function page_delete function with the $page_link var so we delete the right thing. What I need to do is be able to get that function to execute if the js statement below ends up true.

snippet of functions.php

function page_delete($page_link){
	mysql_query("DELETE FROM pages WHERE link='$page_link';") 
	or die(mysql_error()); 
}
?>	
<script language="JavaScript" type="text/javascript">
function deletepage() {
	if (confirm("Are you sure you want to delete this page?")) {
		document.write("Page Deleted");
	} else {
	}
}

</script>

Any ideas anyone? To reiterate: I need a way to execute the php function delete_page with the variable $page_link via the above js function. Currently the php delete_page function is executed when the link ?act=pages&delete=$ and the page in the url there will be deleted. There's just no confirmation from the js function which is what I want to happen.

Help..please.

[Phil]

I think you're getting JavaScript and PHP confused. PHP is strictly server-side, so whatever you do there will be executed before the user has a chance to interact with the site. JavaScript is client-side, so everything you do there happens after the page has been processed by PHP. In other words, you cannot check for JS user input inside your PHP script.

What I normally do is:
- generate a link with the ID/name of the item to be deleted (e.g. blabla.php?act=delete&id=3 ) in PHP
- serve the page
- let the user click the link
- handle the new request with PHP

In order to get the JS confirmation box you can just attach the script to that link:

<a href="blabla.php?act=delete&amp;id=3" onclick="return confirm('Are you sure???');">Delete this item</a>

If the user has JavaScript turned off, the link is just clicked normally, otherwise the confirmation box kicks in. If the function returns false (i.e. the user clicked "cancel"), the link will not be followed.

[Duke]

Right'e-oh. Back for another question. This time - it's about Forms. Let's say I have an initiator type script (index.php) which ultimately sets up database connections, session starts etc. and using a dynamic url (?act=something) I have a form shown on the page. When you view it you can see the things you want to see, say if I want to show what user is logged in, however when you submit that form to 'act/something.php' the browser will be sent to that page. Which is okay, I can send the browser back to the dynamic url again however as soon as we get to that *static* page we don't have direct access to any of the mysql connection and session stuff we defined in the initiator.

Anyone got any methods for beating this?

[Phil]

Either set up your DB connections, sessions, etc. in another "real" init script, which you can include from everywhere (also from your static download.php script)
or set the form action to index.php?act=something.

[Bart]

Yes, why not just direct the form to your index.php?

What I always do is this. I make a common init script (called bootstrap.php) which basically sets everything up. Then in index.php
- I include the script
- I parse the requested page and call the correct action (often called dispatching)

Now, when I have to make a special page (which I rarely do) or a command line script, I can just include the bootstrap file, then do whatever I need to do.

Also, I'd apply URL rewriting, so instead of /index.php?act=something you just get /something. There are various ways to do this, but the best is to use a .htaccess with RewriteRules to direct all requests to index.php, except those that are real files:

RewriteEngine On
RewriteCond %{REQUEST_FILENAME} -s [OR]
RewriteCond %{REQUEST_FILENAME} -l [OR]
RewriteCond %{REQUEST_FILENAME} -d
RewriteRule ^.*$ /index.php [L]

Then, in index.php, read the value of $_SERVER['REQUEST_URI'] (which will be something like "/something/more?any=data") and use that to call the correct action.

By the way, this pattern is called a Front Controller. Maybe you can search around for it.