- 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
So it throws an Exception when invalid, returns 1 for valid and 0 for null.
TRWTF is Java, of course.
Admin
If this line is not blank then maybe 5 seconds can make a frist. If this line is not blank then maybe 5 seconds can make a frist. {snip} If this line is not blank then maybe 5 seconds can make a frist.
Admin
/** Valid signs */
You don't need to read any further than that to know you're not dealing with a programmer here.
But when he went home at night I bet he was thinking yeah I'm sooooo cool I wrote some code today yeehaw!
Admin
If he had used Regular Expressions, he would have had 10 problems
Admin
Well at least he implemented a whitelist not a blacklist. Even if I offered a $10,000 bonus, I don't think I could get our lead web "developer" to grasp that concept.
Admin
10 problems are less than 101 problems.
Admin
You might think this could be reduced to a single function, but you would be forgetting the unique messages:
"Vsadr5 with invalid sign"
(And, of course, your users are naturally intuitively going to know what Vsadr5 means... to say nothing of invalid sign.)
WARNING: CAPTCHA not invalid!
Admin
If Cedille, Eszett, and Y acute are all considered valid address characters, either the company expects to ship all over Europe, or the "programmer" is having too much fun with keyboard symbols.
Admin
That's awesome how all the tests have the same logic, just the exception message and the variable names are different.
Admin
I suppose this might have been written with one of the early Java versions that didn't have regexes. But that's not much of an excuse ...
Admin
Ah the classic case of a clueless/lazy programmer who, rather than sit back after the first one and think "Hey wait a minute, this seems really inefficient. What if we need to validate more than one line?" and then goes and refactors out the code properly, does it once, says "It's done Boss" and then copy-pastes that function when they need to do it again.
It's a damn shame these bozos are more common than they should be.
Admin
Even if it was, that's not an excuse for not refactoring this out into a method so the code doesn't have to be repeated. That's the real WTF here, not the way it's implemented but that it's not reusable so it has to be copied.
Admin
That's what you should expect when people get rated on lines of code instead of quality.
He's missed a trick, though. He could have added code for address lines 6 to 10, `"to allow for future enhancement".
Admin
I was more worried about the semicolon, back slash, and the open and close parens. What, curly braces are too good for your addresses?
Admin
The clunking tedium of creating an array of characters: 'A', 'B', .... yecch. My immediate thought (in absence of any knowledge of the existence of regexps) would be:
String validChars = "ABCDE ... "
But apart from repeating the business end 5 times (oh, and failing to handle exceptions and null cases correctly) I've seen plenty worse.
Admin
He should have written it like this:
Admin
My eyes. The goggles aren't helping. How many times do we need to write something like the following?
Or more likely refactor this to generalize for all the other input fields that need validation, providing a standardized regex pattern.
Admin
I feel great joy when I ask how do we know what we want. Suddenly people realize that they never bothered to figure out what they actually want.
Admin
What, like this:
Possibly even compile the pattern statically and pass the pattern in. At which stage there is a school of thought that suggests you might as well factor out the method itself as it contains but one line. I would probably keep the method just in case it needs to get more complicated in future.
Admin
It must also have been written with one of the early versions of Java where you couldn't call the same function in two different places with two different values for the parameter.
Admin
Ha ha. Absolutely agree.
Admin
Well, I find it helpful that they allow the single quote character ('), for SQL injection attacks. And the ampersand character (&) for HTML injection/cross-site scripting attacks. They're always working harder to make my life easier!!!
Admin
Which is perfectly reasonable since you might want to change the logic for one field.
What's awesome is how the author didn't simply put the common functionality in another method. "I'll write a method for each field, but I'd rather copy and paste my code five times than create a sixth."
Admin
If only ASCII was arranged in a logical manner, then you wouldn't even need regular expressions. You could do something like:
for( int i = 0; i < vsadr1.length(); i++ ) { char c = vsadr1.charAt(i) if(!( (c >= 'A' && c <= 'Z') || (c >= 'a' && c <= 'z') || (c >= '0' && c <= '9') || (c == ' ') // || ... )) return false; } return true;
captcha: sagaciter: The SAGACITER loved to reference "Lord of the Rings"
Admin
The other WTF - yet another data modeller/BA who thinks that addresses consist of 5 fields.
No doubt it is near impossible to create a report to the effect of "sales by state" and the excuse will be "no-one has ever asked for that in all the years I have been here."
Admin
The requirement is mostly bogus, too... We've all seen the multi-page regex to (mostly) get email addresses right, and postal addresses are a lot more irregular. There is no way to construct a whitelist, except closely studying the postal guidelines of all the countries you're shipping to, going insane, then settle on a blacklist of the most esoteric Unicode blocks – it's most likely none of ⟺ or ☻ or 💂.
Then again, why bother? Just print it on the label and have the postman figure it out.
Admin
Wouldn't need a regex, but it's still far simpler...because you can do the same basic thing. Why type out "(c >= 'A' && c <= 'Z') ||" when you could just use "[A-Z]"?
Admin
And then you need to ship to Japan. Oops (times five)!
Admin
So that's:
why not just
?Admin
I think y'all are being too hard on this programmer. He did recognize that he needed only one validSign array, after all.
Admin
Admin
I would suggest that one should first trust the user to be able to type in an address that will get it to them. If they can't do that, then you have more serious problems than playing games with what characters to allow.
OK, you should probably not allow non-printable characters.
But I would suggest that instead of making up "requirements," maybe one should consult the appropriate standards. For United States addresses, it would be "Publication #28."
http://pe.usps.gov/cpim/ftp/pubs/pub28/pub28.pdf
Admin
Admin
Admin
5 x the lines 5 x the functions 5 x the productivity 5 x the maintenance
5 x the pay, right?
TRWTF is that he didn't make 5 copies of the array. WIW is wrong with the programmer?
Sigh. How are we supposed to win when the turkeys outnumber the eagles 5:1?
Admin
This is my address without nonprintable characters: 64/2 רמב"ם
This is my address with nonprintable characters: 64/2 רמב"ם
So even those are important.
Admin
I have yet to come across a situation where the validity of an input field can be correctly validated using a list of valid characters. There are however cases, where a list of valid characters can provide a good enough approximation.
If for example an input field contains a value such as a number or an IP address, then a whitelist of permitted characters could make sense. It would still permit lots of invalid inputs, but you'd rule out all inputs that could perform some sort of injection attack.
As soon as your inputs start looking even slightly similar to free text, then forget about validating using a list of valid characters. The only proper approach to such fields is to consider all characters to be valid and apply proper escaping. Most of the possible escaping bugs can be caught by simply testing that you can indeed use all different characters.
Admin
Because there's no need for anyone at Plummer's Landing to reliably get their mail, is there? Programmers don't get to mangle people's data for their convenience.
Admin
I hope you're trolling.
Admin
Admin
Of course TRWTF is that he replaces the valid characters in that string with spaces instead of empty strings. So he needs the 2nd loop to check each character instead of checking if the string is empty... :-)
Admin
Admin
At least packages won't get lost in the æther.
Admin
Who are the experts in delivering to (hopefully) the right address - a programmer who thinks he understands addresses or a Post Office who deals with addresses every day? Of course, there may be some sense in ensuring there's no data injection, but beyond that the validity of the data requires an expert to assess it - so short of the the PO providing explicit rules (alla the pdf link someone posted) on how the address is parsed, the address should be left up to them to deal with....
Admin
There is only one field that will be present on ALL postal addresses - the country. It's often omitted (and thus local is assumed), but it's present in all addresses.
And this makes it a lot easier. If the country is your own, most post offices offer an address validation system - either online or offline subscription which will contain every valid address in the country. You can use it to validate in-country addresses.
FOr out of country addresses, the postal service looks at the country field and routes it there and lets the local post office handle it.
And let's not forget about Mojibake making things even more interesting.
Admin
This!
Admin
Pretty sure they don't in my country....
Admin
Yahoo auctions resemble eBay. When I give my address to sellers, I omit the country because it's assumed that everyone is in Japan, and in case anyone ever doubted because of my foreign name they'd see my address starting with a postal code and 東京都 (Tokyo). Well, one time a seller shipped from China. They didn't insert 日本国 (Japan) which would go at the beginning in Chinese or Japanese order or at the end in common international addressing order. They just copied my address and mailed the package from China. The Chinese post office figured out what country Tokyo is in. (That predated war moves on the Senkaku islands. Who knows if they'd be cooperate today.)
Admin
In the name of all that is holy, stay far away from me. Blacklists are a great way to ensure that your code is insecure. The fact that it may be easier to get right doesn't make it the right option. The submitter's code is terrible, but only because it is redundant, difficult to maintain, and needlessly inefficient. Doing it with a blacklist would make it redundant, difficult to maintain, inefficient AND insecure.
Admin
Actually, not Norway and Denmark as the letters 'Æ' and 'Ø' are missing, e.g. "Færøy", http://www.faeroy.no/.
/Jon