HN Explorer

25 comments

[ 2.8 ms ] story [ 61.7 ms ] thread
kodablah
I stopped at #3...using stripslashes is not an acceptable deterrent by any means. I am also unsure how htmlentities has anything to do with SQL injection...maybe you meant XSS prevention? I chose not to read the rest.
agumonkey
Oh wow. So happy not having to read such things anymore.
Kenji
100% agreed, I have the same sentiment. In the year 2015, everyone should know about injections and how to use prepared statements and similar insertion mechanisms that clearly separate query from data. A big no-no to solve anything with the presented function.
nkozyra
In addition to stripslashes being woefully inadequate, the vast majority of these are demonstrations of cURL + some API. The PHP aspect is tertiary.
kolme
You chose wisely. What is this _thing_ doing here?

Many snippets are of questionable quality. Also, any decent programmer should be able to come up with all of them and implement them more cleanly.

Oh, and please people, don't use snippets like those without checking the services first (in the case of the emails, SMS). Just go to the page of the company offering the service, read the documentation AND also read the terms of service.

Jeesh.

midnightmonster
The title and description don't match the code there. As you say, the code sample is actually to prevent XSS, not SQL injection.

stripslashes is used to undo the effects of PHP's old, terrible magic quotes misfeature, b/c the original author (probably not the blog post author) is assuming that $input is coming from the user via GET or POST.

DrinkWater
They are NOT useful, rather harmful. Oh Boy
oneeyedpigeon
This doesn't look like the most professionally put together set of snippets I've ever seen. For example:

  function is_validemail($email)
  {
      $check = 0;

      if(filter_var($email,FILTER_VALIDATE_EMAIL))
      {
          $check = 1;
      }

      return $check;
  }
is a long-winded way of doing:

  function is_validemail($email)
  {
      return filter_var($email, FILTER_VALIDATE_EMAIL) ? 1 : 0;
  }
tuananh
from teh function name, i would expect boolean as return type.
nkozyra
Well, it basically is due to type juggling.
oneeyedpigeon
To be fair, filter_var has a pretty screwy return type of its own but, yeah, if you're going to bother wrapping it, you might as well clean that up:

  return filter_var(...) !== FALSE;
might do it ...
hiou
Rocking compact() niiiice. Probably helpful for your average Wordpress developer, beyond that kind of yikes.
Raed667
20. mysql_connect is depreciated, you should use mysqli_connect
gondo
oh my ... this article and praising comments on the website are the main reason why PHP is considered the language for kids/noobs
oneeyedpigeon
"19. Convert seconds ..." is exactly like something from Daily WTF.
jbrooksuk
I'd re-title this as "46 harmful PHP code snippets".
wnevets
its 2005 all over again!
sarciszewski
3. PHP function to help prevent sql injection

No. No no no no no. A million times no.

Public service announcement: Do NOT use escaping to try to prevent SQL injection.

https://paragonie.com/blog/2015/05/preventing-sql-injection-...

20. php code snippet for database connection

> mysql_connect

I give up.

    (╯°□°）╯︵ ┻━┻
EDIT: I've submitted this blog post to HN separately here if anyone wants to discuss it there (since this one was flagged)

    -> https://news.ycombinator.com/item?id=10017409
joshstrange
I had to check twice to make sure this wasn't an article from years ago.... so many bad practices from is_validemail to still using mysql_connect...
birken
I imagine there is going to be a lot of negativity in the comments, but I applaud you for not only solving a bunch of problems, but posting your code.

However, when sharing code that others might use, there are some basic recommendations that I think would help here. They aren't entirely about correctness, though some of them are.

1) Consistent style (function naming style, indent style, are these functions or not, etc). It is confusing for your end users if some of your functions look_like_this, whereas other ones lookLikeThis. Granted, PHP itself has a bunch of problems with this, but since you can easily control this, it is a nice thing to do. Same goes for the code itself and how it is indented, how brackets are used, etc.

2) No commented out lines of code. If there is an option for a particular feature in a function, it should be a $parameter in the function. Your users shouldn't have to comment and uncomment code to enable things. Comments written in english explaining code are of course fine.

3) All options should be $parameters. For example in #13, the $dir variable which affects the output filename is just hard-coded. This is an important value and should be a $parameter for the function. In #18, the reply-to and return-path should be $parameters. When you are writing functions for yourself, it is perfectly functional to hard-code these things (though not necessarily recommended). But if you are sharing it, you need to write for the most general case, so everybody can easily use parameters for their own specifics.

4) Simplest possible code. For example in #9, this function could just be the return statement and it would do the same thing. When writing for other people, every extra line of code is extra work for people to understand. If it can be written in a simpler way (that is also easy to understand), than it should be.

5) Be aware when using APIs. Quite a few of these examples use external APIs. I'd put those in a separate section, because they have external dependencies (internet connection, access keys, etc) that put them in a different class as compared to pure PHP solutions. A person using your code might not be aware of this and it could create problems for them.