CodeSOD: Querieous Strings

When processing HTTP requests, you frequently need to check the parameters which were sent along with that request. Those parameters are generally passed as stringly-typed key/value pairs. None of this is news to anyone.

What is news, however, is how Brodey‘s co-worker indexed the key/value pairs.

For i As Integer = 0 To (Request.Params().Count - 1)
    If (parameters.GetKey(i).ToString() <> "Lang") Then
        If (parameters.GetKey(i).Equals("ID")) OrElse (parameters.GetKey(i).Equals("new")) OrElse _
             (parameters.GetKey(i).Equals("open")) OrElse (parameters.GetKey(i).Equals("FID")) _
         OrElse (parameters.GetKey(i).Equals("enabled")) OrElse (parameters.GetKey(i).Equals("my")) OrElse _
         (parameters.GetKey(i).Equals("msgType")) OrElse (parameters.GetKey(i).Equals("Type")) _
         OrElse (parameters.GetKey(i).Equals("EID")) OrElse (parameters.GetKey(i).Equals("Title")) OrElse _
         (parameters.GetKey(i).Equals("ERROR")) Then
            URLParams &= "&" & parameters.GetKey(i).ToString()
            URLParams &= "=" & parameters(i).ToString()
        End If
    End If
Next

The goal of this code is to take a certain set of keys and construct a URLParams string which represents those key/values as an HTTP query string. The first thing to get out of the way: .NET has a QueryString type that handles the construction of the query string for you (including escaping), so that you don’t need to do any string concatenation.

But the real WTF is everything surrounding that. We opt to iterate across every key- not just the ones we care about- and use the GetKey(i) function to check each individual key in an extensive chain of OrElse statements.

The obvious and simpler approach would have been to iterate across an array of the keys I care about- ID, new, FID, enabled, my, msgType, Type, EID, Title, ERROR– and simply check if they were in the Request.

I suppose the only silver lining here is that they thought to use the OrElse operator- which is a short-circuiting “or” operation, like you’d expect in just about any other language, instead of Or, which doesn’t short circuit (pulling double duty as both a bitwise Or and a logical Or, because Visual Basic wants to contribute some WTFs).

Source: Read More