- 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
That's just overkill. We try 3 times and then give up.
Admin
I think I think it was deleted, but I'm not sure how I feel about that.
/Hofstadter surrenders. Well, giggles.
Admin
Between the break and the continue, it doesn't look as if the delete code is ever actually called... Is ( !$qryObj->Exec( $sSQL ) ) || ( $qryObj->NumRows() == 0 ) ever false?
Admin
I'd love to see some typical values for $iTimes :)
edit:
my bad...too tiredAdmin
I would imagine so, if the query was successful and returned more than zero rows.
Admin
Since the Exec($sSQL) function just runs the code, it returns 1 for success and 0 for failure. Therefore, if it is successful, it passes that if, and then he checks that rows were returned.
Admin
The real WTF is that Robert didn't use a for-loop.
Admin
“I know that you believe you understand what you think I said, but I'm not sure you realize that what you heard is not what I meant.” –Richard Nixon
Admin
How did the code go from being...
[ED: ... snip ...]
WTF is going on WTF?
[ED: The submission was kept mostly intact with the following changes: simplified for ease of understanding, superflous logging code removed, variables renamed, and comments added. The simplification led to an infinte loop, which the original did not contain. I should also note that the comment at the top ("this is stupid") was not added by me. See New Editorial Guidelines]
Admin
Admin
How about just incrementing in the while loop instead of in the first line of the body? There must be some bug there too! Robert is a star!
Admin
All I know is that I really dig the use of Hungarian Notation; without it I doubt I would have known what the variables were. The comments were also particularly helpful.
Admin
In the original posting on the homepage, iTimes is incremented at the end of the while loop. Hence iTimes will never increase upon continual failure.
In the posting above iTimes is incremented at the beginning of the loop. A little less messed up than before...
Could go on about the uselessness of iTimes and bDeleted but I'm sure Robert has already found the bug in Linux PostGristle or PHP...
"No sufficiently complex system is both complete and consistent at the same time." - Douglas R Hofstaedter 'Godel Escher Bach"
Admin
First on Robert.
I have worked with people like Robert and even under the best conditions they can be annoying. When bugs are found they are the first people pointing the fingers at everyone else. Even when the bug is found in their code - it must be the documentation, you should have handled it, etc. They can be very poisonous to a department.
On the WTF
This is the code the festers until someone comes along with the internal cachet to challange Robert on what the hell did he think he was doing. Until the Robert will continue to crap out such nuggets all over the place.
Admin
Looks like a SQL injection waiting to happen.
Admin
I like how he keeps concatenating empty strings onto the end of the SQL queries.
Admin
If you want to give "Robert" credit, perhaps he saw a situation during testing where there was a user session for this user just seconds after this code had executed. But of course that could easily happen if the user revisited the site, since it would create a new session record. Or perhaps the session timeout was set so low that the delete often couldn't complete, thus the need for more retries.
I have to say that "select * from sessions where..." is the most wasteful way to check for record existence though.
Admin
I have worked with Roberts from the test side. (Fortunately they usually weren't *the* senior dev, just a senior dev, so someone else was reviewing their code.) Generally this meant I stood over him until he realizes that yes, the code is indeed breaking, and that yes, he at least has to take ownership long enough to prove that the bug is in the compiler / other person's code.
Oh, and we were the OS team. ;)
Admin
Those foreign key bugs are NASTY!
CAPTCHA: 5000 times is just one short of "AWESOMENESS"!
Admin
How would you do it?
SELECT COUNT(s_user_id) from sessions WHERE s_user_id = input_id;
?
Admin
I love the "E_USER_ERROR " part. How can an inability to delete a record on the database be a "user error"?
During development, Robert didn't know that the other developers and DBA's were playing a very cruel trick on him. They would watch Robert's cubicle, and when they saw him testing the code that would end a session, they would stop the database server. Then Robert would get all angry and wave his arms about and get all flustered. Then they'd restart the server. Robert would just see that his code to end the session would work sometimes, and at other times, it wouldn't work for a while, and then start working again. Robert checked and rechecked his code and saw that there was nothing wrong with it - it was a darn random bug in the darn network/apache/php/postgres/linux stack. As you can see from the code, Robert finally gave up and just decided to repeat the whole code a huge number of times.
And hey, it worked - not because of his code, but because the other developers and DBA's were getting bored with watching Robert, and some of their deadlines where looming.
Admin
Why, yes, Robert, your code certainly is.
I can see how retrying it the first 4999 times is acceptable, but the 5000th check just crosses the line.
Admin
Admin
The sad part of this story is that Robert has obsessive compulsive disorder. Later that night he snuck into the rack and degaussed the database hard drives.
Admin
I can understand retrying a db operation a few times, but that's because I have the glorious job of working with an Advantage database. Just look at the table wrong and BOOM "7008: Unable to ope specified table or memo file". Stupid table lock errors...
Captcha: poprocks. Mmmmm...
Admin
Wow, if this were true Robert would have had to do something very evil to the other Devs/DBAs.
Admin
looks more like 5000 SQL injections waiting to happen. :)
Admin
My first impression is that the while loop will at most only run twice. In the first iteration it will delete the data, in the second it will figure out that there is nothing to be deleted and set $bDeleted to true then the while loop will then exit.
Right?
Admin
[snip posted code]
if (isDeletedLessThanOneHundredTimes(userId)) { ... }
if (isDeletedLessThanOneThousandTimes(userId)) { ... }
if (isReallyReallyDeleted(userId)) { ... }
Admin
The "user" in E_USER_ERROR is the programmer, not the end user.
E_USER_ERROR just indicates that the error being raised is an application-defined one, not a built-in PHP error.
Admin
I dunno, I see iTimes and think Mac is making a newsletter? ;)
Edit: Grr, that was in response to the hungarian notation joke.
Admin
Doing the same thing over and over again and expecting different results is the definition of insanity. I can understand a few retries, but that's ridiculous. Of course, that leads to the philosophical question: "Given that 3 retries is acceptable and 5000 is not acceptable, what number between 3 and 4999 is the maximum acceptable number of retries?"
It probably just took a moment to execute the command. It's more or less the same WTF from last week with the 60-second delay loop to wait for the thread to complete.
All he's doing is putting in a delay to let the command finish without knowing that's what he's doing.
Admin
While I am afraid I'm not infallible like our dear Robert here, I would think to at least, you know, check to see what error was coming out of postgre? If he's trying something 5000 (!) times over, I'd guess it's a concurrency issue of some sort.
Also, I'm pretty sure PHP already has session management, and it works pretty well. Why implement your own, in a table, no less? Oh, well, I guess that's my fallibility getting in the way again.
Admin
You may laugh, but I've actually done something very similar to this!
(manually flashing error codes on the LED on the front of an early IBM RS6000 whenever the guy looked at it).
Admin
I have a crazy question here ... did Sir Robert the Great ever consider that data modifications may require a COMMIT?
Admin
The BBC would like to apologise for the constant repetitions in the show.
Admin
So, what is it called when the different results finally do transpire?
Admin
There are actually quite a few good reasons to use a custom session handler -- security, performance and scalability. Because PHP's default session handler uses temporary files that can be read/written by the web server, this means that on a shared host other users could read your session data. Using a database-driven session handler can greatly increase perfomance and scalability -- you can use in-memory tables, you can have load-balanced web servers that access the same session data, you can more easily manage sessions (e.g. kill a particular session), etc.
Admin
I don't know much about PHP but I had to write something like this in Java, because I needed to access sessions not belonging to the current user's browser. However, since I'm not infallible, I assomed a single DELETE would do the trick.
Admin
The real WTF is that a malicious user's sql injection code will be executed twice.
CAPTCHA: hacker - But Robert is just a hack.
Admin
So, boys and girls, what's the lesson that Robert teaches us in His wisdom?
If you can't make it work the first time, hammer the database server.
Admin
SELECT TOP 1 s_user_id) from sessions WHERE s_user_id = input_id;
That's more efficient in most datbase engines, anyway (so long as there is no ORDER BY clause).
CAPTCHA: shizzle (for rizzle my nizzle!)
Admin
And all this time I was searching for the *fake* WTF.
Admin
Not to mention if the application is farmed across multiple servers and session needs to be stored in a central location...
Admin
My thoughts exactly.
My best guess is that Robert is a little bit of an arrogant pr#@k.
Admin
Dear Kristopher,
There are two ways to program;
Admin
SELECT s_user_id FROM sessions WHERE s_user_id = input_id LIMIT 1
Admin
I would do
mysql_query('SELECT 1 FROM table WHERE x="' . mysql_escape_string($var) . '" LIMIT 1');
if (mysql_num_rows() == 0) {
// Do something
}
Admin
Looks like your post got chewed up a bit... Also, since we are talking Postgres instead of Oracle we should use
SELECT 1 FROM sessions WHERE s_user_id = input_id LIMIT 1;
Admin
OR just SELECT 1 FROM SESSIONS WHERE s_user_id = input_id
we don't use any of the data, just if the row exists or not.