PHP - $_GET variable not set

Question

Initially, I was going to have 3 separate files for creating, update and delete but to cut down the number of files I decided to make it all in 1 file. The delete action was working when the href was going to a page that was only handling the delete process. The issue is my $_GET doesn't seem to be working in the processor when it shares a file with create and update. Currently, if you click on Delete it creates a null record in the table so it's going to the create function.

viewCountries.php

$CountryID = $row['COUNTRYID'];
    <tr>
      <td><?php echo $row['COUNTRYNAME']; ?></td>
      <td><?php echo $row['GDP']; ?></td>
      <td><a href="editCountry.php?CountryID=<?php echo $CountryID; ?>" class="btn btn-warning">Edit</a></td>
      <td><a href="processor/countryProcessor.php?CountryID=<?php echo $CountryID; ?>" class="btn btn-danger">Delete</a></td>

countryProcessor.php

<?php

require('../scripts/x_connect.php');

if(isset($_POST)) {

    // Create Country
    if(!isset($_POST['CountryID'])) {

        if(isset($_POST['CountryName'])) {
            $CountryName = $_POST['CountryName'];
        }
        if(isset($_POST['Gross'])){
            $Gross = $_POST['Gross'];
        }


        $stmt = oci_parse($conn, "INSERT INTO COUNTRY (COUNTRYNAME, GDP) VALUES (:CountryName, :GDP)");

        oci_bind_by_name($stmt, ":CountryName", $CountryName);
        oci_bind_by_name($stmt, ":GDP", $Gross);

        oci_execute($stmt);
        $Affected = oci_num_rows($stmt);
        oci_commit($conn);

        oci_free_statement($stmt);
        oci_close($conn);


        // echo $Gross;
        // echo $CountryName;
        if(count($Affected) > 0){
            header("Location: ../viewCountries.php?Success=$CountryName has been created!");
        } else {
            header("Location: ../viewCountries.php?Danger=$CountryName hasn't been created!");
        }   

    // Update Country   
    } else {

        if(isset($_POST['CountryID'])) {
            $CountryID = $_POST['CountryID'];
        }
        if(isset($_POST['CountryName'])) {
            $CountryName = $_POST['CountryName'];
        }
        if(isset($_POST['Gross'])){
            $Gross = $_POST['Gross'];
        }


        $stmt = oci_parse($conn, "UPDATE COUNTRY SET COUNTRYNAME = :CountryName, GDP = :GDP WHERE COUNTRYID = :CountryID");

        oci_bind_by_name($stmt, ":CountryID", $CountryID);
        oci_bind_by_name($stmt, ":CountryName", $CountryName);
        oci_bind_by_name($stmt, ":GDP", $Gross);

        oci_execute($stmt);
        $Affected = oci_num_rows($stmt);
        oci_commit($conn);

        oci_free_statement($stmt);
        oci_close($conn);

        // echo "CountryID" . ' ' . $CountryID . "<br>";
        // echo "GDP" . ' ' . $Gross . "<br>";
        // echo "Country Name" . ' ' . $CountryName . "<br>";
        // echo "Rows Affected" . ' ' . $Affected;

        if(count($Affected) > 0){
            header("Location: ../viewCountries.php?Success=$CountryName has been updated!");
        } else {
            header("Location: ../viewCountries.php?Danger=$CountryName hasn't been updated!");
        }

    }

} else {

    // Delete Country
    if(isset($_GET['CountryID'])) {

        $CountryID = $_GET['CountryID'];

        $stmt = oci_parse($conn, "DELETE FROM COUNTRY WHERE COUNTRYID = :CountryID");

        ocibindbyname($stmt, ":CountryID", $CountryID);

        oci_execute($stmt);
        $Affected = oci_num_rows($stmt);
        oci_commit($conn);

        oci_free_statement($stmt);
        oci_close($conn);


        if(count($Affected) > 0){
            header("Location: ../viewCountries.php?Success=Country has been deleted!");
        }   else {
            header("Location: ../viewCountries.php?Danger=Country hasn't been deleted!");
        }
    }

}
?>

`$_POST` is a superglobal array and will always exist. Your check for `isset($_POST)` will always return true. Maybe try `if($_POST)` — Dharman, Mar 03 '19 at 22:45

ArtisticPhoenix · Answer 1 · 2019-03-03T23:37:36.010

Instead of:

if(isset($_POST)) {

It's better to do this:

if ('POST' === $_SERVER['REQUEST_METHOD']) {
    #its prefered to put constants, values, or function calls on the left (but its less intuitive to do)
    #this is because you can do this if($_SERVER['REQUEST_METHOD'] = 'POST') 
    #which will assign `POST` to `$_SERVER['REQUEST_METHOD']` and return true on every request
    #without an error and it can be very hard to debug, but the other way around will throw an error.
    #so if we do our conditions this way, we can avoid that completely

This tells you if the request to the server was an actual post or not. Where the Super Global $_POST is always set, it's just empty.

By the way $_SERVER is another super global like $_COOKIE,$_POST or $_GET. But it contains information about the server and the request headers etc...

There are some considerations when using information from $_SERVER you should treat most of it as you treat $_POST or $_GET in that you cannot trust it to be safe as some of it can be edited by the client or comes from the client.

But that is a whole other topic for another day..

Good Luck!

score 1 · Answer 2 · answered Mar 03 '19 at 22:50

1

The problem is that the POST super global is always set, it's just empty. If you var_dump post you'll see this. Either check that the post variable is not empty, or check that a specific value is set, e.g. isset(post['submit'])

answered Mar 03 '19 at 22:50

imposterSyndrome

896
1
7
18

score 0 · Answer 3 · answered Mar 03 '19 at 22:50

isset() checks if a variable exists and is not null. In case of $_POST or $_GET, they are superglobals and will always exist in every PHP script. Even if the HTTP method was not POST the superglobal will exists and will contain empty array. This means that isset($_POST) will always return true.

There is another way you can compare if $_POST contains any values and it is this:

if($_POST){
    // your code
}

You could also check what was the request method used with $_SERVER['REQUEST_METHOD']

score 0 · Answer 4 · answered Mar 03 '19 at 23:05

Check for posted parameter name such isset($_POST['CountryID']) not just isset($_POST).

So the following should work.

<?php

require('../scripts/x_connect.php');

if(isset($_POST['CountryID'])) {

// Create Country
if(!isset($_POST['CountryID'])) {

    if(isset($_POST['CountryName'])) {
        $CountryName = $_POST['CountryName'];
    }
    if(isset($_POST['Gross'])){
        $Gross = $_POST['Gross'];
    }


    $stmt = oci_parse($conn, "INSERT INTO COUNTRY (COUNTRYNAME, GDP) VALUES (:CountryName, :GDP)");

    oci_bind_by_name($stmt, ":CountryName", $CountryName);
    oci_bind_by_name($stmt, ":GDP", $Gross);

    oci_execute($stmt);
    $Affected = oci_num_rows($stmt);
    oci_commit($conn);

    oci_free_statement($stmt);
    oci_close($conn);


    // echo $Gross;
    // echo $CountryName;
    if(count($Affected) > 0){
        header("Location: ../viewCountries.php?Success=$CountryName has been created!");
    } else {
        header("Location: ../viewCountries.php?Danger=$CountryName hasn't been created!");
    }   

// Update Country   
} else {

    if(isset($_POST['CountryID'])) {
        $CountryID = $_POST['CountryID'];
    }
    if(isset($_POST['CountryName'])) {
        $CountryName = $_POST['CountryName'];
    }
    if(isset($_POST['Gross'])){
        $Gross = $_POST['Gross'];
    }


    $stmt = oci_parse($conn, "UPDATE COUNTRY SET COUNTRYNAME = :CountryName, GDP = :GDP WHERE COUNTRYID = :CountryID");

    oci_bind_by_name($stmt, ":CountryID", $CountryID);
    oci_bind_by_name($stmt, ":CountryName", $CountryName);
    oci_bind_by_name($stmt, ":GDP", $Gross);

    oci_execute($stmt);
    $Affected = oci_num_rows($stmt);
    oci_commit($conn);

    oci_free_statement($stmt);
    oci_close($conn);

    // echo "CountryID" . ' ' . $CountryID . "<br>";
    // echo "GDP" . ' ' . $Gross . "<br>";
    // echo "Country Name" . ' ' . $CountryName . "<br>";
    // echo "Rows Affected" . ' ' . $Affected;

    if(count($Affected) > 0){
        header("Location: ../viewCountries.php?Success=$CountryName has been updated!");
    } else {
        header("Location: ../viewCountries.php?Danger=$CountryName hasn't been updated!");
    }

}

} else {

// Delete Country
if(isset($_GET['CountryID'])) {

    $CountryID = $_GET['CountryID'];

    $stmt = oci_parse($conn, "DELETE FROM COUNTRY WHERE COUNTRYID = :CountryID");

    ocibindbyname($stmt, ":CountryID", $CountryID);

    oci_execute($stmt);
    $Affected = oci_num_rows($stmt);
    oci_commit($conn);

    oci_free_statement($stmt);
    oci_close($conn);


    if(count($Affected) > 0){
        header("Location: ../viewCountries.php?Success=Country has been deleted!");
    }   else {
        header("Location: ../viewCountries.php?Danger=Country hasn't been deleted!");
    }
}

}
?>

`if(isset($_POST['CountryID'])) { if(!isset($_POST['CountryID'])) ` does not seem logical. — Pinke Helga, Mar 03 '19 at 23:43

score 0 · Answer 5 · answered Mar 03 '19 at 23:08

As others have already mentioned it, if (isset($_POST)) will always return true.

IMHO, I believe that you should consider doing things the proper way while you are at it. Consider reading about CRUD at https://www.codecademy.com/articles/what-is-crud.

You will learn there that a GET call is not usually what you should use for deleting stuff. Deleting should use the DELETE verb/method. Adding should use the POST method and updating should use the PUT method, etc.

Additionally, with your current setup, code can be deleted from your database when a search engine Bot crawls your URLs especially given the fact that there is no confirmation needed before deleting a row from the DB.

At least, make sure that your CountryID is not a sequential ID and that it is some sort of UUID hash that is much more difficult to guess.

Thanks for the information @asiby, would adding a js confirm be sufficient? — Raj R96, Mar 03 '19 at 23:46
It would help a bit, but it would not be sufficient because users would just disabled JS and bypass. You could make sure that the delete request is coming from a user who is authorize to delete a country (at least, the user must be logged in). — asiby, Mar 04 '19 at 00:04

score 0 · Accepted Answer · edited Nov 19 '19 at 21:45

I've decided to make the processor look less confusing by adding a hidden input tag in the view to differentiate between create and update that way. So viewCountries.php stays the same.

createCountry.php

<input type="hidden" name="Action" value="Create">

editCountry.php

<input type="hidden" name="Action" value="Update">

countryProcessor.php

// Create Country
if($_POST['Action'] == "Create") {

    if(isset($_POST['CountryName'])) {
        $CountryName = $_POST['CountryName'];
    }
    if(isset($_POST['Gross'])){
        $Gross = $_POST['Gross'];
    }


    $stmt = oci_parse($conn, "INSERT INTO COUNTRY (COUNTRYNAME, GDP) VALUES (:CountryName, :GDP)");

    oci_bind_by_name($stmt, ":CountryName", $CountryName);
    oci_bind_by_name($stmt, ":GDP", $Gross);

    oci_execute($stmt);
    $Affected = oci_num_rows($stmt);
    oci_commit($conn);

    oci_free_statement($stmt);
    oci_close($conn);


    // echo $Gross;
    // echo $CountryName;
    if(count($Affected) > 0){
        header("Location: ../viewCountries.php?Success=$CountryName has been created!");
    } else {
        header("Location: ../viewCountries.php?Danger=$CountryName hasn't been created!");
    }   

// Update Country   
} elseif($_POST['Action'] == "Update") {

    if(isset($_POST['CountryID'])) {
        $CountryID = $_POST['CountryID'];
    }
    if(isset($_POST['CountryName'])) {
        $CountryName = $_POST['CountryName'];
    }
    if(isset($_POST['Gross'])){
        $Gross = $_POST['Gross'];
    }


    $stmt = oci_parse($conn, "UPDATE COUNTRY SET COUNTRYNAME = :CountryName, GDP = :GDP WHERE COUNTRYID = :CountryID");

    oci_bind_by_name($stmt, ":CountryID", $CountryID);
    oci_bind_by_name($stmt, ":CountryName", $CountryName);
    oci_bind_by_name($stmt, ":GDP", $Gross);

    oci_execute($stmt);
    $Affected = oci_num_rows($stmt);
    oci_commit($conn);

    oci_free_statement($stmt);
    oci_close($conn);

    // echo "CountryID" . ' ' . $CountryID . "<br>";
    // echo "GDP" . ' ' . $Gross . "<br>";
    // echo "Country Name" . ' ' . $CountryName . "<br>";
    // echo "Rows Affected" . ' ' . $Affected;

    if(count($Affected) > 0){
        header("Location: ../viewCountries.php?Success=$CountryName has been updated!");
    } else {
        header("Location: ../viewCountries.php?Danger=$CountryName hasn't been updated!");
    }

} else {

// Delete Country
if(isset($_GET['CountryID'])) {

    $CountryID = $_GET['CountryID'];

    $stmt = oci_parse($conn, "DELETE FROM COUNTRY WHERE COUNTRYID = :CountryID");

    ocibindbyname($stmt, ":CountryID", $CountryID);

    oci_execute($stmt);
    $Affected = oci_num_rows($stmt);
    oci_commit($conn);

    oci_free_statement($stmt);
    oci_close($conn);


    if(count($Affected) > 0){
        header("Location: ../viewCountries.php?Success=Country has been deleted!");
    }   else {
        header("Location: ../viewCountries.php?Danger=Country hasn't been deleted!");
    }
}

}

Why make it hidden? Do you have 2 separate buttons? Use them to distinguish the difference. — Dharman, Mar 04 '19 at 00:32
It's just to differentiate between the actions in the processor, there isn't any need for the user to see it .So when data is sent to the processor the first thing it does is to check which action is needed; Create, Edit/Update or Else(Delete). — Raj R96, Mar 04 '19 at 01:18

score -1 · Answer 7 · answered Mar 03 '19 at 22:54

-1

As a few others have mentioned $_POST is always set, so your script is never going into your else{} clause.

Try this; change your first instance of:

if(isset($_POST)) to if(sizeof($_POST))

answered Mar 03 '19 at 22:54

AgileFox

174
6

PHP - $_GET variable not set

7 Answers7