- Feature Articles
- CodeSOD
- Error'd
- Forums
-
Other Articles
- Random Article
- Other Series
- Alex's Soapbox
- Announcements
- Best of…
- Best of Email
- Best of the Sidebar
- Bring Your Own Code
- Coded Smorgasbord
- Mandatory Fun Day
- Off Topic
- Representative Line
- News Roundup
- Editor's Soapbox
- Software on the Rocks
- Souvenir Potpourri
- Sponsor Post
- Tales from the Interview
- The Daily WTF: Live
- Virtudyne
Admin
There is no cost to using prepared statements for field substitution. Indeed, its use can improve performance when multiple queries are made. Used consistently, it makes your code provably invulnerable to SQL injection attacks. It avoids other potential bugs that come from data changes (spaces, commas, etc in your fields). For very good reasons, it is the current best practice for forming SQL string queries.
Using string concatenation for SQL field substitution is always the wrong way to do it. It promotes sloppiness in your code base, provides dangerous sample code for people coming on later to the project, and it is a hole in your code where you can't unequivocally claim there is no chance for SQL injection attacks. Arguing that it doesn't matter shows a deliberate disregard for good programming practices.
Admin
You do when you are well educated and fresh out of school, or when you work for an organization where efficiency (value for $ spent) or timeliness are of no concern (e.g. the government) or you work on something where it really does matter - e.g. operating systems, medical/military areas etc..
In the most part, no, it makes no sense to spend even 10% of your time covering off all the edge cases for theoretical scenarios which just aren't going to happen.
Admin
Why must the WTF's have been made by juniors? It never mentions their age. Typically they are submitted by someone who inherits a project from a more senior person - someone who was there before and has been in the field longer.
Admin
If I had been drinking my iced tea at the moment I read your solution, my screen would now be iced tea stained. Thanks for the laugh. It's even funnier because it works and is a very clever solution to the question. If we were hiring, I would ask you to work for us...lol
Admin
Some mistakes are more common from Junior devs. Typically a junior is much less aware of available library functions/3rd party tools as they just have not had the same experience as more senior staff, this leads to them sometimes using incorrect tools for the required job.
Admin
In fact "SELECT COUNT(homnum) FROM tblleads WHERE homnum = ? AND reviewed = ?;" is more what he wants.
(Count(*) would be unnecessarily forcing the db to process all fields.)
Admin
Oh, sure, I know. But there is this temptation to blame things on young bucks just because we're older than them. This website was used as proof that they don't know any better. I pointed out that there isn't any proof that the WTF's are caused by juniors unless we assume that they are, using this website as proof to support the assumption that the content of this website is proof...
See the problem?
But, yeah, I broke the build a few times in my day.
Admin
Nobody said it was caused by a junion, but I was using them as an example as they are often utilized to perform maintenance on existing applications.
But I do agree, I have seen bad code from juniors, but far worse/scary/dangerous code from old-timers because coupled with ignorance is a shitty "I've been doing this for years" attitude which closes their eyes and ears to any new ideas.
Admin
There's nothing theoretical about a function getting called outside of the scope for which it was designed. Functions are for code reuse. If your function's implementation is tied to its caller's semantics, you better make sure only the caller can call it. Otherwise, it's fair game.
Admin
Yes, I realized that already. In summary: don't be stupid, use prepared statements (and use the functions that are provided which do what you are trying to accomplish).
Admin
I just hope people don't take this into ridiculous territory:
vs.
And what about checking for overflow when two numbers are added? Do it EVERY TIME? Or only when it seems plausible that overflow could occur?
Admin
Not quite.
http://stackoverflow.com/questions/59294/in-sql-whats-the-difference-between-countcolumn-and-count
Admin
Admin
That depends on your work application environment. I have written many many functions which could be called by other code, but which just plain won't.
I would do standard 'best effort' safety measures to protect other code reusers, but I am not going to add an extra weeks implementation time to a project by coding against every conceivable bad thing which could happen. My employers would rather pay for more functionality and I see a lot of sense in this.
Of course some functions are highly reused and should be written more carefully, and some applications are critical that the whole thing must be written defensively.
Admin
Is this supposed to be a counter-example? It is obviously better to check "pre-conditions" in the function, or else you have to check for them every time you call the function.
Why don't you just stick the check in a function for adding numbers, and then use that? That way, you don't have to think about it at all, and are guaranteed correct behavior.
Admin
Your functions are doing way too much work if it takes you weeks to code defensively.
Some languages even warn you when your functions aren't total. It takes me about 10 seconds to read the warning and 30 seconds to come up with an acceptable solution.
Admin
Admin
Like this:
And then in any other function we want to increment an int, we call this function?
The additional class/library of math functions you would write to cover all these cases would surely show up here as a WTF.
Admin
Of course you're not going to validate at every layer because that wouldn't make any sense (everything that I've read states that it should be when data crosses trust boundaries, but that definition is flexible).
You'll validate when you (the method or function) are directly acting on the data because you should know how the data should look (bounds checking, divide by zero, etc.). It's funny that, although you would be reinventing the wheel, your example works against your argument because that would be the ideal situation to validate the parameters.
A better example:
Are you really suggesting that I should blindly "trust" the JavaScript validation that may have been implemented in the UI, or that I should perhaps take 2 CPU cycles to have a quick look at them myself?
Admin
what are you talking about? I said the project would take an extra week, not that it would take weeks per function, or however else you have misinterpreted it.
It's sad that you spend so much time trying to troll that your largely intelligent answers become lost.
Admin
You know what, if you can't use your judgement to tell you when something is reasonable, then you're either a troll or a terrible developer.
Either way, I'm done trying to explain colors to someone who is blind...
Admin
so a comment for someone else was too long for you to read and you thought we'd care to know. quaint.
Admin
Admin
Of course not, I wasn't even talking to you!
Admin
I don't think I have trouble telling if something is reasonable. The people who say ALWAYS and NEVER are essentially overlooking the possibility if being reasonable.
Admin
Well, it's hard to tell because your rampant straw man usage. But hey, if it makes you feel better to 'win' a discussion by bringing it to it's extreme yet logical conclusion, then go ahead.
Admin
Take your petty little quabbles elsewhere.
Admin
So TRWTF is not using a prepared statement to create the SELECT string. Do I win?
Admin
@Lone Marauder There are two in this example. The first is instead of using the databases count() function he loops through all the results and increments a counter. The second and more major one is the code will allow sql injection by simply adding an apostrophe. The correct way to do this would have been to use a parametrized query.
Admin
Imagine how much better this code would have been if he had had proper development tools.
Admin
Ignoring whether or not it is possible to alter the input, I'd have to question how a single apostrophe would achieve anything more than an sql syntax error.
Admin
Funny, I thought that's what you were doing, back there were you were making assumptions as though they were facts. And that part where (someone else, not even you) made an absolute statement and when I questioned its absoluteness, you call me unreasonable?
What gives, man? You like my other sockpuppets.
Admin
Admin
Ugh, you have no idea what you're talking about.
First, anyone who tells you what the DBMS is going to do based on a snippet of code is an idiot. Second, anyone who calls a DBMS a DB is as clueless as someone who calls WINWORD.EXE a word-processing document.
Count(*) means count all rows, including those that have nulls.
select Count(foo) from X means the same as select count(*) from (select foo from X where foo is not null)
Now for all the people who think they understand SQL: explain why many DBMSs can't just keep a counter on each table so that COUNT(*) is instantaneous.
Admin
This should be:
But instead it is:
Admin
You do realize that you just described the three main characters from the IT Crowd, don't you?
Admin
Admin
Admin
Admin
Admin
SELECT TOP 1 field_to_be_counted FROM count_table ORDER BY field_to_be_counted DESC
...?
This of course assumes that field_to_be_counted is numeric, and if not, you'd have to cast as the right data type...
But why are you asking a question like this? How does this actually benefit your company if they can answer this question?
Admin
The previous comment was of course to the guy saying select max() without using max. Failquote ftw (trwtf?)
Admin
Admin
It's always wrong to build SQL statements by concatenating strings. Simple as that.
It's just wrong, arguing otherwise is not only pointless, it's encouraging sloppy development.
Admin
Admin
SELECT COUNT(*) FROM InterestingComments
(0 row(s) affected)
Admin
To everyone that is discussing wether this code is vulnerable to SQL injection:
Note that the method is [b]PRIVATE[b]! Hence, we know that any of the calls to this method can only be done from other methods in the same class.
Of course, it would be infinitly better to use parameterized queries and NOT concatenate a SQL query, and judging by the code there is a significant risk there are no encoding what so ever, or that the encoding is buggy or broken, but we don't KNOW that.
Someone mentioned reflection can call this method with unsafe parameters. That's true, but if an attacker can run such arbitrary code with sufficient permissions, the system is compromized anyway and there is nothing that prevents an from accessing the DB directly.
(If a developer decides to use reflection to call this method, then THAT is the WTF, not the accidental bypassing of parameter encoding.)
Admin
Seems intuitive to me
Admin
I'll bite, despite not being experienced in the ways of DBS
Admin
With regards to SQL, it is completely acceptable assuming that the input is correctly escaped to make SQL injection impossible. These "NEVER" comments tend to identify coders that have only been exposed to very limited problems and environments.
With regards to non-SQL string concatenation NEVER being acceptable, you are an idiot.