The Daily WTF: Curious Perversions in Information Technology

PiisAWheeL · 2012-03-19 Reply Admin

that guy:
I wouldn't call this GOOD code; it could certainly be improved. But it appears to work and mostly do what the author intended. Sure, it could be slightly more efficient (both in implementation and execution) but really... not a WTF.

So the code is less than Reputable?

2012-03-19 Reply Admin

mag:
confirmString.equalsIgnoreCase("")

I like best when he makes sure to check for upper and lower case empty strings

I'm not a Java programmer, so I may need to ask Nagesh about this, but according to http://www.javaperformancetuning.com/, "String.equalsIgnoreCase is a lot faster than String.equals when the majority of the compared data have different lengths because equalsIgnoreCase first checks the length of the two strings." Maybe he's just in the habit of using that method and I assume since it breaks out when the length differs, there's not much of a speed impact.

2012-03-19 Reply Admin

    public void addUser()
    {
        String user = userfld.getEditableText();
        String email = emailfld.getEditableText();
        String pass = passfld.getEditableText();
        String confirm = confirmfld.getEditableText();
        String secretQuestion = questions[selected];
        String secretAnswer = secretafld.getEditableText();

        if(user == null || user.isEmpty())
        {
            app.displayError(SignUpScreen.this, app.getString(R.DIALOG_TITLE_OOPS), "Username field must be filled in.");
        }
        else if(email == null || email.isEmpty())
        {
            app.displayError(SignUpScreen.this, app.getString(R.DIALOG_TITLE_OOPS), "Email field must be filled in.");
        }
        else if(pass == null || pass.isEmpty())
        {
            app.displayError(SignUpScreen.this, app.getString(R.DIALOG_TITLE_OOPS), "Password field must be filled in.");
        }
        else if(confirm == null || confirm.isEmpty())
        {
            app.displayError(SignUpScreen.this, app.getString(R.DIALOG_TITLE_OOPS), "Confirm Password field must be filled in.");
        }
        else if(secretAnswer == null || secretAnswer.isEmpty())
        {
            app.displayError(SignUpScreen.this, app.getString(R.DIALOG_TITLE_OOPS), "Secret Answer field must be filled in.");
        }
        else if(!pass.equals(confirm))
        {
            app.displayError(SignUpScreen.this, app.getString(R.DIALOG_TITLE_OOPS), "Password does not match Confirm Password");
        }
        else if(!user.matches("[a-zA-z0-9_]*"))
        {
            app.displayError(SignUpScreen.this, app.getString(R.DIALOG_TITLE_OOPS), "Username must only contain alphanumeric characters and _");
        }
        else if(!pass.matches("[a-zA-Z0-9]*") || pass.length() < 6)
        {
            app.displayError(SignUpScreen.this, app.getString(R.DIALOG_TITLE_OOPS), "Password must only contain alphanumeric characters, and be at least 6 characters.");
        }
        else if(!confirm.matches("[a-zA-Z0-9]*"))
        {
            app.displayError(SignUpScreen.this, app.getString(R.DIALOG_TITLE_OOPS), "Confirm Password must only contain alphanumeric characters");
        }
        else if(email.indexOf("@") == -1 || email.charAt(0) == '@' || email.charAt(email.length()-1) == '@')
        {
            app.displayError(SignUpScreen.this, app.getString(R.DIALOG_TITLE_OOPS), "This email does not seem to be valid");
        }
        else
        {
            submitUser(user, email, secretQuestion, secretAnswer);
        }

    }

    public void submitUser(String user, String email, String secretQuestion, String secretAnswer)
    {
        String processRequest = String.format(ServerCommands.ADD_USER, URLEncoder.encode(user),
                URLEncoder.encode(email), URLEncoder.encode(secretQuestion),
                URLEncoder.encode(secretAnswer), StaticUtils.md5(pass));

        String result;
        try
        {

            InputStream is = Networking.getStreamFromURL(processRequest);
            result = StaticUtils.getResult(is);
        }
        catch(IOException ioe)
        {
            logger.warn("Unable to submit user.", ioe);
            app.displayError(SignUpScreen.this, app.getString(R.DIALOG_TITLE_OOPS), "The user could not be created.");
            return;
        }

        if(result.equals(ServerCommands.EMAIL_ALREADY_REGISTERED))
        {
            app.displayError(SignUpScreen.this, app.getString(R.DIALOG_TITLE_OOPS), "This email is already registered, please choose another email");
        }
        else if(result.equals(ServerCommands.USER_ALREADY_REGISTERED))
        {
            app.displayError(SignUpScreen.this, app.getString(R.DIALOG_TITLE_OOPS), "This username is already registered, please choose another username");
        }
        else if(!result.equals(ServerCommands.OK))
        {
            app.displayError(SignUpScreen.this, app.getString(R.DIALOG_TITLE_OOPS), "An unknown error occured.");
        }
        else
        {
            StaticUtils.toaster(app, "This username has now been registered, you can login using this username now");
            finish();
        }
    }

    // in StaticUtils
    public static String getResult(InputStream is) throws IOException
    {
        StringBuffer buffer = new StringBuffer();
        Reader in = new BufferedReader(new InputStreamReader(is));
        int ch;
        while((ch = in.read()) > -1)
            buffer.append((char)ch);
        in.close();
        return buffer.toString();
    }

2012-03-19 Reply Admin

TRWTF is that this method has enough stupid-looking little details to obscure the fact that the network I/O is happening on the main thread. I'm sure the user experience on a slow connection is wonderful.

Also, client-side hashing of the password. What are the odds that StaticUtils.md5() generates a random salt, applies it while hashing the password a few hundred times, and returns the concatenation of the salt and the final hash? What are the odds that it runs MD5 once without a salt and returns that? Yeah...

2012-03-19 Reply Admin

I can't believe no one found the real error in here: userString.matches("[a-zA-z0-9_]*")

They're allowing "[]^`" in the username.

Psyckers · 2012-03-19 Reply Admin

Its so mind bendingly bad yet logical it is almost a beautiful WTF.

Coyne · 2012-03-20 Reply Admin

I've discovered serious deficiencies in my code, due to the fact I wasn't even aware that empty string ("") was mixed case.

Severity One · 2012-03-20 Reply Admin

Ben Jammin:
mag:
confirmString.equalsIgnoreCase("")
I like best when he makes sure to check for upper and lower case empty strings
I'm not a Java programmer, so I may need to ask Nagesh about this, but according to http://www.javaperformancetuning.com/, "String.equalsIgnoreCase is a lot faster than String.equals when the majority of the compared data have different lengths because equalsIgnoreCase first checks the length of the two strings." Maybe he's just in the habit of using that method and I assume since it breaks out when the length differs, there's not much of a speed impact.

It could be; looking at the source code of String.java, equals() just checks whether the operand is the same object (with the '==' operator), if not check if both are strings, and if so, checks each character of both strings.

equalsIgnoreCase() does indeed check for the length first, and if your strings start with the same characters, there could be a difference in speed.

However, one has to realise that equals() and equalsIgnoreCase() are two very different beasts. equals() takes an Object as a parameter, so you can invoke the equals() method on a String and pass a JAXBContext as a parameter (or indeed, the other way around). equalsIgnoreCase always takes a string as a parameter.

But more importantly, comparing against an empty string is silly, because you can get the length() of the string and compare it with 0, or if you're using Java 6 or later, you can invoke the isEmpty() method on a string.

2012-03-20 Reply Admin

DTrain:
I have to agree. This is not particularly pretty code, but it's hardly "Oh God, what were they thinking?!" code.

I think it's more like, what weren't they thinking. If it was nicer it would check everything, and generate a list of errors.

Could be a lot worse, broken, enterprisie, or broken and enterprisie. The writer could have used this as an excuse to add a couple of new classes to the code base, but didn't. Instead he wrote some unimaginative procedural code to solve an unimaginative procedural problem.

2012-03-20 Reply Admin

DTrain:
I have to agree. This is not particularly pretty code, but it's hardly "Oh God, what were they thinking?!" code.

It's still an extremely disgusting piece of crap.

If you do indeed think it's just "not particularly pretty", you prolly have a lot of WTF of your own writing to submit.

2012-03-20 Reply Admin

Mason Wheeler:
airdrik:
So TRWTF is a language which requires you to write 8 lines of boiler-plate just to read the entirety of an input stream (be it network, file, process IO, etc)
 InputStream is = Networking.getStreamFromURL(processRequest);
 StringBuffer buffer = new StringBuffer();
Reader in = new BufferedReader(new InputStreamReader(is));
int ch;
while ((ch = in.read()) > -1) {
buffer.append((char)ch);
}
in.close();
String result = buffer.toString();
Just give me the string and be done with it: Reader reader = new ...; String result = reader.readAllString(); (I'm sorry, that's too high-level for our high-level IO library :P ).
Wow! Coming from a Delphi background, where "LoadFromStream" methods are idiomatic and can be found on pretty much any object that could conceivably make use of them, that's incredibly ugly.

Delphi.

2012-03-20 Reply Admin

that guy:
I wouldn't call this GOOD code; it could certainly be improved. But it appears to work and mostly do what the author intended. Sure, it could be slightly more efficient (both in implementation and execution) but really... not a WTF.

I will not tolerate anymore of this ultra-tolerance.

savar · 2012-03-20 Reply Admin

Tractor:
Funny how the regex matches too if only one of the character is in the allowed range.

I'm not sure how that API works, but * matches zero or more characters, so I would have guessed that would match any string. (Although the match itself might be an empty string.)

2012-03-20 Reply Admin

Is this an example of poor code?
Yes.
Is this an example of a WTF? Not really.

It looks like a cut and past script job done by someone who either does not fully understand what they are doing, or can't be bothered to try. But you can still understand what's going on. He's taking values from a form, and trying to perform some validation against them. Aside from stupid things like calling the ToSting() method on the text input (Hint...it's already a string why make it an extra stringy string), and the tolower() on numerics etc... there is nothing monumentally stupid or worth of a jaw dropping HUH?

If he had done something like created his own string class because he thought that the existing string class would not handle all the values users could input for strings, or sent all the input to an XML file, and then had a service validate it and send it back etc.. then you would be looking at WTF quality code.

This is just an example of poor coding style. If the article submitter thinks this is stinky messy WTF code, then he's led a very sheltered life and needs get out some more.

Steve The Cynic · 2012-03-20 Reply Admin

Some Damn Yank:
ldam:
I always wondered about how to type those uppercase digits...
With the <SHIFT> key, silly! Uppercase '8' is '*', etc.

Actually, no. Uppercase '_' is '8', etc.

On my keyboard, the top-row keys unshifted are: ²&é"'(-è_çà)= and shifted: ~1234567890°+

And if I turn on the up-arrow padlock, they swap...

Other people don't have your computer.

DaveK · 2012-03-20 Reply Admin

minitech:
secretafld.getEditableText().toString()
Is it unnecessary, or is it Java?

... or do you have no clue what the return type of the getEditableText method is?

Hint: it's not String.

2012-03-20 Reply Admin

savar:
I'm not sure how that API works

Wovon man nicht sprechen kann, darüber muß man schweigen.

2012-03-20 Reply Admin

They used a Visual Studio Add-In to submit a Java article?

2012-03-20 Reply Admin

I'm not a Java programmer...

But you did stay at Holiday Inn Express last night?

2012-03-20 Reply Admin

airdrik:
So TRWTF is a language which requires you to write 8 lines of boiler-plate just to read the entirety of an input stream (be it network, file, process IO, etc)
 InputStream is = Networking.getStreamFromURL(processRequest);
 StringBuffer buffer = new StringBuffer();
Reader in = new BufferedReader(new InputStreamReader(is));
int ch;
while ((ch = in.read()) > -1) {
buffer.append((char)ch);
}
in.close();
String result = buffer.toString();
Just give me the string and be done with it: Reader reader = new ...; String result = reader.readAllString(); (I'm sorry, that's too high-level for our high-level IO library :P ). You'd think that at least you could hook up the reader to the writer and say go and it would take care of the direct IO under the covers (looks like java.nio does this, but it's still more convoluted than it needs to be)

Um, if you're going to use a BufferedReader, you might as well call readLine() to get the whole line instead of going character by character.

TRWTF is people who diss a language or library out of ignorance.

2012-03-20 Reply Admin

TRWTF is why a java snippet was sent via the "Visual Studio Add-in"

2012-03-20 Reply Admin

I believe that the CAPTCHA for each new commenter should be a one-line replacement for the horrific code-of-the day.

I admit, this would create a whole new subsytem of DailyWTF that could be written in oh-so-many ways. Perhaps each submission could be voted upon for correctness, and then validated by Amazon Mechanical Turk workers for semantic correctness (as there are some things a computer just cannot do).

The shortest, or most elegant entry would win a bebe gun and a number of bebes equal to the number of characters (ASCII only, please) in her/her answer.

An additional prize would be awarded if the winner were able to locate and wound the perpetrator, hopefully in the area of his/her fingers. It would be pointless to aim at the head, as it has already been demonstrated to be useless.

2012-03-21 Reply Admin

airdrik:
Just give me the string and be done with it: Reader reader = new ...; String result = reader.readAllString(); [...]

If I run this a million years, will it eventually read Shakespeare?

vic · 2012-03-21 Reply Admin

It makes sense to test the password & confirm contents before their equality. If not the user experience would be "confirm does not match password, please correct". Then after the user corrects it would tell it that the caracters are invalid. That would be stupid. The other way around saves the user some operations. Same goes for length. Ideally of course all problems should be displayed at once.

TRWTF is submitting Java code with the Visual Studio add-in.

2012-03-21 Reply Admin

"else if (!passString.matches("[a-zA-Z0-9]*") || passString.length() < 6)"

If you're already using a regex (clearly a inefficient for a simple check like this, but if you are), you may as well do !passString.matches("[a-zA-Z0-9]{6,}") and avoid the second check.

Medinoc · 2012-03-22 Reply Admin

Can't put base64 data in my password? THIS SUCKS!

2012-03-22 Reply Admin

Wow! Through all of this people will allow email addresses to be entered without any type of domain suffix... so I can register as DWTF@DWTF and it's ok.

Kivi · 2012-03-23 Reply Admin

Fx:
Wow! Through all of this people will allow email addresses to be entered without any type of domain suffix... so I can register as DWTF@DWTF and it's ok.

It is actually okay if your email account is at a TLD. A friend of mine once had n@ai, which was cool because it was his name backward.

2012-03-24 Reply Admin

RRR:
This is a _very_ LAME submission. There are prioritized error codes returned for each test. How else could it be done if not like that? Please don't touch the code in that function! You clearly have no clue what and why it does what it does. And, please, uninstall your Submit-to-WTF Visual Studio Add-In! It encourages trigger happy WTFers to submit crappy TDWTF entries...

I tend to agree some what. I've made better validation but not by much. I tend to adhere to DRY and make very good use of things such as arrays, switches and OO. Also, I wouldn't regex check the pass string. It's not terrible as far as I can see. A few tweaks and it would be acceptable. I've seen much worse.

2012-03-24 Reply Admin

Deffy:
"else if (!passString.matches("[a-zA-Z0-9]*") || passString.length() < 6)"
If you're already using a regex (clearly a inefficient for a simple check like this, but if you are), you may as well do !passString.matches("[a-zA-Z0-9]{6,}") and avoid the second check.

Since it's check minimum length (I would just swap comparisons otherwise) I wouldn't bother to optimise like that. If the input data is rarely less than six characters it might actually end up slower, not that you would notice though.

You have a lot to learn about optimisation. All you can really be sure of by doing what you suggested is that the code will be a little bit smaller.

2012-03-26 Reply Admin

vic:
It makes sense to test the password & confirm contents before their equality. If not the user experience would be "confirm does not match password, please correct". Then after the user corrects it would tell it that the caracters are invalid. That would be stupid. The other way around saves the user some operations. Same goes for length. Ideally of course all problems should be displayed at once.
TRWTF is submitting Java code with the Visual Studio add-in.

+1, -1

It makes sense to check the contents of the password before comparing it against the confirm string, for the reason you say, and if the confirm string was different there's a fair chance that the user will end up correcting it as part of correcting the password.

But the confirm shouldn't ever be checked for matching the password rules, it should only ever be checked for matching the password.

2012-03-29 Reply Admin

Why on earth can I only use alphanumeric characters in my password?

2012-04-02 Reply Admin

Hmm....maybe I COMPLETELY missed the point of the WTF, but I don't get it. User is not logging in. User is submitting an initial set of credentials, and the app wants to produce an error if there password is not of the correct format (regex) or doesn't match (string compare). Regardless of the order, based on that requirement, BOTH commands need to be checked. Does it really matter which order? Seriously? If someone F'd up the entry, they have to redo anyways...

2012-04-18 Reply Admin

If you think that that's filthy code then consider yourself lucky to have a job in this area.

Less-than Reputable

Leave a comment on “Less-than Reputable”