I'm writing middleware that checks if `request.get_host()` is coupled to a
site, else we do a redirect to an external domain.
{{{
def test_redirect(self):
response = self.client.get("/", follow=False,
HTTP_HOST=self.site1.domain)
self.assertRedirects(response, '/accounts/login/?next=',
fetch_redirect_response=True)
}}}
The `assertRedirect` does an extra request to follow the redirect. What
goes wrong is that the HTTP_HOST with the request is not set to
'{{ self.site1.domain }}' but to `testserver`. This results in a redirect
to our external domain.
Middleware code:
{{{
class SiteMiddleware(object):
def __init__(self, get_response):
self.get_response = get_response
def __call__(self, request):
host = self.request.get_host()
try:
request.site = Site.objects.get(domain=host)
except ObjectDoesNotExist:
return HttpResponseRedirect("http://example.com")
return self.get_response(request)
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/28337>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
* type: Bug => Cleanup/optimization
* stage: Unreviewed => Accepted
--
Ticket URL: <https://code.djangoproject.com/ticket/28337#comment:1>
* owner: nobody => Patrick Jenkins
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/28337#comment:2>
Comment (by Patrick Jenkins):
The problem with the provided example is that under certain conditions
assertRedirect will replay the request with different parameters than
originally specified. The conditions necessary for this to occur are
passing follow=False to the original client.get and also passing 'extra'
parameters that will be interpreted as HTTP header's. Assert redirect
(version 1.11.4:
https://github.com/django/django/blob/1a34dfcf797640d5d580d261694cb54e6f97c552/django/test/testcases.py#L317)
makes an assumption that it is possible to exactly recreate the original
request with the information that is available.
To properly recreate the the original request it would be necessary to
pass ALL method parameters AND all of the kwarg parameters that were
originally passed in. This would require storing the 'extras' in the
Client (django.test.client.py) so they could be acccessed in
assertRedrrects and properly replayed.
I believe this is the proper solution because it removes the assumptions
present when assertRedirect attempts to recreate the request. Although
this is a corner case, it is certainly acceptable for Django users to
utilize the infinite space that is HTTP headers and provide varying logic
depending on the values present. The downside of this implementation is
that it bloats the Client by requiring it to store the extra parameters
passed in. These extra parameters will rarely be used but I believe the
tradeoff of adding properties to Client and potentially increasing memory
footprint are small enough to warrant this change.
--
Ticket URL: <https://code.djangoproject.com/ticket/28337#comment:3>
* has_patch: 0 => 1
Comment:
Pull request https://github.com/django/django/pull/8931
--
Ticket URL: <https://code.djangoproject.com/ticket/28337#comment:4>
* needs_better_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/28337#comment:5>
* owner: Patrick Jenkins => Hasan Ramezani
* needs_better_patch: 1 => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/28337#comment:6>
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"46e74a525671f2eef09021c06933c45b49f9d421" 46e74a52]:
{{{
#!CommitTicketReference repository=""
revision="46e74a525671f2eef09021c06933c45b49f9d421"
Fixed #28337 -- Preserved extra headers of requests made with
django.test.Client in assertRedirects().
Co-Authored-By: Hasan Ramezani <hasa...@gmail.com>
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/28337#comment:7>