Code

Opened 4 years ago

Closed 6 months ago

Last modified 6 months ago

#13725 closed Bug (fixed)

assertRedirects not taking url scheme into account

Reported by: rvdrijst Owned by: unaizalakain
Component: Testing framework Version: master
Severity: Normal Keywords: test client assertRedirects https scheme
Cc: rvdrijst Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

The assertRedirects method of TestCase doesn't work correctly with https redirects. There are actually two problems here.

Take the following scenario. You access a view with test client mimicking https by passing in **{'wsgi.url_scheme':'https'}. This view then redirects to a relative url which is converted by fix_location_header to an absolute url with https since the request is_secure(). So far so good.

But assertRedirects will blatantly ignore the scheme, checking the redirect url without the scheme. Since I have views that will redirect to https if not called with https, this will fail because the redirect check will again be redirected because the scheme was ignored.

This first problem can be fixed by adding **{'wsgi.url_scheme':scheme} to the redirect check and also taking the scheme into account when matching the url with expected, but this creates another problem. The fix_location_header response fix will build the absolute url using request.get_host(), which for the test client will return testserver:80. The port part is not expected by assertRedirects, which again fails, trying to match the expected https://testserver/some/path with https://testserver:80/some/path.

Attachments (0)

Change History (10)

comment:1 Changed 4 years ago by russellm

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted

comment:2 Changed 3 years ago by julien

  • Severity set to Normal
  • Type set to Bug

comment:3 Changed 2 years ago by aaugustin

  • UI/UX unset

Change UI/UX from NULL to False.

comment:4 Changed 2 years ago by aaugustin

  • Easy pickings unset

Change Easy pickings from NULL to False.

comment:5 Changed 6 months ago by unaizalakain

  • Owner changed from nobody to unaizalakain
  • Status changed from new to assigned

Depends on #21341

comment:6 Changed 6 months ago by unaizalakain

  • Version changed from 1.2 to master

comment:7 Changed 6 months ago by unaizalakain

  • Has patch set

comment:8 Changed 6 months ago by loic84

  • Triage Stage changed from Accepted to Ready for checkin

Other than the comment I left on the PR, LGTM.

comment:9 Changed 6 months ago by Unai Zalakain <unai@…>

  • Resolution set to fixed
  • Status changed from assigned to closed

In 9c5f6cd565f1bc519dec20e3e44313cb8d8e0e94:

Fixed #13725 -- take url scheme into account in assertRedirects

Scheme is handled correctly when making comparisons between two URLs. If
there isn't any scheme specified in the location where we are redirected to,
the original request's scheme is used. If present, the scheme in
expected_url is the one used to make the comparations to.

comment:10 Changed 6 months ago by Anssi Kääriäinen <akaariai@…>

In 30203a0deaf82c8af26b1b5453851f83d2248b67:

Merge pull request #1850 from unaizalakain/ticket_13725

Fixed #13725 -- take url scheme into account in assertRedirects

Thanks to Loic for review.

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.