PHP no nos

[Daniel0]

Doing like $foo = "hello $bar[baz] world"; is perfectly valid though.

[waynew]

I never use heredoc. Usually, I do something like this:

 

<?php
//people using mysql_fetch_array when they obviously don't need it
//also bugs the hell out of me
while($row = mysql_fetch_assoc($result)){
   include("html/newsitem.html.php");
}
?>

 

Then in newsitem.html.php

<tr>
   <td>
      <img src="<?php echo $row['img']; ?>" alt="<?php echo $row['colname']; ?>" />
    </td>
   <td>
      <?php echo $row['colname']; ?>
    </td>
</tr>

 

I never add "\n" etc to my HTML. IMO it's not worth it.

[waynew]

Doing like $foo = "hello $bar[baz] world"; is perfectly valid though.

 

I think it makes it look less readable. Maybe it's just me. I like my variables to stand out.

[oni-kun]

You never use newlines? I woke up one day on a massive html dump log, and noticed it was nearly impossible to sift through as it was all on one line! (and it is 600KB+).

 

Newlines are your friend, but by god please don't place them in single quotes, That would make me cry.

[waynew]

You never use newlines? I woke up one day on a massive html dump log, and noticed it was nearly impossible to sift through as it was all on one line! (and it is 600KB+).

 

Newlines are your friend, but by god please don't place them in single quotes, That would make me cry.

 

You could use a HTML tidy program if you really needed to. I just think they look ugly.

[oni-kun]

By practise I always write: echo "$var_one <br/>\n"; for ease of mind, OCD thing!

 

You could use a HTML tidy program if you really needed to. I just think they look ugly.

 

HTML Tidy adds so many newlines though!

[waynew]

By practise I always write: echo "$var_one <br/>\n"; for ease of mind, OCD thing!

 

You could use a HTML tidy program if you really needed to. I just think they look ugly.

 

HTML Tidy adds so many newlines though!

 

You see, I usually use single quotes when generating HTML, so it would probably look like this for me.

<?php
while($row = mysql_fetch_assoc($result)){
echo '<img src="'.$row['img'].'" alt="Image" title="Image" />'."\n";
}
?>

[tibberous]

Using $_GET and $_POST instead of $_REQUEST

Using some kind of big framework they wrote themselves that makes the code unreadable to anyone but them.

 

$databaseConnection = new dbc();
$databaseManager = new databaseManager($databaseConnection, $response, true, 4);
$databaseManager->executeQuery('users', 'insert', array('name'=>'Bob', 'age'=>'5'), 1);
// wtf??

[.josh]

so tibberous, is using $_GET and $_POST instead of $_REQUEST just a personal pet peeve of yours, or do you have a real reason why that's some kind of bad practice? If anything, I would argue the opposite, as $_REQUEST holds both, it's one step closer to the concept of register globals.

 

...and same goes with someone using their own framework they built...I sort of fail to see how that's bad practice.  Do you know the ins and outs of all the existing "big name" frameworks out there?  And btw they did have to start somewhere themselves; at one point in time they were also no-name frameworks. 

[nrg_alpha]

so tibberous, is using $_GET and $_POST instead of $_REQUEST just a personal pet peeve of yours, or do you have a real reason why that's some kind of bad practice? If anything, I would argue the opposite, as $_REQUEST holds both, it's one step closer to the concept of register globals.

 

...and same goes with someone using their own framework they built...I sort of fail to see how that's bad practice.  Do you know the ins and outs of all the existing "big name" frameworks out there?  And btw they did have to start somewhere themselves; at one point in time they were also no-name frameworks. 

 

Actually CV, according to Chirs Shiflett (author of 'Essential PHP Security'), $_REQUEST is not recommended. One of the reasons is that it is susceptible  to CSRF (Cross-Site Request Forgeries) attacks. CSRF is when an attacker sends arbitrary HTTP requests from the victim. The book mentions this is particularly dangerous when say the victim is always logged onto a site that sells goods and uses $_REQUEST in its form. The attacker can profile the form code, see what elements and their expected values are, and observe the overall form's behavior. Once this is figured out, the attacker tests to see if GET data can perform the same behavior as achieved by using the form normally.

 

If so, the doors are blown wide open for the attacker to use this to his/her advantage (getting the victim to visit the set-up URL in question). One of the solutions in mitigating CSRF attacks is to use POST instead.

 

Granted, I'm no security expert (and having not read the book in a while, it would do me good to re-read it as a refresher).

 

EDIT - Here's an article from Chris' site regarding this issue:

http://shiflett.org/articles/cross-site-request-forgeries

[Daniel0]

If you're vulnerable to CSRF you're vulnerable regardless of whether you use $_GET, $_POST, $_REQUEST or filter_input.

[nrg_alpha]

Sorry, I didn't state that solution properly (now upon re-reading it).

 

The book mentions:

 

You can take a few steps to mitigate the risk of CSRF attacks. Minor steps include using POST rather than GET in your HTML forms that perform actions, using $_POST instead of $_REQUEST in your form processing logic, and requiring verification for critical actions (convenience typically increases risk, and it's up to you to determine the appropriate balance).

 

The most important thing you can do is to try to force the user to use your own forms. If the user sends a request that looks as though it is the result of a form submittion, it makes sense to treat it with suspicion if the user not recently requested the form that is supposedly being submitted...

 

So I mistated the solution. So when I mentioned 'one of the solutions', I meant it was part of a series of steps.. but yeah, poorly worded on my part.. my bad. In any case, the link provided in the edit in the previous post should help shed light on the issue.

[.josh]

Actually CV, according to Chirs Shiflett (author of 'Essential PHP Security'), $_REQUEST is not recommended.

 

tibberous said he hates it when people use $_GET or $_POST instead of $_REQUEST (IOW he was saying you should use $_REQUEST).  I was questioning why he felt that way, and argued that if anything, using $_REQUEST is not ideal.

[Daniel0]

Sorry, but I don't see how that would protect you. How is it more difficult doing POST /foo/bar.php HTTP/1.1 rather than GET /foo/bar.php HTTP/1.1 in your request?

 

Using $_POST instead of $_GET or $_REQUEST is at best security theater.

 

[...] it's one step closer to the concept of register globals.

 

There is nothing inherently wrong with register globals. The problem begins when people don't initialize their variables before using them. The only reason why register globals would be bad is because it makes it easier for stupid people to screw up. The only place where register globals would be a security issue is when you otherwise would have gotten an E_NOTICE.

 

That being said, I don't like using register globals either, but that's just a matter of style.

[.josh]

I don't disagree that there is nothing inherently wrong with register globals.  But it was apparently abused to the point that it was changed to being off by default, and soon to be altogether removed.  It's the same argument as guns killing people: guns don't kill people, people kill people.  That's right, but if there's so many people killing each other, it's a problem that needs to be addressed, and a good way to address that is by controlling the guns. 

 

My logic with $_REQUEST being like register globals was not that it in and of itself is bad, but that it's easier to get lazy about programming, and end up opening up abusing it the same way as register globals was abused. 

[gizmola]

I don't disagree that there is nothing inherently wrong with register globals.  But it was apparently abused to the point that it was changed to being off by default, and soon to be altogether removed.  It's the same argument as guns killing people: guns don't kill people, people kill people.  That's right, but if there's so many people killing each other, it's a problem that needs to be addressed, and a good way to address that is by controlling the guns. 

 

My logic with $_REQUEST being like register globals was not that it in and of itself is bad, but that it's easier to get lazy about programming, and end up opening up abusing it the same way as register globals was abused. 

 

I agree with CV here:  if you are accepting a post, it's better to read from $_POST.  $_REQUEST is a big slush bag of input, and just opens up more attack vectors, so why bother to make it easier for someone to mess around with your site.  In fact it can be quite useful to log attempts that are not using your prescribed method, as being people who are probing your site for vulnerabilities.

 

I also think that Register Globals was just a really bad idea from day 1.  Allowing someone to artificially inject their own variables into an interpreted environment, especially where people had a propensity to use exec(), not to mention system() etc.  is a disaster of unmitigated proportions.  It goes hand in hand with globals.

 

Sure, a top notch programmer can code defensively and avoid these pitfalls, but to me that's not the point of PHP.  The point of PHP was to not have to worry about baggage, overhead and syntax, and get down to being productive and achieving your application goals.  Ironically register globals was a feature thrown in exactly because they wanted to make it easier for newbs to be productive, but it's one place where I agree entirely that the feature is far more trouble than its worth.

[roopurt18]

I don't use single or double quotes when I write my HTML; I use a special

class from my framework.

<?php
$html = HTML::Make( 'div' )
    ->child( 'form' )->action( 'foo.php' )->method( 'post' )
        ->child( 'p' )->class( 'errors hidden' )
            ->_If( count( $form_errors ) > 0 )
                ->child( 'ul' )
                    ->Iterate( $form_errors, 'iterate_helper' )
                ->top()
            ->_EndIf()
        ->top()
        ->child( 'p' )
            ->child( 'label' )->for( 'username' )->child( 'Username:' )->top()
            ->child( 'input' )->type( 'text' )->id( 'username' )->name( 'username' )->top()
        ->top()
        // etc...
    ->top();
echo $html->topMost()->__toString();

function iterate_helper( HTML $parent, $value ) {
    $parent->child( 'li' )->child( $value );
}    
?>

 

Using some kind of big framework they wrote themselves that makes the code unreadable to anyone but them.

Would you consider my framework unreadable?

<?php
class UsersController extends Controller {
    public function AddAction() {
        $rq = $this->Request;
        if( $rq->IsPost() ) {
            $user = new User();
            $user->username = $rq->GetParam( 'username' );
            $user->password = $rq->GetParam( 'password' );
            if( $user->Save() ) {
                $this->SetForwardMessage( 'The new user has been saved.' );
            }else{
                $this->SetForwardMessage( 'Unable to create new user at this time.', true ); // true denotes error
            }
            $this->Redirect();
        }
        
        // If we make it this far, the view with the AddUser form automatically
        // is rendered.
    }
}
?>

[Philip]

I have to admit, that HTML class looks like a little overkill (for most projects) =/

[roopurt18]

I have to admit, that HTML class looks like a little overkill (for most projects) =/

 

Until you consider the benefits:

 

1) I don't have to jump in and out of PHP parsing mode.  My files are PHP which improves readability IMO.

2) I don't have to type extra characters to set an attribute, i.e:

<input type="text" name="user" value="<?php echo $somevalue; ?>" />

3) I don't have to worry about escaping output since the HTML class does it for me.  In other words, I don't have htmlentities() littered throughout my code.

4) I don't have to think about a tag being self-closing or requiring a close tag; the class does it for me.

5) All of my markup is well formed, guaranteed.  I can't make mistakes like this:

<input type=text" value="foo" >

6) Nor can I make mistakes like this:

<input type="text" valeu="foo" />

 

I'll admit it was slightly awkward using the class after I first wrote it, but now I love that little guy.

[nrg_alpha]

tibberous said he hates it when people use $_GET or $_POST instead of $_REQUEST (IOW he was saying you should use $_REQUEST).  I was questioning why he felt that way, and argued that if anything, using $_REQUEST is not ideal.

 

Right.. sorry, the way his post was worded, I was interpreting it as suggestive of using $_POST / $_GET as opposed $_REQUEST., but I now see the context.

All for naught..

[Philip]

 

Until you consider the benefits:

 

1) I don't have to jump in and out of PHP parsing mode.  My files are PHP which improves readability IMO.

2) I don't have to type extra characters to set an attribute, i.e:

<input type="text" name="user" value="<?php echo $somevalue; ?>" />

3) I don't have to worry about escaping output since the HTML class does it for me.  In other words, I don't have htmlentities() littered throughout my code.

4) I don't have to think about a tag being self-closing or requiring a close tag; the class does it for me.

5) All of my markup is well formed, guaranteed.  I can't make mistakes like this:

<input type=text" value="foo" >

6) Nor can I make mistakes like this:

<input type="text" valeu="foo" />

 

I'll admit it was slightly awkward using the class after I first wrote it, but now I love that little guy.

 

It might be more readable, but definitely not at first. You make some good points about not dealing with typos & such, but how much more processing power are you using (on a larger site)? (unless you're running with a cache)

[ignace]

That's right, but if there's so many people killing each other, it's a problem that needs to be addressed, and a good way to address that is by controlling the guns.

 

Or kill all the people

[.josh]

@roopurt: Seems like an awful lot of baggage just to prevent some syntax errors...one of the easiest types of bugs to squash.  And what's to say you won't typo the php code itself!  But I remind myself that I am by no means as versed a coder as most people around here, so I'm sure there's some greater benefit to it I'm not seeing...

[roopurt18]

It might be more readable, but definitely not at first. You make some good points about not dealing with typos & such, but how much more processing power are you using (on a larger site)? (unless you're running with a cache)

 

Most of my sites would be considered small and I'm not at all concerned with the extra processing power incurred by my HTML class.  I'm much, much more concerned with the extra development time wasted when my markup is incorrect.  If your markup is incorrect then browsers will demonstrate weird behavior with your JavaScript and CSS.  This can cause you to spend time writing unnecessary JavaScript/CSS.

 

In addition, "Is my markup correct?" is one less question I have to ask myself when weird things do happen.  In my projects where I use that class, I spend little or no time inspecting the actual markup or even looking at the insides of the HTML class.

 

I also develop with the infamous 80/20 rule in mind.  As it turns out, I've never once had a situation where the extra processing done by that class has facilitated that I create markup the "traditional" way.  But if that did happen, well then I might have one page in 100 where I don't use that class.

 

And if my site became so large that I really did become concerned with speed, I'd probably be using a compiled language.  This is of course after obtaining separate servers for database, static content, dynamic content and load-balancing solutions. 

[roopurt18]

@roopurt: Seems like an awful lot of baggage just to prevent some syntax errors...one of the easiest types of bugs to squash.  And what's to say you won't typo the php code itself!  But I remind myself that I am by no means as versed a coder as most people around here, so I'm sure there's some greater benefit to it I'm not seeing...

 

Syntax errors in PHP will result in immediate errors and the script will not run.

 

Using the class incorrectly, for example setting an attribute that doesn't exist on a tag, will result in exceptions.  Again the code will not work until the error is addressed.

 

Having syntax errors in your markup will not stop the page from loading into the browser.  If you're not careful, you may not notice the markup is incorrect.  Or you may have to go out of your way to validate it (which is problematic since we tend to reload pages quite frequently).  And as I said, invalid markup will cause weird behavior in CSS and JavaScript and you will just end up wasting time writing useless code.