Orchard 1.7 Orchard.OutputCache Module form redirects adds querystring, i have code for querystring suppression, but any down sides?

[orchardbot]

JhonVi created: https://orchard.codeplex.com/workitem/21215

Regarding the Orchard 1.7 Orchard.OutputCache (version listed below) We have SEO releated need to suppress the querystring from being appended on a form redirect to a simple "thank you" page. The Cache module wants to append a QS but not sure why.

See line 452 in Orchard.OutputCache.Filters

var querystring = "?" + string.Join("&", Array.ConvertAll(qs.AllKeys, k => string.Format("{0}={1}", HttpUtility.UrlEncode(k), HttpUtility.UrlEncode(qs[k]))));

What I have done seems too simple a fix, and I fear I am missing some landmine.

I have simply changed the code a bit to add a condition -

I get the "ignored" and check it against the "redirecturl" that was retrieved in line 421. So now instead of always running the line above, if the redirect url is declared in ignore list, no querystring appended - good for SEO team. These "thank you" pages are basic CMS content so are not effected by the form entry.

My question is - But what are the down sides if any?

Below is updated code around line 452 in Orchard.OutputCache.Filters of

--- module.txt--

Name: Output Cache AntiForgery: enabled Author: Sébastien Ros Website: http://orchardproject.net/ Version: 1.7.2 OrchardVersion: 1.7.2

code was changed from simply...

var querystring = "?" + string.Join("&", Array.ConvertAll(qs.AllKeys, k => string.Format("{0}={1}", HttpUtility.UrlEncode(k), HttpUtility.UrlEncode(qs[k])))); to...

var querystring = "";

_ignoredUrls = _cacheManager.Get("CacheSettingsPart.IgnoredUrls", context => { context.Monitor(_signals.When(CacheSettingsPart.CacheKey)); return _workContext.CurrentSite.As().IgnoredUrls;

} );

if (!IsIgnoredUrl(redirectUrl, _ignoredUrls)) { querystring = "?" + string.Join("&", Array.ConvertAll(qs.AllKeys, k => string.Format("{0}={1}", HttpUtility.UrlEncode(k), HttpUtility.UrlEncode(qs[k]))));

} We have many sites (dozens) that will need this update, and this seems to work fine, My question is - But what are the down sides if any? Might anyone know why that querystring is append on a form submission redirect in the first place?

I'd rather not want to find bad news after releasing this update all around.

Any comments at all are very welcome.

Thanks in advance.

[orchardbot]

@jtkech commented:

It's when Orchard add a RefreshKey (based on time ticks) to the query string, this to prevent to render a cached version (from server / client / proxies) on a post redirection. In some versions the RefreshKey was removed, then re-intoduced, in part because of an incorrect key used to remove the cached item on server side (now resolved)

Anyway, the RefreshKey is useful to prevent from client / proxies caching. Currently, the RefreshKey isn't used if the maxAge setting is > 0, this because here you only use server caching. That said, in your case you can safely not to use the RefreshKey in the query string for ignoredUrls

I've not a 1.7.2 version and I don't see all your code, but it seems that, for ignoredUrls, you remove all the query string. Take care if you have some redirections in this case but with regular query parameters you want to preserve

In fact, in your case, for ignoredUrls you only have to use the original redirectUrl at the end of the TransformRedirect() method, this will preserve possible regular query string parameters

Best,