Checking if form is submitted

[lurtnowski]

Im trying to use this code to let me know if my form was submitted

<?php
if(isset($_GET['id']))  
{ 

//do a database lookup

   if ((($count==1) && ($_SESSION['id'] == $_GET['id']))  || isset($_POST['submit']))
  {
	  if (isset($_POST['submit']))
	  {
		  //display thank you message
          }
  //display html form
  }
}

Once the page is loaded (with the GET variable added), the form is displayed.
But whenn I submit the form I get nothing.
So I output the POST and get

userID 1
Name Luke Urtnowskid
Email lurtnowski@gmail.com
Phone 6199426938
About I like turtles!
avatar-2
submit

Since that exists, shouldn’t I be seeing both the thank you message and the form?

Thanks

[SamA74]

The correct way to check if a form has been submitted (by post) is:-

if($_SERVER['REQUEST_METHOD'] == 'POST'){ ... }

But maybe you need another pair of brackets around that condition, if you are going to use that.

if ((($count==1) && ($_SESSION['id'] == $_GET['id']))  || (isset($_POST['submit']))) { ... }

[MarkAtRamp51]

It depends on the FORM Action being posted. Have you checked what the url is that is being posted to, when submit is clicked (use your browser dev tools)?

If I understand correctly, initially the browser makes a get request with an ID on the querystring.
The page loads and displays a form. Does the action in that form html element have a value? If it does, does that value also contain a querystring of id={value}? If it does not, the form data is posted to the server without the ID parameter on the querystring, so your initial if condition fails.

However, if the value of the action attribute of the form element is blank, the form should be posting to the same url in the browser address bar which should still contain the querystring including ID, which should work based on what I know of your problem.

Hope this helps.

[SamA74]

@MarkAtRamp51

Does the action in that form html element have a value?

That’s probably it, this test seems to work.

<?php
session_start();

$_SESSION['id'] = 3 ; // set the session

if(isset($_GET['id']))  
{ 

		//do a database lookup
		$count = 1; // pretend we got this from the db

   if ((($count==1) && ($_SESSION['id'] == $_GET['id']))  || isset($_POST['submit']))
  {
	  if (isset($_POST['submit']))
	  {
		  //display thank you message
		  echo "<h1>Thanks!</h1><p>You said: ".htmlspecialchars($_POST['text'])."</p>" ;
         }
  //display html form
  echo "<form method='post' action='wibble?id=3'>
          <input type='text' name='text'><br>
           <input type='submit' name='submit' value='submit'>
       </form>";
  }
}
else{
	echo "No ID!"; // How you know that's the problem
}

[spaceshiptrooper]

@SamA74 gave you the correct way of checking for form submission. if(isset($_POST[...])) and derivatives of this are usually made from amateur PHP users who have absolutely no idea what they are doing and may have just copy&paste it from somewhere like w3schools which is also a well infamous website that uses legacy and no broken code.

Moreover, it seems that what is actually wrong is the logic you are writing your submission checker in. The $_GET id in the above snippet has to be equaled to the session id. This means that if the user’s session id does not equal the $_GET id, then the submission checker won’t do as you desire. For proper form submission checking, just use what @SamA74 has given in the first snippet sample.

[SamA74]

@spaceshiptrooper

@SamA74 gave you the correct way of checking for form submission

@SamA74

this test seems to work.

Yep, it works and the missing variable is probably why the OP’s didn’t, but it’s not right. There is room for improvement.

[SamA74]

@spaceshiptrooper

Moreover, it seems that what is actually wrong is the logic

Yes, I don’t understand the reasoning of the logic here.
First of all, you require an id value to see anything at all. OK so far…
Then if viewing the form for the first time, the database query must return a record and the id must match the session.
But if you have submitted the form, you just need an id variable, any old value will do, that’s fine, so long as there is one, id=250 or id=fish is acceptable if there is a submit value in $_POST.

[lurtnowski]

ok, made a few changes but am having difficulty still
I think im just having trouble with my logic…
Once I login, setting the SESSION (SESSION[‘id’]=1) and go to the URL http://svr.teamluke.net/edit_profile.php?id=1

so this is my info (after a database lookup)
If I make a change and submit the form, I get,

Heres my code

<?php
session_start();
include("db/configPDO.php");
?>
<table>
<?php 


    foreach ($_POST as $key => $value) {
        echo "<tr>";
        echo "<td>";
        echo $key;
        echo "</td>";
        echo "<td>";
        echo $value;
        echo "</td>";
        echo "</tr>";
    }


?>
</table>
<?php
if(isset($_GET['id']))  
{ 

$Id = $_GET['id']; 

    //if the database query returns a result, $count = 1

?>	
//begin HTML
<?php
   if ((($count==1) && ($_SESSION['id'] == $_GET['id'])) || ($_SERVER['REQUEST_METHOD'] == 'POST'))
  {
	  if ($_SERVER['REQUEST_METHOD'] == 'POST')
	  {
		  
		   //the form has been submitted, so make the change to the database & show message confirming change
	  }
?> 
      //form
<?php 
			
	} else {
	
		echo	"<h2 class='text-danger text-center'><span class='glyphicon glyphicon-remove' aria-hidden='true'></span>&nbsp;&nbsp;Profile doesn't exist!</h2>";
		echo	"</div>";
	}
?>
//end HTML
<?php
}
?>

The thing that is confusing to me is right when the page is loaded, im logged in and the database query works isn’t only the first part if the if statement successful (((($count==1) && ($_SESSION[‘id’] == $_GET[‘id’]))) so only the form is shown.
then once the form is submitted, the 2nd part of the if statement is true (($_SERVER[‘REQUEST_METHOD’] == ‘POST’)))
but since the or (||) is in it the form should be shown, but before that, wouldn’t the message saying the database has been changed appear?
and I have the forms action itself and its method is POST (as seen when I returned the POST)

[Pepster64]

This answer explains why very well. The $_SERVER method is more robust, but not precise. The $_SERVER method is good if you do not care what was posted to the server. The $_POST method s great if there are multiple submissions and it matters what was picked. As long as you sanitize then either method is good in my opinion. For example:

$submit = filter_input(INPUT_POST, 'action', FILTER_SANITIZE_FULL_SPECIAL_CHARS);

if (isset($submit) && $submit === 'login') {
    $username = filter_input(INPUT_POST, 'username', FILTER_SANITIZE_FULL_SPECIAL_CHARS);
    $password = filter_input(INPUT_POST, 'password', FILTER_SANITIZE_FULL_SPECIAL_CHARS);

    $result = $users->read($username, $password);
    if ($result) {
        after_successful_login();
    }
}

I take it one step further by doing this in my forms →

<input type="hidden" name="action" value="login">

instead of the relying on the submit button. However, if you want to be 100 percent sure (well nothing on the net is 100 percent) then just stick to $_SERVER. I’m personally not doing any banking transactions or something that requires NSA type security, so I just use $_POST for my websites. I do my due diligence with other security measures that I have learned from reputable online resources (Lynda.com and Treehouse.com).

[spaceshiptrooper]

@Pepster64

This answer explains why very well. The $SERVER method is more robust, but not precise. The $SERVER method is good if you do not care what was posted to the server.

The only reason one should check against $_POST is during validation, not for form checking. The reason why it should be done for validation and not form checking is because user’s can modify HTML elements and therefore, it would be wise to check if those fields exist. Otherwise, you’ll get an undefined index error while trying to validate those specific fields using regex or any regular validations. The form itself should be done via $_SERVER['REQUEST_METHOD'].

@Pepster64

The $_POST method s great if there are multiple submissions and it matters what was picked.

The question for this is, “Why do you need to have multiple form submissions within a single file?” It would be wise to separate each action based on what the form is for. Would stuffing everything from a user login to a signup page be appropriate when you only need the login part? I honestly don’t think so. It makes no sense to have <form action="login.php" method="POST"> for both login and signup and then check for those 2 different form submissions based on a value. This is redundant, causes work havoc, and promotes bad practice. It would be a smart thing to separate the forms into different files based on what they are for and it would lessen the hassle on you or future developers who are going to maintain your code.

[spaceshiptrooper]

Ok, now I think I know what you are trying to do. What you should be doing is creating just a single page for a user to edit their details. I don’t think using the $_GET parameter would be appropriate since you are only editing your own details. I think you are making this much harder than it really is. All you really need to do is make sure that the user is logged in. If they are not, either redirect them to the home page or a login page. If the user is logged in, display an HTML form where they can edit their information from. While you have the HTML form on the page, populate the fields using the logged in user’s details. This means that you don’t really need the $_GET parameter. All you really need is the user’s ID from a session cookie. Once you have the user’s ID, you then grab their details from the database based on their ID and populate the fields appropriately. Again, no $_GET parameter is needed.

[SamA74]

Just a few concerns to mention.
Starting with your user table.

foreach ($_POST as $key => $value) {

We have not yet established if the post data has been sent, should there be a condition?
Or perhaps if not, should the table be populated with the user’s data from the database?
There is no validation or sanitisation of this data, so both the keys and values could be absolutely anything!
At the very least escape the strings: echo htmlspecialchars($key); and htmlspecialchars($value);
For a more robust approach, don’t use the post array, as it could contain anything, but use a pre-set array of valid form fields that you are expecting to get.

$validFields = array('name', 'email', 'phone', 'aboutme');

foreach($validFields as $field) {
        echo "<tr>";
        echo "<td>";
        echo $field;
        echo "</td>";
        echo "<td>";
        echo htmlspecialchars($_POST[$field]);
        echo "</td>";
        echo "</tr>";
}

So you only get what you expect to get.
This could be made more robust with an if isset befpre the post output.

@lurtnowski

$Id = $_GET[‘id’];

//if the database query returns a result, $count = 1

Please tell me you are using prepared statements to put the id in the query.
You can ensure the value is nothing but an integer (assuming is should be) with a simple preg replace.

$Id = preg_replace('#[^0-9]#u', '', $_GET['id']) ; // strip out what is not numeric

That is if you still think you need the id form get.

[lurtnowski]

Yes, im using the PDO thing for security.
I have the php function,

function test_input($data) {
  $data = trim($data);
  $data = stripslashes($data);
  $data = htmlspecialchars($data);
  return $data;
}

and heres where I use it

	$sql =  "SELECT name,email,phone,about,avatar,password FROM users WHERE id = :id AND display = 1";

	$STM = $conn->prepare($sql);
	
	$STM->bindParam(":id", test_input($Id));
	
	$STM->execute();
	$count = $STM->rowCount();
	
	$row  = $STM -> fetch();

I also use it to update the database

			 $sql = "UPDATE users SET name=:name,email=:email,phone=:phone,about=:about,avatar=:avatar WHERE id = ".$_POST['userID']; 

			 $statement = $conn->prepare($sql);
			 $statement->bindValue(":name", test_input($_POST['name']));
			 $statement->bindValue(":email", test_input($_POST['email']));
		     $statement->bindValue(":phone", test_input($phone));
 			 $statement->bindValue(":about", test_input( $_POST['about']));
 			 $statement->bindValue(":avatar", test_input($_POST['avatar']));
			 
			 $statement->execute();

is that ok?