CodeSOD: Regular Query String

Robert H is taking the first steps required to make existing code supportable: writing unit tests. The code in question isn't that old, it was just developed by someone who didn't care about mundane tasks, like testing.

They also didn't care about things like paying attention to web standards, and thus were using the same copy/pasted utility functions they'd been using for a decade.

The application in question was a web application with a large amount of client side code. The navigation system would construct a URL with a query string, then programatically alter the window.location.href property, triggering a page load.

Now, since circa 2016, the "correct" way to manipulate search parameters in every browser that wasn't Internet Explorer is through the URLSearchParams object, which gives you handy-dandy get and set methods, along with some iterators.

Some organizations might have been (or might still be- the horror!) supporting IE. But that doesn't really apply to Robert's case. But even if one were supporting a legacy browser, this pile of string mangling probably isn't the right way to do it:

replaceQueryParam(param: string, newval: string, search: string): string {
	const regex = new RegExp("([?;&])" + param + "[^&;]*[;&]?");
	const query = search.replace(regex, "$1").replace(/&$/, '');        
	return (query.length > 2 ? query + "&" : "?") + (newval ? param + "=" + newval : '');
}

This searches through the search string (containing our query params) using regexes. Now, personally, I think regular expressions are overkill for this exercise- split would do the job here. But let's trace through the regex.

We're searching for one of (?, ;, &), followed by our parameter name, followed by zero or more of anything but a& or ;, optionally followed by a ; or &.

The inclusion of ; is a fascinating choice, because it helps us date this code to having been written prior to 2014- once upon a time, ; was a valid URL parameter separator. Few web servers supported it, it never caught on, and in 2014 it was officially made an illegal token to use as a separator.

Now, with this regex in hand, we do a find/replace on our string- replacing the entire expression with just the ?, ;, or & that we captured, and then chopping off the trailing ampersand if there is one.

With the old parameter out of the way, we can now append the new value to the end of the string. We use a ternary to check the length of the string, so we know whether or not to use a ? or an & to start it. Then we append the new version, if newval has a value (otherwise we're actually removing the parameter).

Now, having cut my teeth on web development back in the bad old days where this kind of code was common, I'd still never have written this. The better solution is to maintain a dictionary of all the properties you want to put in the query string, and then have a function that serializes/deserializes that data to a string. No regexes, no guessing on whether or not you need to prepend a question mark, just an extremely simple pair of functions.

This is a wonderful example of someone who found a utility function circa 2009, and added it to their toolbelt and never thought about whether it was a good tool (it isn't) or if it ever should be replaced (it should). That said, at least it should be easy to write unit tests for, though I'm sure it will break in interesting ways on malformed query strings.

[Advertisement] BuildMaster allows you to create a self-service release management platform that allows different teams to manage their applications. Explore how!

This post originally appeared on The Daily WTF.

Leave a Reply

Your email address will not be published. Required fields are marked *