search

skybrud / Skybrud.Umbraco.Redirects

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

Query String in URL returns 404 when forward query toggle is off #187

Closed Rhoyo closed 1 year ago

Rhoyo commented 1 year ago

Which version of Skybrud Redirects are you using? (Please write the exact version, example: 4.0.8)

4.0.8

Which Umbraco version are you using? (Please write the exact version, example: 10.1.0)

10.1.4

Bug description

Screenshot 2023-10-04 at 4 31 52 PM

I have the forward query string toggled to disabled. However, when I curl and add a query string at the end it returns a 404, serving my custom 404 page. Not a 301 response. It seems it acts as it should when no query is added.

Screenshot 2023-10-04 at 4 33 46 PM Screenshot 2023-10-04 at 4 33 25 PM

When I toggle the forward query string and add a query string to my link it acts as it should, with and without a query string.

Screenshot 2023-10-04 at 4 36 04 PM Screenshot 2023-10-04 at 4 37 07 PM

If I may ask, how come it returns a 404 when the forward query toggle is turned off and a query string is added? Is this something that could be accounted for or perhaps not? I was thinking more on the lines that the package would handle the query string, clip it off and return the url without the query string as a 301 response when the forward query toggle is off. For example: localhost:44388/test?q=test would be localhost:44388/test-page/ with a 301 status code instead of a 404. Thanks again @abjerner

Rockerby commented 1 year ago

I came across this today, but actually looks like this is intended behaviour. Some sites would want /test?x=y and /test to be different pages altogether. In my experience I feel as though the default behaviour should be to have Forward query string on, allowing users to turn it off.

What I think would be great is some default settings we could change; luckily enough there's something discussed in i151 that fits.

abjerner commented 1 year ago

Like @Rockerby mentions, this is the indented behavior. It's something we settled on when starting the package, but since that was back in 2016, I can't remember exactly why we settled on one over the other. It might have been because that aligned best with our client's needs.

Since it has been this way for years, I'm not a big fan of changing it. But I'm all for making it a bit easier to customize the default value - eg. as suggested in https://github.com/skybrud/Skybrud.Umbraco.Redirects/issues/151#issuecomment-1752655093