search

skybrud / Skybrud.Umbraco.Redirects

Redirects manager for Umbraco.
https://packages.limbo.works/skybrud.umbraco.redirects/
MIT License
38 stars 41 forks source link

Ignore query string on original URL #43

Closed ronaldbarendse closed 2 years ago

ronaldbarendse commented 5 years ago

When the original URL contains a query string, e.g. /bad-url?cookie=yes and a redirect exists for /bad-url, it doesn't match. Only when the option 'Forward query string' is enabled, the redirect kicks in (although it keeps the unnecessary query string)...

I would expect the redirect would work anyway, because anyone could just add a query string to the bad/old URL (some tools do this, e.g. link shortners, social media, Google ads).

ronaldbarendse commented 5 years ago

Should be easily fixed by removing && x.ForwardQueryString from:

https://github.com/skybrud/Skybrud.Umbraco.Redirects/blob/542bac07140d3614e28653ed546d4ebb9d768a77/src/Skybrud.Umbraco.Redirects/Models/RedirectsRepository.cs#L271

abjerner commented 5 years ago

@ronaldbarendse thanks for reporting 👍

As the package has worked by checking against both the path and query for a while now, I think it will be a breaking change to skip the check on the query string.

I also fear that changing this could have more consequences. For instance it will be harder to check if a redirect already exists as it can then be either a full match or a partial match.

Nevertheless, I see your reasons for creating this issue. I will try to raise this issue with my colleagues to find the best approach.

ronaldbarendse commented 5 years ago

No breaking change is needed, as it first checks for a redirect with the query string and if it doesn't find one, tries to get the redirect only by URL.

The ForwardQueryString option should only impact the handling of matched redirects, not be a predicate when looking up redirects, right?

ronaldbarendse commented 5 years ago

@abjerner Any chance of looking at the submitted PR? With the latest commit, this should work as expected: https://github.com/skybrud/Skybrud.Umbraco.Redirects/pull/44/files?w=1

So if a redirect without query string exists, but the incoming URL does have a query string, it first checks for the exact match (URL and query string), but if it can't find one, checks for a redirect without a query string.

MMasey commented 2 years ago

Out of curiosity, what's the current status of this issue? I have a V8 site that is experiencing this same issue. For a quick patch I could fork the repo and pull this @ronaldbarendse's fix above and build it locally, but just wanted to see if it might be actioned in this repo.

I know you're probably super busy @abjerner so no worries.