Link to home
Start Free TrialLog in
Avatar of doctorbill
doctorbillFlag for United Kingdom of Great Britain and Northern Ireland

asked on

PHP and if statements

I am passing the following variable to my page:

$ticketstatus = $_POST['status'];

I need to add some code saying that if this is = to "Closed" then add a form field to the body

ie
<?php
if($ticketstatus == "Closed")
{
<form>
some variables and data fields
</form>
}
?>

Problem is I get "page not displayed" when I ad this to the body of the page
Avatar of Chris Stanyon
Chris Stanyon
Flag of United Kingdom of Great Britain and Northern Ireland image

Don't know whether the code you've posted is pseudo code or your actual code, but you seem to be mixing PHP and HTML. You should also check whether your POST variable is actually available otherwise you'll get errors:

<?php
$ticketstatus = isset( $_POST['status'] ) ? $_POST['status'] : "";

if($ticketstatus == "Closed"):
?>

<form>
    some variables and data fields
</form>

<?php endif; ?>

Open in new window

Avatar of doctorbill

ASKER

Perfect - so what was incorrect in my code, out of interest?
ASKER CERTIFIED SOLUTION
Avatar of Chris Stanyon
Chris Stanyon
Flag of United Kingdom of Great Britain and Northern Ireland image

Link to home
membership
This solution is only available to members.
To access this solution, you must be a member of Experts Exchange.
Start Free Trial
Chris has answered this - I am just going to through in a couple of other suggestions that can make your coding cleaner and more natural.

The first is to use the include statement.
This is useful when you have a large amount of output code you want to include to keep this output out of your controller / code logic. Clean code leads to less bugs and easier maintenance. You can then maintain the <form> code entirely independently of the logic that uses it.

For example - using your sample we would do this
<?php
$ticketstatus = isset( $_POST['status'] ) ? $_POST['status'] : "";

if($ticketstatus == "Closed") {
   include('ticket_status_form.php');
}

Open in new window


[ticket_status_form.php]
<form action="someaction.php" method="post">
</form>

Open in new window

The scond suggestion is to use HEREDOC. HEREDOC allows you to format your output more naturally while still being within <PHP> code tags. This removes the requirement for using a lot of <?php ?> tags when you need to include variable output.
These two methods can be combined - expanding on the above example
<?php
$ticketstatus = isset( $_POST['status'] ) ? $_POST['status'] : "";

if($ticketstatus == "Closed") {
   include('ticket_status_form.php');
}

Open in new window


[ticket_status_form.php]
<?php
  // Typically this would be defined in your controller before you include the file
  // I am doing it here to illustrate
  $somevalue = 123; 
  echo <<< FORM
    <form>
        <input type="text" name="defaultOrderValue" value="{$somevalue}">
  </form>
FORM;

Open in new window


Using the above strategies you can neaten up your controller logic and you can make your output code easier to work with when variables have to be added to the output.

NOTE: If the form is static - no variable insertion - then there is no reason to use HEREDOC - only use the latter if you need to add variables to the output.
Mhhh... just some remarks about
$ticketstatus = isset( $_POST['status'] ) ? $_POST['status'] : "";

Hypothesis #1 : There will be no bad guys/ girls / robots working on your pages
Then I usually handle that with a slightly different instruction,
$ticketstatus = TRIM( ' '  . $_POST['status'] ) ;
which will also handle the frequent case of data typed in a form with extra useless spaces (you will never penalize users for extra spaces, right?)
Note that this returns a character string, which later in the program would be converted to numeric if needed

Hypothesis #2 : There might be some bad guys / girls / robots working on your pages
In that case, the data POSTed may contain some javascript code which would attack anyone displaying the data. The minimum that you may do is to at least prevent javascript injection. An easy way to do that is to neutralize HTML type code (eg, any text like <script> )

So my preferred code would be
$ticketstatus = HTMLSPECIALCHAR(TRIM( ' ' .$_POST['status'] )) ;
(but of course beware not to reconvert neutralized special chars into html)

Extra note: this is not enough if you data is used for a SQL search, which might open for sql injection. For that, removing any semi-colon ";" would be the simplest way.
Hypothesis #1 : There will be no bad guys/ girls / robots working on your pages

Rookie mistake :)

The whole point of the isset() line is to prevent an Undefined Index error. Simply calling TRIM on the POST variable won't prevent that error and the script will blow up if the form isn't sent or is somehow manipulated. It's a simple check to make sure the variable is set before trying to use it.

open for sql injection. For that, removing any semi-colon ";" would be the simplest way.

That's exactly the reason why we use Prepared Statements!
In that case, the data POSTed may contain some javascript code which would attack anyone displaying the data

Only if the data is actually echoed back out. In the code above it's not. It's just used in a comparison.
I do not agree with the suggestions above.
This line is necessary and should not be changed. At no time should you ever try to access the POST / GET variables directly
$ticketstatus = isset( $_POST['status'] ) ? $_POST['status'] : "";

Open in new window


Santising is an important step but I would not do it as part of the variable extraction. what if your data coming in is an array
$data = isset( $_POST['rows'] ) ? $_POST['rows'] : "";
Array (
   'row1',
   'row1'
)
If you run htmlspecialchars() on the result your script will crash.

So, bottom line - keep extraction separate and do it in a way that ensures you can recover gracefully if the incoming data does not exist.
1 - Sorry about my "rookie error", I have not written complete php programs for 2 years, just edited typos (mine and others' )

My favorite formula does indeed handle the error case, I pretend quite cleanly:

$ticketstatus = HTMLSPECIALCHAR(TRIM( ' ' . @$_POST['status'] )) ;

see http://php.net/manual/en/language.operators.errorcontrol.php

2 - Not sure what happens in the array case, but not sure either which %age of uses it represents.

Let's say that my formula should be used with an additional comment in this present thread or in the code, eg
$ticketstatus = HTMLSPECIALCHAR(TRIM( ' ' . @$_POST['status'] )) ;
// WARNING: the above line might fail is the POSTed data is an array

Open in new window

// WARNING: the above line might fail is the POSTed data is an array
Unfortunately compilers don't understand this.

1. Safely extract
2. Sanitise and validate

Don't compound functionality - as it leads to mistakes and unexpected results.

After step 1 you KNOW you have a value - if false - you KNOW you did not get a value - these are useful bits of information which can be used to route your code.

Once passed step 1 you can now further process the data (if needed)

Separation of function is a good programming practice.
B-)
As you rightly guessed, the // comment is of course not for the interpreter or compilier, just a reminder to humans in charge of possible later changes
B-)
Thanks all