SetQueryParam Bug - If Key is substring of other querystring param, it matches incorrectly

1 
[TestMethod]
public void ServiceStackSetQueryParamWorksWhenMatchingParameterIsSubsetOfKey()
{
    var url = "http://www.google.com?PageSize=25".SetQueryParam("Page", "1");
    Assert.AreEqual("http://www.google.com?PageSize=25&Page=1", url);
}

The above test case fails. Instead I end up with ?Page=1 in the querystring, with PageSize being eliminated. I’m on v4.x of SS (can’t update to 5 just yet).

2

Changing it to look for key= insetad of just key fixes it. However, there might be a reason you weren’t doing that lookup?

[TestMethod]
public void ServiceStackSetQueryParamWorksWhenMatchingParameterIsSubsetOfKey()
{
    var url = SetQueryParam("http://www.google.com?PageSize=25", "Page", "1");
 
    Assert.AreEqual("http://www.google.com?PageSize=25&Page=1", url);
}
 
public static string SetQueryParam(string url, string key, string val)
{
    if (string.IsNullOrEmpty(url))
        return (string) null;
 
    int startIndex1 = url.IndexOf('?');
 
    if (startIndex1 != -1)
    {
        int num = (startIndex1 + 1 == url.IndexOf(key + "=", startIndex1, PclExport.Instance.InvariantComparison))
                        ? startIndex1 
                        : url.IndexOf("&" + key + "=", startIndex1, PclExport.Instance.InvariantComparison);
 
        if (num != -1)
        {
            int startIndex2 = url.IndexOf('&', num + 1);
            if (startIndex2 == -1)
                startIndex2 = url.Length;
            return url.Substring(0, num + key.Length + 1) + "=" + val.UrlEncode(false) + url.Substring(startIndex2);
        }
    }
 
    string str = startIndex1 == -1 ? "?" : "&";
 
    return url + str + key + "=" + val.UrlEncode(false);
}
3

Use AddQueryParam() extension method when you want to add additional Params to the QueryString.

4

The query param might already exist. So now you want me to check if it exists and use Add instead in those cases, and Set (which will still do it wrong) in others? I don’t think that solution changes the fact that the Set method is wrong.

5

Ok yeah I see what bug you’re referring to now, thought you were just using the wrong API.

This should be resolved in this commit from v5.0.3 that’s now available on MyGet.

thx for the fix + repro!

1 Like