- 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
Wow, this is the best TDWTF for a while.
So much wrongness encapsulated in one line, combined with the definite code smell that if this is what they do to QueryString, what on Earth is going on when they get to the complicated stuff?
Admin
String inspection & mangling is the second thing budding programmers learn; usually right after they "master"
for (int i = 0, i < n, i++)
loops. For too many devs, it's also one of the last things they ever learn about programming.Admin
If you're parsing user-supplied input like query strings, you MUST look through it with a fine-toothed comb from a security point of view. What it the query string doesn't have a
zones=
or]
in it?Assuming this is C#, if
]
isn't found, the innerIndexOf()
will return -1, and then theSubstring()
will throw anArgumentOutOfRangeException
, likely resulting in a 500 error for the user. Not great, not terrible.If a
]
is found butzones=
isn't found, then thanks to the+"zones=".Length+1
bit, it'll keep chugging along, and the outerSubstring()
will return whatever random data starts at index 5 in the query string. Not likely to result in a good result.Admin
And this is why we learn regex :)
Admin
Since they're using C#, they should know that they don't just have access to the raw query string but also a pre-made dictionary of query string values, with full support for parsing multiple values with the same name into a string array. That also means they don't need to use JavaScript to build the string and can take it from a native form submission. But that would make the complicator saaaad...
Admin
I saw those square brackets in the query string and was afraid this was going to be something involving PHP eval.
$zones = eval("return $_GET['zones']");
Admin
Still no.
Whatever language they're using, I have to assume it's at least as capable as PHP in this regard. And in PHP, you can do this:
$zones[] = "pnw"; $zones[] = "apac";
as a way to make an array containing ["pnw", "apac"]. (No, you wouldn't do it like this if you were making the array in PHP from scratch.)
And you can do it as part of the query string:
zones[]=pnw&zones[]=apac (shown without percent encoding).
And you can do it as part of an HTML form:
<input type="checkbox" name="zones[]" value="pnw"> <input type="checkbox" name="zones[]" value="apac">In each case you'll get a proper array on the backend. So there's no excuse for this approach and no need for regular expressions. At all.
Admin
This looks an awful lot like parsing strings in Excel formulas...
Admin
Somehow I suspect this was generated in honor of little Amy Tables (Bobby's kid sister).
Admin
This is a WTF. But a query string is not just a set of key/value pairs, and representing it as a map results in WTFs too.
e.g. what do you do with:
domain.com?a=1&a=2 domain.com?a[]=1&a[]=2 (this variant is submitted by HTML forms with multiple items of the same name iirc) domain.com?a
The language they're using probably has a good query parsing library available natively, but if it doesn't, doing some string parsing yourself is not a WTF. Doing it wrong is, sure.
Admin
I agree, it does look awful. And also like that awful mess.
Admin
Bonus points on this wtf for including the bit:
account_values.IndexOf("zones=") + "zones=".Length + 1) - (account_values.IndexOf("zones=") + "zones=".Length + 1)
I was scratching my head for quite a minute trying to figure out how that doesn't evaluate to 0
Admin
obviously the wtf is that they used square brackets instead of round
Admin
Is it too hard to extract the string starting from [ and ending in ], then use something normal to return an array of values, already handily delimeted by a comma?
Totally standard database stuff, but let's NOT tell the developer of this because we all know where that would lead...