|
Get him fired. Tell him exactly why. If he asks for a reference, tell him you'll tell them the truth about why he was fired. Well, maybe warn him first, I guess? Maybe. Full disclosure: I used to work with a bunch of people who coded like that and it was loving terrible, so my opinion may be somewhat more extreme than it otherwise would be.
|
#
?
Apr 20, 2012 20:25
|
|
|
|
| # ? Apr 28, 2024 04:38 |
|
|
420 SMOKEAWEED posted:I was thinking the same thing but then I noticed the function name has 2 misspellings and inconsistent capitalization, "category" is consistently misspelled "catagory" except in the call to Model_DBTable_UserFilterCategory, and the return value from that is stored in "userilftercat". Like, misspelling filter as ilfter once or twice is easy to understand but misspelling it and consistently sticking with that misspelling to the point of committing it is pretty hosed up. One of my biggest shames was spelling a class (and hence filename) as persistance instead of persistence. So for years everyone had to copy my misspelling. What's worse most of the people didn't realize it was misspelled though it grated on me.
|
|
#
?
Apr 20, 2012 22:03
|
|
|
Hughlander posted:One of my biggest shames was spelling a class (and hence filename) as persistance instead of persistence. So for years everyone had to copy my misspelling. What's worse most of the people didn't realize it was misspelled though it grated on me. Sounds like a especially persistent error 
|
|
#
?
Apr 20, 2012 22:39
|
|
|
I remember having to go through and fix about 50 swapped 'ie's once, many of which were user visible. I have come to appreciate spellcheck as a development tool.
|
|
#
?
Apr 20, 2012 23:03
|
|
|
Hughlander posted:One of my biggest shames was spelling a class (and hence filename) as persistance instead of persistence. So for years everyone had to copy my misspelling. What's worse most of the people didn't realize it was misspelled though it grated on me. That's understandable though. It's hard to miss those kinds of spelling mistakes, especially when the only one who gives a gently caress about it is the compiler. Having an IDE that auto completes everything makes it even harder to notice while you are developing since you don't even have to think about the variable name but rather what it represents.
|
|
#
?
Apr 20, 2012 23:08
|
|
|
In one of our codebases, we have a class called AssynchronousRead. There's also a persistent misspelling of switch as 'switche', in class names, comments, method names, etc.
|
|
#
?
Apr 20, 2012 23:23
|
|
|
http://en.wikipedia.org/wiki/HTTP_referer
|
|
#
?
Apr 20, 2012 23:25
|
|
|
Hello breadcums my old friend. Okay so it's a non-english shop but really? Everywhere?
|
|
#
?
Apr 20, 2012 23:25
|
|
|
KARMA! posted:Hello breadcums my old friend. Maybe someone has a bread fetish? v  v
|
|
#
?
Apr 21, 2012 00:36
|
|
|
Zhentar posted:I remember having to go through and fix about 50 swapped 'ie's once, many of which were user visible. I have come to appreciate spellcheck as a development tool.
|
|
#
?
Apr 21, 2012 04:46
|
|
|
The real reason recv(2) is abbreviated
|
|
#
?
Apr 21, 2012 04:51
|
|
|
Destroyenator posted:a macro to make "recieve" even in comments a compile time error. Were you running on a stupidly dumb C preprocessor? That's not how macros work.
|
|
#
?
Apr 21, 2012 05:14
|
|
|
Suspicious Dish posted:Were you running on a stupidly dumb C preprocessor? That's not how macros work.
|
|
#
?
Apr 21, 2012 05:20
|
|
|
I wouldn't be shocked if there were early implementations of the CPP which processed words in comments. The preprocessor had to be aware of comments for the #define CONCAT(a, b) a/**/b trick to work (which doesn't work in ANSI C), and there were implementations which substituted inside string literals.
|
|
#
?
Apr 21, 2012 07:10
|
|
|
Suspicious Dish posted:Were you running on a stupidly dumb C preprocessor? That's not how macros work.
|
|
#
?
Apr 21, 2012 07:57
|
|
|
Here's a coding horror: https://noplaintext.com/ It's a tool for sending encrypted messages. However, it: - Uses only 47.6 bits of entropy taken from the non-cryptographically secure Math.random() (which according to MDN is seeded from the current time). - Encrypts messages but doesn't bother authenticating them. The server can flip arbitrary bits within the message. - Obviously all "javascript encryption in the browser" is inherently insecure because you have to trust the server sending you the web page. But wait! View source! code:And... code: We also have to trust whoever controls the crypto-js project and the people running googlecode.com.
|
|
#
?
Apr 21, 2012 13:15
|
|
|
shrughes posted:
But it's GOOGLE, they do no evil... right? 
|
|
#
?
Apr 22, 2012 11:43
|
|
|
Chopper posted:But it's GOOGLE, they do no evil... right? I have a feeling Google has a lot easier ways to "steal" your online messages.
|
|
#
?
Apr 23, 2012 03:09
|
|
|
php:<?
if ($apifunc=='dosoemthingelse'){ // function somethingelse
$mysql = "Select allowed from users where username = '" . $user . "' AND password = '". $pass ."'";
$result = mysql_query("$mysql");
$num_rows = mysql_num_rows($result);
if ($num_rows > 0){
$row = mysql_fetch_assoc($result);
$myallowarray = explode(",",$row['allowed']); # note: for some reason, yes, we store CSV in a database.
foreach($myallowarray as $k) {
if ($k = $allowid) {
$goodtogo = 1;
}
}
}
mysql_free_result($result);
if($goodtogo==1){
$mysql = "update data set dataname = '" . $dataname . "' WHERE iddata = " . $iddata . " AND idarea=" . $areaid;
$result = mysql_query("$mysql");
}
mysql_close($con);
}
}?> I don't know why those two lines aren't left-justified like the rest, but there you have it. Also note the subtle renaming from "idarea" to "areaid", too. (Note that this is a separate file from the other one that had an `API` command. This one, of course, reuses none of the other code and isn't accessible without using a completely different URL... The $user/$pass things are also escaped earlier in the file, at least.) Zamujasa fucked around with this message at 14:44 on Apr 23, 2012 |
|
#
?
Apr 23, 2012 14:42
|
|
|
Zamujasa posted:
'dosoemthingelse' seems like curiously good career advice.
|
|
#
?
Apr 23, 2012 15:01
|
|
|
That's mostly just me anonymizing the actual variable name, though the real one isn't much better. I've been looking, but jobs here are few and far between. The fact this place is apparently trying to get me to go overboard and work at 140% (and seems to not understand that adding another programmer does not reduce the time programming takes in any sort of predictable manner) has been reasons to look around. We actually had a whole refactor/rewrite planned, which I wrote a bunch of notes on, and then they decided the next day that nope, we're not going to rewrite it, just pick the top 3 issues off your three-page list and fix those so we can go live with HUGE_CUSTOMER. 
|
|
#
?
Apr 23, 2012 23:56
|
|
|
That doesn't sound like such a horror. Refactors / rewrites don't deliver value in themselves. And if it's an enterprise customer, they'll have their own QA processes where you can continue to work on v1.1 whilst they review v1.0.
|
|
#
?
Apr 24, 2012 00:08
|
|
|
Milotic posted:That doesn't sound like such a horror. Refactors / rewrites don't deliver value in themselves. And if it's an enterprise customer, they'll have their own QA processes where you can continue to work on v1.1 whilst they review v1.0. Normally I would agree. I have seen what rewrites do to things. But this is not an ordinary project, this is a 'cloud-based' solution (so basically we own it and maintain it period, nobody else gets to see inside the guts). I'm talking things like "Oh, this table is randomly InnoDB instead of MyISAM", "No foreign keys here, nope", "Why is a column named SYSTEM_ID storing user_id values" (capitalization included, it's the only one in the whole db that way), "Why are we storing CSV in a database", and "Wow, there's a total lack of cooperation between these different portions". We have 3 different "areas" (arguably four) and none of them share any code, having been written by about 4 different people. The project scope has changed about 3 times (sometimes majorly) so that we're stuffing e-mail addresses into a phone number field (and still calling it phone number) and a bunch of other things that go along the same line. I've run some checks on the database with JOINs and found a bunch of de-referenced, leftover, inaccessible data (that wasn't used anywhere visible but still was making our backend system work harder). There's a lot more really severe issues as well -- a critical component is copied no less than five times, and every time we want to update that component, we have to modify each one... the problem is that at some point they fell out of sync and now it's a total crapshoot as to which component has what features. I know better than anybody that a mass redesign/refactor is almost always a bad idea, but this is the one time this thing feels justified. The system barely works as it is, and the proposed fixes won't really fix any core issues, just sort of put band-aids on everything.
|
|
#
?
Apr 24, 2012 00:19
|
|
|
Zamujasa posted:Normally I would agree. I have seen what rewrites do to things. But this is not an ordinary project, this is a 'cloud-based' solution (so basically we own it and maintain it period, nobody else gets to see inside the guts). I think this is every job in a nutshell.
|
|
#
?
Apr 24, 2012 01:25
|
|
|
revmoo posted:I think this is every job in a nutshell. Game developer at Valve or 3d realms might be the only exceptions.
|
|
#
?
Apr 24, 2012 03:19
|
|
|
This isn't really a horror per se because minifying JS is always going to look really weird, but I admit I never would have thought of using PNG compression (requires WebGL). A writeup of the technique is here.
|
|
#
?
Apr 24, 2012 05:15
|
|
|
MEAT TREAT posted:That's understandable though. It's hard to miss those kinds of spelling mistakes, especially when the only one who gives a gently caress about it is the compiler. Having an IDE that auto completes everything makes it even harder to notice while you are developing since you don't even have to think about the variable name but rather what it represents. A job I used to have was all written in a BASIC derivative language. The code base was a horrible messy mess and the flag to require declaration of variables before use was not set. Also people are horrible at spelling. I can't count the number of times I spent debugging some simple code because I knew how to spell a word but whoever wrote code before me didn't. Also there almost 0 tooling for this language, and almost everything was flying around in global scope so it was slightly less than trivial to verify names (if the variables even had actual names).
|
|
#
?
Apr 24, 2012 16:34
|
|
|
So my company has an "ancient" website (been around since ~2005). Kind of an entrepreneurial first draft business website by the company's founders. It was made by some outside group that was gone before the CTO ever came aboard years ago; he counts himself lucky that he has full access to the hosting account that the server runs through. The site is very simple and "works", plus the company changed its focus years ago and that site only sees a trickle of clients and is all managed by one of the senior sales/account managers. Nobody in IT has had to ever go near it. Cut to this morning, suddenly they need some mostly cosmetic changes to the site and the sales/account manager guy decides to email me about it. I figure it's a 5 minute deal so whatever, I drum up the server credentials and log in. Here's the highlights of what I find about this site: 1) PHP is a mess of logic, presentation, and data all dovetailed together in one giant ball. Okay, and the CTO warned me about that the last time we ever talked about the old site. But that's not the worrying part... 2) All db queries are hand crafted in php and--you guessed it--pull directly from post/get variables, raw and unescaped. That's super unpleasant, but at least the database is isolated to just this trainwreck of a site. Then I notice the even bigger worry... 3) All passwords are stored in plaintext. Oh, and of course we have their email addresses too, along with all other sorts of contact info. The rowcount in users is about 800. At the very least, payment is handled by an outside invoicing system that is secure, so there's no CC info in that db (I went through every single table to make sure). Our main site has to be HIPAA compliant as we deal with SSNs and medical data and so on. This site I'm pretty sure doesn't need to fall under HIPAA but it's still pretty unsettling and now I've just created a bunch more work by discovering how easily that data will be compromised. Now I want the name of the group or person who made that site just so I can follow up and verify they're not making websites for anyone anymore. Bhaal fucked around with this message at 19:23 on Apr 25, 2012 |
|
#
?
Apr 25, 2012 19:21
|
|
|
Bhaal posted:2) All db queries are hand crafted in php and--you guessed it--pull directly from post/get variables, raw and unescaped. That's super unpleasant, but at least the database is isolated to just this trainwreck of a site. Then I notice the even bigger worry... I was curious how popular this was. Let's try the StackOverflow tags for php+mysql. In the past two hours, out of the first 15 results, we have: http://stackoverflow.com/questions/10320720/getting-an-error-with-my-query http://stackoverflow.com/questions/10319977/php-mysql-select-box http://stackoverflow.com/questions/10319398/function-populates-first-listbox-but-not-the-second-from-a-database-table (it uses two variables, but I can't tell if they come from user input or not) http://stackoverflow.com/questions/10319032/select-statement-not-working Four questions that have SQL injections in the last two hours. Yep. EDIT: I also want to point out that the top result for "php mysql tutorial" also has this issue. Suspicious Dish fucked around with this message at 19:30 on Apr 25, 2012 |
|
#
?
Apr 25, 2012 19:27
|
|
|
Suspicious Dish posted:EDIT: I also want to point out that the top result for "php mysql tutorial" also has this issue. Oh, so that's where they all come from. Edit: lmao, that site hasn't been updated since 2003. Hammerite fucked around with this message at 19:53 on Apr 25, 2012 |
|
#
?
Apr 25, 2012 19:51
|
|
|
Suspicious Dish posted:the top result for "php mysql tutorial" also has this issue. This site and page are hilarious. quote:Why Would I Want A Database? Why would I want a database? Why, to have a database of course! I'd love to have one of those sites which get "all there information from a database". The "Websites" bullet point is actually describing a template system, but it isn't too far off. I hate this site for the bad writing more than I dislike the horrible technical advice.
|
|
#
?
Apr 25, 2012 19:55
|
|
|
Suspicious Dish posted:EDIT: I also want to point out that the top result for "php mysql tutorial" also has this issue. So does the w3schools tutorial.
|
|
#
?
Apr 25, 2012 20:03
|
|
|
Bhaal posted:So my company has an "ancient" website (been around since ~2005). Kind of an entrepreneurial first draft business website by the company's founders. It was made by some outside group that was gone before the CTO ever came aboard years ago; he counts himself lucky that he has full access to the hosting account that the server runs through. The site is very simple and "works", plus the company changed its focus years ago and that site only sees a trickle of clients and is all managed by one of the senior sales/account managers. Nobody in IT has had to ever go near it. I don't know what to tell you other than condolences, but it's probably best to not think that most developers write code like that (because they do). At the very least when I first heard about SQL injection I got real nervous and used addslashes at the very least, then eventually the mysql_escape_string function. Note: I always thought it was PHP's terrible decisions that led the current escape string function to be called, "mysql_real_escape_string" but apparently this is MySQL's doing as PHP is just copying the function name implemented in MySQL API 
|
|
#
?
Apr 25, 2012 20:09
|
|
|
DaTroof posted:So does the w3schools tutorial. w3schools is the loving worst. Almost all of their advice/tutorials/poo poo is horrible in that it's either out of date or flat out wrong (or both!). That their name (intentionally) suggests that they actually are connected with the real W3C and that they have refused to publicly state that they aren't connected on their site (despite the W3C requesting them to) is loving terrible as well.
|
|
#
?
Apr 25, 2012 20:10
|
|
|
Strong Sauce posted:I don't know what to tell you other than condolences, but it's probably best to not think that most developers write code like that (because they do). And now the real answer: Parameterized queries.
|
|
#
?
Apr 25, 2012 20:17
|
|
|
Strong Sauce posted:I don't know what to tell you other than condolences, but it's probably best to not think that most developers write code like that (because they do). All of which are the wrong solution, and can be bypassed in novel ways. Use prepared statements. Look Around You posted:w3schools is the loving worst. Almost all of their advice/tutorials/poo poo is horrible in that it's either out of date or flat out wrong (or both!). That their name (intentionally) suggests that they actually are connected with the real W3C and that they have refused to publicly state that they aren't connected on their site (despite the W3C requesting them to) is loving terrible as well. They at least have one thing going for them. They sell certifications that lets potential employers know that they should never hire them!
|
|
#
?
Apr 25, 2012 20:18
|
|
|
Strong Sauce posted:I don't know what to tell you other than condolences, but it's probably best to not think that most developers write code like that (because they do). I don't know if I'll experience pushback on taking the time to minimally fix things like throwing escapes around request data and changing password handling to use MD5. It's really not a priority site as far as business goes, but if I do get pushback on it I'm still deciding on how to emphasize how bad this is. I already made a new account and used a special affiliate id of "%", which matched my new account with a major affiliate and discounts heavily anything I order (plus waiving some other service fees). I'm honestly not too good with DOING sql injection so I'll need to bone up a little on exactly what you can put in to log in as anyone or list ALL order data not just your account's, but just from looking at the code I know it can be done. The table with email/name/etc with plaintext passwords is what bothers me the most, though. All the rest of our servers are in a cage at a very large and very secure colo. The network/firewall hierarchy was setup by a very professional (and expensive) consulting group that focuses almost entirely on that. Every query is either prepared or through zend's parameterized stuff. I can't even connect to anything that isn't client facing unless I'm in the office, and so on. But this site in question is on some consumer grade CPanel controlled hosting service with hardly any security configured or added in besides how it comes out of the box. EDIT: Suspicious Dish posted:EDIT: I also want to point out that the top result for "php mysql tutorial" also has this issue. DaTroof posted:So does the w3schools tutorial.  Okay now I want to donate all my earthly possessions and go live out my days in a nice cave somewhere.Bhaal fucked around with this message at 21:07 on Apr 25, 2012 |
|
#
?
Apr 25, 2012 21:04
|
|
|
Bhaal posted:I don't know if I'll experience pushback on taking the time to minimally fix things like throwing escapes around request data and changing password handling to use MD5. It's really not a priority site as far as business goes, but if I do get pushback on it I'm still deciding on how to emphasize how bad this is. I already made a new account and used a special affiliate id of "%", which matched my new account with a major affiliate and discounts heavily anything I order (plus waiving some other service fees). I'm honestly not too good with DOING sql injection so I'll need to bone up a little on exactly what you can put in to log in as anyone or list ALL order data not just your account's, but just from looking at the code I know it can be done. You might find this worth a read. Also, I just found out that the PHP development team is planning to deprecate the mysql extension. Well I never.
|
|
#
?
Apr 25, 2012 21:32
|
|
|
Bhaal posted:I don't know if I'll experience pushback on taking the time to minimally fix things like throwing escapes around request data and changing password handling to use MD5. md5 is broken
|
|
#
?
Apr 25, 2012 23:40
|
|
|
|
| # ? Apr 28, 2024 04:38 |
|
|
Aleksei Vasiliev posted:no don't Use bcrypt, scrypt, or hell even SHA2 iterated 512 times. MD5 is utter trash.
|
|
#
?
Apr 25, 2012 23:49
|
|