Opened 11 months ago

Closed 3 weeks ago

#27999 closed New feature (fixed)

Add test Client support for HTTP 307 and 308 redirects

Reported by: anentropic Owned by: Tom Forbes
Component: Testing framework Version: master
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 Changed 11 months ago by Claude Paroz

Component: UncategorizedTesting framework
Triage Stage: UnreviewedAccepted
Type: UncategorizedNew feature
Version: 1.10master

comment:2 Changed 11 months ago by Tim Graham

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

comment:3 Changed 9 months ago by Tom Forbes

Owner: changed from nobody to Tom Forbes
Status: newassigned

comment:4 Changed 9 months ago by Tom Forbes

Has patch: set

comment:5 Changed 3 months ago by Tim Martin

Patch needs improvement: set

comment:6 Changed 3 months ago by Tom Forbes

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 Changed 2 months ago by Tim Martin

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 Changed 2 months ago by Tom Forbes

Patch needs improvement: unset

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

comment:9 Changed 3 weeks ago by Carlton Gibson

Triage Stage: AcceptedReady for checkin

OK. Patch looks good.

comment:10 Changed 3 weeks ago by Tim Graham <timograham@…>

Resolution: fixed
Status: assignedclosed

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