Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Preserve query string on redirect #609

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

c-w
Copy link

@c-w c-w commented Sep 19, 2024

Hi there 👋

On one of my Django projects, I got tripped up by whitenoise stripping query strings on redirect, so for example /foo?bar=baz redirects to /foo/ when I was expecting it to instead redirect to /foo/?bar=baz. Would you be open to merging a patch that preserves the query string on redirect?

I'm opening a pull request to show what an implementation could look like and to start the discussion. If you're happy with the proposal, I can work on putting the functionality behind a setting (to not break backwards compatibility), unit tests, and what-ever else is needed.

Thanks for your time!

@Archmonger
Copy link

Archmonger commented Sep 27, 2024

Given that WhiteNoise (and by extension ServeStatic) currently do nothing with query strings, what is the proposed use case of preserving query strings?

@c-w
Copy link
Author

c-w commented Sep 27, 2024

My use-case is that I'd like to have links to pages like /some/page?utm_source=value but whitenoise will currently redirect this to /some/page/ which means that client-side code loses access to the query param.

@Archmonger
Copy link

Archmonger commented Sep 27, 2024

So if I understand correctly, in this scenario you (or some proxy) are creating/embedding tracking IDs into static files for tracking/metrics purposes?

@c-w
Copy link
Author

c-w commented Sep 28, 2024

Yes, essentially that's right. The example I used is for tracking conversion rates from Facebook ads. But the use case is actually still broader.

Concretely, I'm using whitenoise to serve a NextJS statically generated app. This means that I have several index.html files across various directories which get served, but within each of these files all logic happens client-side in React. So losing the query params on redirect means that I can't customise what happens on the page via shareable links like for example /some/page?search=foo to link to a search results page which will fetch the data for foo client-side.

Technically I can do what I need by including a trailing slash in all URLs (thus bypassing the redirect), but that's ugly and more importantly error prone as it's easy to forget.

@Archmonger
Copy link

Archmonger commented Sep 28, 2024

Okay I understand the benefit. Would you be interested in duplicating this PR into ServeStatic?

For the last 5 years, things have been moving fairly slowly in WhiteNoise I don't know if/when this PR would get merged here.

@c-w
Copy link
Author

c-w commented Sep 30, 2024

@Archmonger Done, see Archmonger/ServeStatic#48

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants