1.9 introduced the "HTTP redirects no longer forced to absolute URIs"
change and the fix has disappeared, the reason being "... allows relative
URIs in Location, recognizing the actual practice of user agents, almost
all of which support them.".
Unfortunately, this is not fully the case of test.Client. It doesn't
support relative-path reference - not beginning with a slash character,
but only absolute-path reference - beginning with a single slash character
([https://tools.ietf.org/html/rfc3986#section-4.2, ref]). A GET to
{{{inbox/}}} leads to 404.
I'm using a workaround with {{{... url=reverse_lazy('app_name:inbox')
...}}}, referred by a TestCase.urls attribute, to produce
{{{/messages/inbox/}}}, but I'm not happy with this hardcoded namespace.
--
Ticket URL: <https://code.djangoproject.com/ticket/26428>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
* severity: Normal => Release blocker
* needs_better_patch: => 0
* needs_tests: => 0
* needs_docs: => 0
* type: Bug => New feature
* stage: Unreviewed => Accepted
Comment:
I think we should try to fix it in 1.9 given the regression nature of the
report. A test is attached.
--
Ticket URL: <https://code.djangoproject.com/ticket/26428#comment:1>
* Attachment "26428-test.diff" added.
* has_patch: 0 => 1
Comment:
Does [https://github.com/django/django/pull/6365 this patch] fix your
issue? If not, could you provide a sample of your test code so we
understand exactly what you're doing? Thanks.
--
Ticket URL: <https://code.djangoproject.com/ticket/26428#comment:2>
Comment (by master):
Replying to [comment:2 timgraham]
The patch does fix my issue but a simple concatenation doesn't cover all
cases.
Suppose your test scenario as: {{{url(r'^accounts/optional_extra$',
RedirectView.as_view(url='login/')),}}}
with: {{{response = self.client.get('/accounts/optional_extra')}}}
I think a more appropriate code is:
{{{
# Prepend the request path to handle relative-path redirects.
if not path.startswith('/'):
url = urljoin(response.request['PATH_INFO'], url)
path = urljoin(response.request['PATH_INFO'], path)
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/26428#comment:3>
Comment (by timgraham):
Thanks for the feedback. I updated the pull request for your suggestion
and added some release notes for 1.9.6.
--
Ticket URL: <https://code.djangoproject.com/ticket/26428#comment:4>
Comment (by master):
Replying to [comment:4 timgraham]
Typo in 1.9.6.txt
--
Ticket URL: <https://code.djangoproject.com/ticket/26428#comment:5>
* status: new => closed
* resolution: => fixed
Comment:
In [changeset:"d2569f89f28883d07ede0e44a0a69ae678d3b35f" d2569f89]:
{{{
#!CommitTicketReference repository=""
revision="d2569f89f28883d07ede0e44a0a69ae678d3b35f"
Fixed #26428 -- Added support for relative path redirects in
assertRedirects().
Thanks Trac alias master for the report and review.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/26428#comment:6>
Comment (by Tim Graham <timograham@…>):
In [changeset:"192b06594958b701a4b47dd51148a15357ac8f76" 192b0659]:
{{{
#!CommitTicketReference repository=""
revision="192b06594958b701a4b47dd51148a15357ac8f76"
[1.9.x] Fixed #26428 -- Added support for relative path redirects in
assertRedirects().
Thanks Trac alias master for the report and review.
Backport of d2569f89f28883d07ede0e44a0a69ae678d3b35f from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/26428#comment:7>
* Attachment "26428-extra.diff" added.
Also fixes Client.get()
* status: closed => new
* resolution: fixed =>
Comment:
Though the fix solves the original problem i.e. assertRedirects(), the
summary of the ticket suggests broader support.
I've attached a patch based on the original fix, that also fixes
Client.get() / Client.post() etc. when follow=True.
--
Ticket URL: <https://code.djangoproject.com/ticket/26428#comment:8>
Comment (by Tim Graham <timograham@…>):
In [changeset:"2f698cd9916cb3d64932248c7114049e02b74fb3" 2f698cd]:
{{{
#!CommitTicketReference repository=""
revision="2f698cd9916cb3d64932248c7114049e02b74fb3"
Refs #26428 -- Added support for relative path redirects to the test
client.
Thanks iktyrrell for the patch.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/26428#comment:9>
Comment (by Tim Graham <timograham@…>):
In [changeset:"d2338bae786f2fabfd01c30522fda9d79e02eb45" d2338ba]:
{{{
#!CommitTicketReference repository=""
revision="d2338bae786f2fabfd01c30522fda9d79e02eb45"
[1.9.x] Refs #26428 -- Added support for relative path redirects to the
test client.
Thanks iktyrrell for the patch.
Backport of 2f698cd9916cb3d64932248c7114049e02b74fb3 from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/26428#comment:10>
* status: new => closed
* resolution: => fixed
--
Ticket URL: <https://code.djangoproject.com/ticket/26428#comment:11>
Comment (by Jacob):
This issue was not fully addressed. There are still a couple problems:
1) It assumes that a relative redirect does not start with '/'. But what
if you redirect relative to host with a '/', as in redirecting to
'/subpath' instead of 'https://example.com/subpath'?
2) It does not set the secure kwarg on the client.get call based on the
original host scheme, because urlsplit is called only on the relative
path. It seems that urlsplit should be called after building an absolute
url.
Should this ticket be re-opened, or a separate ticket created?
--
Ticket URL: <https://code.djangoproject.com/ticket/26428#comment:12>
Comment (by Tim Graham):
New ticket, please, as the fix for this one has been released for years.
--
Ticket URL: <https://code.djangoproject.com/ticket/26428#comment:13>