2
\$\begingroup\$

I have a form that posts (a,b) values into the following .php file, and displays an error message when the user tries to reach the .php file directly, instead via POST request.

This code is probably pretty messy, for example, I'm not sure I can post <p> tags right away, instead of having them inside the <html> tags, but since it's an if/else function, I'm not sure how to build this in the most efficient way.

<?php
    if ($_SERVER['REQUEST_METHOD'] !== 'POST') {
?>
    <p>You didn't come here through a Post.</p>
<?php
    } else {

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

    switch ($_POST['a']) {

    case "1":
        $var1 = "word1";
        break;

    case "2":
        $var1 = "word2";
        break;

    default:
        $var1 = "other";
    }
}

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

    switch ($_POST['b']) {

    case "1":
        $var2 = "word3";
        break;

    case "2":
        $var2 = "word4";
        break;

    default:
        $var2 = "other";
    }
}

?>  
<!doctype html>
<html>
<head>
<title>test</title>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
<style>
#various-styles {
}
</style>
</head>
<body>
<div>
HTML-blocks that contain PHP such as <?php echo $var1; ?>, and <?php echo $var2; ?>.
</div>
</body>
</html>
<?php
    }
?>
\$\endgroup\$

3 Answers 3

Reset to default
2
\$\begingroup\$

A few suggestions.

  1. Instead of writing styles in same file use an external style sheet

  2. Also you can restructure it like:

     <!doctype html>
 <html>
 <head>
 <title>test</title>
 <meta http-equiv="Content-Type" content="text/html; charset=utf-8">
 <style>
 #various-styles {
 }
 </style>
 </head>
 <body>
 <div>
 <?php
     if ($_SERVER['REQUEST_METHOD'] !== 'POST')
     {
 ?>
 <p>You didn't come here through a Post.</p>
<?php
     }
     else
     {
         if(isset($_POST['a']))
         {
            $status = "HTML-blocks that contain PHP such as ";
            switch ($_POST['a'])
            {
                case "1":
                  $status .= "word1";
                break;
                case "2":
                  $status .= "word2";
                break;
                default:
                  $status .= "other";
            }
        }

        if(isset($_POST['b']))
        {
            $status .= "and ";
            switch ($_POST['b'])
            {
                case "1":
                  $status .= "word3";
                break;
                case "2":
                  $status .= "word4";
                break;
                default:
                  $status .= "other";
            }
        }
        echo $status;
 ?>
 <!-- other html code else block here -->
 <?php
    }
 ?>

 </div>
 </body>
 </html>
\$\endgroup\$
2
  • \$\begingroup\$ thanks, but i'm afraid you have broke it down too much, the output inside the <DIV> is just an example, what If I'm going to have a whole <TABLE> inside with nested div, and I want a certain cell to echo something? I'm looking for a way for having the code of the HTML block as natural as it can be, so it can be easy to spot and modify as well when necessary. \$\endgroup\$
    – rockyraw
    Commented Apr 1, 2014 at 19:13
  • \$\begingroup\$ @rockyraw: ohh, I see, I'm editing the answer \$\endgroup\$
    – Midhun MP
    Commented Apr 1, 2014 at 19:14
1
\$\begingroup\$

+1 to Midhun MP, the error page also should be valid HTML. Some other notes:

  1. Using indentation in the HTML code would be easier to read/follow:

    <!doctype html>
<html>
    <head>
        <title>test</title>
        <meta http-equiv="Content-Type" content="text/html; charset=utf-8">
        <style>
            #various-styles {
            }
        </style>
    </head>
    <body>
        <div>HTML-blocks that contain PHP such as 
            <?php echo $var1; ?>, and <?php echo $var2; ?>.</div>
    </body>
</html>

  2. You should initialize the variables in every code path. If a malcious client send a POST requiest without an a or b field you'll get some warnings in the error log or on the page (depending on the settings of your server and error_reporting):

    [error] [client 127.0.0.1] PHP Notice:  Undefined variable: a in .../index.php on line 3
[error] [client 127.0.0.1] PHP Notice:  Undefined variable: var1 in .../index.php on line 57
[error] [client 127.0.0.1] PHP Notice:  Undefined variable: var2 in .../index.php on line 57

    I'd set the default value before the isset condition to avoid that:

    $var1 = "other";
if(isset($_POST['a'])){

    switch ($_POST['a']) {

    case "1":
        $var1 = "word1";
        break;

    case "2":
        $var1 = "word2";
        break;
    }
}

  3. Instead of this error message:

        <p>You didn't come here through a Post.</p>

    I'd print something helpful. What should the user do? Should they go back to the main page or to the form? Consider linking it.

    (You might also find useful #3 here.)

\$\endgroup\$
1
\$\begingroup\$

Ideally, when an request is unacceptable because it was made using the wrong HTTP method, the response should have a 405 (Method Not Allowed) status code rather than the usual 200 (Success).

To do so, use header('HTTP/1.0 405 Method Not Allowed'). This has to be called before a single byte of the HTML output has been sent; once the HTTP body starts, it is too late to alter the headers.

\$\endgroup\$
2
  • \$\begingroup\$ Thanks for the suggestion, why is it more idle? Also, in that Instance I've noticed that Chrome and FF won't display any message, rather a blank page. Does it mean I'll have to add to my htaccess a ErrorDocument 405 /mesaage.php If I want to display some message? \$\endgroup\$
    – rockyraw
    Commented Apr 2, 2014 at 11:26
  • \$\begingroup\$ I've just tried including a custom message and noticed that while FF/Chrome would display the custom message, IE would still have the 405, is that Ideal? \$\endgroup\$
    – rockyraw
    Commented Apr 2, 2014 at 12:15

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.