Skip to content

Fixed #26293 - Moved append_slash logic outside of prepend_www logic#6312

Closed
samuelmullin wants to merge 1 commit into
django:masterfrom
samuelmullin:ticket_26293
Closed

Fixed #26293 - Moved append_slash logic outside of prepend_www logic#6312
samuelmullin wants to merge 1 commit into
django:masterfrom
samuelmullin:ticket_26293

Conversation

@samuelmullin
Copy link
Copy Markdown

Moving logic required dynamically building the redirect_url as well as updating test_non_ascii_query_string_does_not_crash to expect a 301 from process_request instead of expecting no redirect.

Also added a regression test (and renamed another test to differentiate the two).

Comment thread tests/middleware/tests.py Outdated
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since process_request() returns a response, the second part of the test isn't needed (see 434d309).

@timgraham
Copy link
Copy Markdown
Member

Some edits: http://dpaste.com/2J098KX
We should probably backport to 1.9 since it's a regression. Could you add a release note for 1.9.5 and try to rephrase the commit message to describe the issue that's fixed rather than the implementation? Thanks!

This issue was a regression that was causing appending a slash to not
return a redirect unless we also prepended a www, as the logic to
append the slash was located side the prepend WWW logic.

Moving logic required dynamically building the redirect_url as well
as updating test_non_ascii_query_string_does_not_crash to expect
a 301 from process_request instead of expecting no redirect.

Also added a regression test (and renamed another test to differentiate the two)
and release notes so this can be included in the 1.9.5 release.
@samuelmullin
Copy link
Copy Markdown
Author

I've updated the tests as you requested, changed the commit message and added release notes under 1.9.5 - let me know if anything else needs to be updated/changed!

@samuelmullin
Copy link
Copy Markdown
Author

Oh, I totally missed your edits. Will add those later. Thanks for your feedback!

@timgraham
Copy link
Copy Markdown
Member

merged in 9390da7, thanks!

@timgraham timgraham closed this Mar 23, 2016
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