Handling 404 page

[nayen]

Hi,

I have dynamic pages on my website where I use a post.php file to display single posts that are retrieved from the database. I have the following at the top of my post.php:

$post = get_post($_GET['p']);

And here is the get_post function declaration:

function get_post($p) {
	$query = "SELECT * FROM posts WHERE post_name = '$p'";
	if ($query && mysqli_fetch_assoc(DB::$db->query($query)) > 0) {
		$post = mysqli_fetch_assoc(DB::$db->query($query));
		return $post;
	} else {
		header('HTTP/1.1 404 Not Found');
		header('Location: /404.php');
	}
}

This function checks if a given post exists, displays the post if it exists and displays the 404 page if it doesn’t. I read that redirecting to the 404 page was not recommended but I couldn’t think of an alternative way to handle the 404 error. Any ideas if what I am doing is ok or not?

Thanks.

[Rubble]

@nayen

else {
header(‘HTTP/1.1 404 Not Found’);
header(‘Location: /404.php’);
}

How about:
else {
$post = “Sorry that post was not found”;
return $post;
}

Or something along those lines?

I assume you are validating $p somewhere?

[nayen]

@Rubble

else {
		$post = "Sorry that post was not found";
        return $post;
	}

If I remove header(‘HTTP/1.1 404 Not Found’); the error will not be triggered and logged on server side, right? Also, I want to display the custom 404 page which is styled differently than post.php.

Any suggestions for validation?

Thanks.

EDIT:

I have the validation in .htaccess in the rewrite rule:

RewriteRule ^([a-z0-9-]+)/$ post.php?p=$1

[Rubble]

One thing I would do for validation is have another query and select all the values for $post and then check the value of $post was in that array. You could also use this to trigger your error. If $post is valid then do your query; it just means writing your function slightly differently.

You could log any errors to a text file or send yourself an email. You could expand this $post = “Sorry that post was not found”; to have styling etc. if you wanted.

[spaceshiptrooper]

It seems like you aren’t using prepared statements when you should be. You are prone to SQL Injection because you are stuffing the actual string into your SQL statement. Also, your logic of checking if records exist seems to be an odd one along with your $p variable not being sanitized or validated.

I have cleaned up your snippet and made it less prone to SQL Injection. You can choose to use my snippet or not. In the end, I’m not the one who has to deal with users having their accounts compromised. You will be.

All of my snippets follow the same logic and same syntax. You can also use the logic with PDO if you want.

<?php
function get_post($p) {

    $p = filter_var($p, FILTER_SANITIZE_STRING, FILTER_FLAG_STRIP_HIGH); // Sanitize your string

    $sql = "SELECT id, post_name, post_date FROM posts WHERE post_name = ?"; // Your sql statement
    $query = DB::$db->prepare($sql); // DB:: Prepare the sql statement
    $query->bind_param('s', $p); // Bind the placeholder to your $p variable with the string type
    // s = string | i = integer | d = double | b = blob
    $query->execute(); // Execute the prepared statement
    $query->store_result(); // Store the result for later checking

    // Check to see if the result returns a 0 or 1. 0 meaning false and 1 meaning true.
    // Don't do num_rows > 0
    // I've seen this in a lot of amateur work.
    // The result will already returns a 0 or 1
    // By doing num_rows > 0
    // You are being redundant
    if($query->num_rows) {

        $query->bind_result($id, $post_name, $post_date); // Append the columns with their specific variable

        // Throw them in a while loop
        while($query->fetch()) {

            // Throw the variables in an array
            // This is an alternative to MySQLi's get_result()
            // get_result() is only available in PHP 5.3 +
            // I'm not sure if this is correct, because I remember get_result() breaking
            // In PHP 5.4
            $post = array('id' => $id, 'post_name' => $post_name, 'post_date' => $post_date);
            return $post; // Return the result

        }

    } else {

        return false; // Return false since there are no results/ records

    }

}

// Check to see if your function returns false
if(get_post('sample') == false) {

    print('No results were found'); // The search has ended in false which means there are no results with that name
    // You can also you use your redirect header now.
    // Just un-comment them out and remove the print('No results were found'); part
    // header('HTTP/1.1 404 Not Found');
    // header('Location: /404.php');

} else {

    print_r(get_post('sample')); // The result is going to be an array so you have to use print_r instead of print

}

[chrisofarabia]

Think you have a very small typo on the last line of code “sampe” vs “sample”

[Olami]

the code seems not to be valid, check it very well and validate and try and test again

[nayen]

@spaceshiptrooper Thank you for your detailed reply and code sample. I am all for simplicity in my coding as long as it does the required job and it is secure enough. The rewrite rule I have

RewriteRule ^([a-z0-9-]+)/$ post.php?p=$1

already does the filtering, why would I need to do additional filtering/validation? Just asking to learn if I am missing anything.

Can you elaborate why I should be using prepared statements. Do they provide performance/speed advantages in my specific case?

I’m just trying to patch together whatever info I can find around the web to do what I need to do as there is no definitive guide that teaches everything in the right/recommended way. The code I have is taken from some tutorial I found somewhere. Anyway, aside from the validation, my main question was if it is ok to redirect to the 404 page. Seems one alternative is not to redirect but to display the 404 page content within the else block. I still don’t have a clear idea about which way to go and why.

[spaceshiptrooper]

Simple isn’t always best. That’s how I thought too when I was beginning PHP back in 2010. But I was totally wrong. Also that isn’t filtering. That’s just rewriting the URL to be pretty. Like

/spaceshiptrooper

instead of

/?id=224625

Apache isn’t the package that is going to be dealing with user inputs. It’ll be PHP who is doing it. And thus will bring us to prepared statements.

Prepared statement is suppose to separate user input from SQL statement. If you deliberately stuff your variable in your SQL statement, someone can actually hack your website with ease. And it would really suck to be your users who are storing their personal information on your website. They could get their credit card stolen, identity theft, and if possible, you could face criminal charges if it gets even that far. That’s only if a user gets really out of hand and calls the FBI on you. Not sure if that’ll happen, but it could be possible since people’s personal information are being stored where there is no security at all.

Anyways, back on topic. When you use prepared statements, you aren’t deliberately stuffing your user input into your SQL statement and it separates user input from code.

So if someone wanted to use this line as their input

' UNION SELECT 1, 2, 3, …, n --

It will be nothing more than just a string that has no meaning. But if you don’t use prepared statements, PHP will assume that

' UNION SELECT 1, 2, 3, …, n --

Is part of your SQL statement thus it might select all your tables from your database and display every single entry you have stored in what ever database you are using to a random user.

You can actually just do a Google search on it.

https://www.google.com/search?q=php+sql+injection

---

Heard of Bobby Tables?

---

As per your second question. It doesn’t actually matter. If you want to redirect the user to a 404 page, that’s fine. Same thing with displaying it to them right from the page. It’s basically the same thing in a way since you are still displaying a 404 error message.

[nayen]

@spaceshiptrooper

Also that isn’t filtering. That’s just rewriting the URL to be pretty.

Hi and thanks for your input. I know what you mean but just to give you a different perspective, the rewrite rule I am using, actually does filtering.

RewriteRule ^([a-z0-9-]+)/$ post.php?p=$1

It takes URLs that have only characters from a to z and from 0 to 9 and -, and sends to my post file, which display the post. If there’s any other character in the URL, it won’t send the user to my post file.

I understand your comments about security and I know about SQL injection, but my site does not involve any user interaction other than viewing pages, so I’m seeking the simplest ways to accomplish what I need. Not looking for building unnecessary layers of security, where they will not be doing any additional job.

[spaceshiptrooper]

@nayen

Hi and thanks for your input. I know what you mean but just to give you a different perspective, the rewrite rule I am using, actually does filtering.

RewriteRule ^([a-z0-9-]+)/$ post.php?p=$1

It takes URLs that have only characters from a to z and from 0 to 9 and -, and sends to my post file, which display the post. If there’s any other character in the URL, it won’t send the user to my post file.

I understand your comments about security and I know about SQL injection, but my site does not involve any user interaction other than viewing pages, so I’m seeking the simplest ways to accomplish what I need. Not looking for building unnecessary layers of security, where they will not be doing any additional job.

I believe you have set your mind to believing that Apache’s RewriteRule will handle all of your security and you’re actually wrong. Apache is not a package that you should be putting your trust into when it comes to security. You should be using the procedures that you are doing your work on which is PHP. If you take a look at the RewriteRule, it is just making the URL pretty. But then, if you look at what the original file was, you can actually access that file instead of the pretty URL. Now all I (the user) just needs is the URL and poof, I can basically bypass all of this “filtering” and do my SQL injection simply because you have this set mind set that Apache’s RewriteRule some how magically “protected” you from me trying to bypass all of this “filtering”.

You don’t have to listen to any of us if you don’t want to. You’re just putting your website at harm and maybe even the hosting server you are hosted on. I wish you good luck on that. I already warned you, but if your mind is already set on using Apache’s RewriteRule as a “filtering” whatever that means, then I won’t stop you. Just remember that I already warned you.

[nayen]

Just trying to figure out the simplest “enough” way, that’s all I’m about. Thanks again, with the light of your warnings and sample code, I will further my research about security topic and see how I can improve my scripts.

[felgall]

@nayen

Just trying to figure out the simplest “enough” way

Validating all inputs in the PHP file is the bare minimum - if you don’t do that then you haven’t done enough - if you do more than that then it is more than the minimum.