Opened 8 years ago

Closed 7 years ago

#27999 closed New feature (fixed)

Add test Client support for HTTP 307 and 308 redirects

Reported by: Paul Garner Owned by: Tom Forbes
Component: Testing framework Version: dev
Severity: Normal Keywords:
Cc: 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

In the TestClient... 308 status is not recognised at all currently.

307 is just treated the same as the other redirects (301, 302, 303) and is converted to a GET request, regardless of the original request method.

This is incorrect according to the RFCs... 307 and 308 redirects are supposed to preserve the original method and request body, so a POST request resulting in 307 response should cause the client to re-POST the body to the new Location.

​https://tools.ietf.org/html/rfc7231#section-6.4.7
​https://tools.ietf.org/html/rfc7538#page-3

I'm happy to prepare a PR for this if it's agreed this should be changed.

Change History (10)

comment:1 by Claude Paroz, 8 years ago

Component: Uncategorized → Testing framework
Triage Stage: Unreviewed → Accepted
Type: Uncategorized → New feature
Version: 1.10 → master

comment:2 by Tim Graham, 8 years ago

Summary: TestClient does not correctly handle 307 and 308 redirects → Add test Client support for HTTP 307 and 308 redirects

comment:3 by Tom Forbes, 7 years ago

Owner: changed from nobody to Tom Forbes
Status: new → assigned

comment:5 by Tim Martin, 7 years ago

Patch needs improvement: set

comment:6 by Tom Forbes, 7 years ago

Patch needs improvement: unset

Not sure if I'm meant to remove this flag myself after I've made the changes the reviewer requested, please let me know if not.

comment:7 by Tim Martin, 7 years ago

Patch needs improvement: set

It's fine to clear the "patch needs improvement" flag yourself, since that's one of the main ways reviewers can spot that it's ready to re-review. As it happens, there are a couple more style issues (not from me) on the patch, so I'm going to set this again for now.

comment:8 by Tom Forbes, 7 years ago

Patch needs improvement: unset

Thanks! I've made some changes based on the reviews

comment:9 by Carlton Gibson, 7 years ago

Triage Stage: Accepted → Ready for checkin

OK. Patch looks good.

comment:10 by Tim Graham <timograham@…>, 7 years ago

Resolution: → fixed
Status: assigned → closed

In 272f685:

Fixed #27999 -- Added test client support for HTTP 307 and 308 redirects.

Note: See TracTickets for help on using tickets.
Back to Top