sending visitors to an error page

[webdeveloper123]

Hi,

I've got some code here that works for re-directing the user to an custom error page if there ID in the query string is not in the database. Problem is that although it works, the last record will always re-direct to the error page even though that ID exists in the database. So I realised this at Id 17, redirected me to an error page, than I added a new record (18) and now 17 shows me to the page its supposed to take me to, but now 18 is re directing to the error page. Doesn't make any sense.

Here is my code:

include 'includes/db.php'; 
include 'includes/functions.php';

$keys       = "SELECT customer_id FROM customer_details;";
$statement = $pdo->query($keys);
$members   = $statement->fetchAll();

//var_dump($members);

$id = $_GET['user_id'] ?? '';

$valid = array_key_exists($id, $members);

if (!$valid) {
    http_response_code(404);
    header('Location: page-not-found.php');
    exit;
}

Shall I paste my functions.php include file?  it's pretty long.

Thanks

[ginerjm]

You're seem to be talking about a loop process, but not showing us this loop. So to us, it simply means you don't have the url's id in your array of members.  I see nothing wrong here.

[benanamen]

First, you should validate that you have a GET id at all and if so that it is a proper id format, i.e all numbers.

If you have a GET id, then you should use that in a WHERE condition in your query and go from there.

Your error response could be a problem though. If this is public facing you are building in a user id enumeration attack. In the case of a blog page or something a not found error is fine, but you are using customer_id's. If this is behind a secured admin then not a problem, but then you have to ask how you would end up with a non-existent customer_id to even get the error which really should only be able to happen by manually manipulating the URL.

Edited July 21, 2022 by benanamen

[webdeveloper123]

11 minutes ago, ginerjm said:

You're seem to be talking about a loop process, but not showing us this loop.

There is no loop, that's the whole code.

@benanamen - Im trying to validate the query string here. So If I have members 1 to 100 it will take me to the correct next page, but if 101 is entered it takes me to an error page

[benanamen]

1 minute ago, webdeveloper123 said:

but if 101 is entered it takes me to an error page

Entered how?

A high level overview of what you are doing would be helpful to give you the best and more specific advice.

[requinix]

To make sure I'm understanding what's going on so far,

You have an ID in $_GET. To see if that's a valid ID, you grab every single ID from the database and then check to see if the one you want is in there?

[ginerjm]

Right now it appears that you have coded it to do just what you have described.  Assuming that there is more code following to handle the valid response.

BTW  what is your default fetch mode?

[webdeveloper123]

5 minutes ago, requinix said:

You have an ID in $_GET. To see if that's a valid ID, you grab every single ID from the database and then check to see if the one you want is in there?

yes, correct!

 

6 minutes ago, benanamen said:

A high level overview of what you are doing would be helpful to give you the best and more specific advice.

https://example.com/customer/edit.php?user_id=7 - So basically I am validating the user id here, in this case example 7 (which exists and takes me to the correct next page)  I can see your point it would have to be manually done to get to the error page but I thought it was a good place to start. So if i only have users 1 - 100 in there and someone types in 101 (which doesn't exist) they should be shown to an error page

 

5 minutes ago, ginerjm said:

BTW  what is your default fetch mode?

I'm using fetchAll for this one. Where is the default fetch mode saved? 

Edited July 21, 2022 by webdeveloper123

[ginerjm]

I really don't see any problem here.  You are describing something as if it were an issue but your code does exactly what you have said you wanted it to do.

[webdeveloper123]

2 minutes ago, ginerjm said:

but your code does exactly what you have said you wanted it to do.

Then why does the last record ALWAYS take me to an error page, even though the ID & the record exists in the database

Edited July 21, 2022 by webdeveloper123

[webdeveloper123]

And it's always the last id, not the first or 2nd or 3rd, always the last one

[ginerjm]

I do not believe you.  I used your code to query a table of mine that pulled 20 rows and every  one that I test it with shows correctly.  First, middle and last.  Check your data.  Perhaps that last one has a character in it you don't expect.

[webdeveloper123]

lol why would I lie and make this up? I could be getting on with validating the rest of my code!

[ginerjm]

Just telling you what works.

Good bye.

[benanamen]

This is basically it...

* Your biggest problem is that you are selecting ALL the customer_id's. Only SELECT the one you want.
 

if ($_SERVER['REQUEST_METHOD'] === 'GET') {
    
    // check if GET customer_id isset
    
    // if customer_id isset validate (check format)
    
    // if no problem
    
    // query db - USE A PREPARED QUERY HERE. THIS IS JUST FOR DEMONSTRATION
     SELECT customer_id FROM customer_details WHERE customer_id = $_GET['customer_id']
    
     // Do something with result. * You likely need to SELECT more columns than just the customer_id
    
}

 

Edited July 21, 2022 by benanamen

[webdeveloper123]

@benanamen - but that's not what I am trying to achieve. this is very strange

[ginerjm]

I see you skimmed over my last post's suggestion that you check your data to see what you "think" is your last id.  That's the problem.

[benanamen]

2 minutes ago, webdeveloper123 said:

that's not what I am trying to achieve.

I am lost then. How about you give us the high level overview that I asked you for.

[webdeveloper123]

@requinix nailed it. What he said is what i am trying to acheive

Use Get - Store the customer Id into a variable from the query string

Sql select to pull all customer id from the customer table

execute in pdo

save resultset into members array

check to see if the customer ID I got in get exists in the database (e.g compare it against the resultset in members array

if its not valid, take me to custom error page 

otherwise take me to the correct next page (which in this case is a form to edit the record in the database) 

thats it

Maybe there is something in the functions file stopping it? 

[webdeveloper123]

btw this custom error page won't load unless I include the functions.php file

[benanamen]

Your "logic" is all over the place.

The bottom line is you want to edit a customers record based on the customer_id. (Which, by the way is the high level overview I was looking for, not the steps you think you should be taking to do it.)

I showed you how to do it. If there are no results to the specific customer_id query, then show your error page or whatever.

You might want to read my signature about the "XY Problem".

Edited July 21, 2022 by benanamen

[requinix]

18 minutes ago, webdeveloper123 said:

@requinix nailed it. What he said is what i am trying to acheive

I was actually trying to point out that you're attempting to solve your problem the wrong way.

If you have an ID and want to make sure it exists then what you do is execute a query to see if there are rows matching that ID. You do not retrieve every single one. That's terribly, terribly inefficient.

SELECT 1 FROM customer_details WHERE customer_id = ?

Use that as a prepared statement and put your expected ID value into it as a parameter. Then execute and see if you got any rows back.

But what's more, I'm skeptical you even need this at all. What are you going to do later on? You said redirect to yet another page? Why? Why can't you check that the row exists on that page? And you know, when you do that, your edit page is going to need to retrieve the records from customer_details too, and won't that look very, very much like the above query? So not only do you not need this page, you don't even need this query because your edit page will find out if the ID doesn't exist when it tries to load up the data it needs.

Does that make sense?

[phppup]

I'm with @requinix on this one.

Although I don't enjoy PDO and haven't used GET, I would approach this by starting this way

$sql = "SELECT id FROM yourTable";
if($result = mysqli_query($conn, $sql)){
    if(mysqli_num_rows($result) > 0){
        

//do stuff to the record that was found  
//and since an id is unique, it will either exist or not
    } else{
        echo "No record was found.";
    }
}

You can then adapt the code with another condition of it's between 0 and 101.

BTW, if you're pulling your current info from an array, remember that the FIRST position is ZERO. This could be the problem also.

Edited July 21, 2022 by phppup
Forgot item

[benanamen]

6 minutes ago, phppup said:

I would approach this by starting this way

Then you would be doing it wrong. The right way has been provided in earlier threads.

//do stuff to the record that was found  
//and since an id is unique, it will either exist or not

What record? You just selected every single one of them.

Edited July 21, 2022 by benanamen

[cyberRobot]

2 hours ago, webdeveloper123 said:

And it's always the last id, not the first or 2nd or 3rd, always the last one

First, I should mention I'm with the others. You shouldn't need to pull all the customer IDs from the database. Just run the prepare query to see if the ID is valid. If it is, display the edit form...else display (or redirect them to) the error message. Going that route should fix the issue you are currently having.

 

With that said...the issue you are currently experiencing is likely caused by the following line returning a value that you are not expecting.

$members = $statement->fetchAll();

If you try adding the following after the above line

print '<pre>' . print_r($members, true) . '</pre>';

You'll likely see output like the following:

Array
(
    [0] => Array
        (
            [customer_id] => 1
            [0] => 1
        )

    [1] => Array
        (
            [customer_id] => 2
            [0] => 2
        )
        
    ... 
    
)

Note that the first array index is "0".

Then the following line compares the user's ID against the array indices...not the customer IDs.

$valid = array_key_exists($id, $members);