PATCH get response 301(moved permanently) is redirected with empty body

I don't know this is normal behaviour or not but any reason to set body to empty with patch?


What kind of body are you sending?


i sent something like this
`r = requests.patch(url, data={"data": "test"})`


I think we're a bit over-zealous with stripping bodies on redirects. In #1084 we allowed 307 and 308 to keep their bodies, but all other redirects lose theirs. Arguably, a 301 for a non-POST should also behave the same way. Assigning this to 3.0.0, because it'd be a breaking change.


Same for response 302.


<blockquote>I think we're a bit over-zealous with stripping bodies on redirects.</blockquote>

Out of interest, why would you _ever_ strip bodies?


On a redirect? Consider being redirected to a different domain that you did not want to send that body to... That is why we tend to be overzealous


If you don't want to send the body to the new location, presumably, you don't want to send the request at all. Either preserving the body or raising an exception would be sensible behavior. Sending an altogether _different_ request to the new location makes no sense at all, IMHO. Certainly not as default behavior.

-------- Original message --------
From: Ian Cordasco notifications@github.com 
Date:  
To: kennethreitz/requests requests@noreply.github.com 
Cc: Kilian Evang intendanz@texttheater.net 
Subject: Re: [requests] PATCH get response 301(moved permanently) is redirected with empty body (#2590) 

On a redirect? Consider being redirected to a different domain that you did not want to send that body to... That is why we tend to be overzealous

—
Reply to this email directly or view it on GitHub.


> Out of interest, why would you ever strip bodies?

Well that's easy, really. A very common pattern is to POST to some endpoint, receive a 302, and GET the location from the 302. You obviously want to strip the body in that case.

> Sending an altogether _different_ request to the new location makes no sense at all, IMHO. Certainly not as default behavior.

Again, sure it does. That's exactly what many redirects _are_. Status codes 301, 302 are standardised to turn POST into GET (which is definitionally an "altogether different request"), and status code 303 is standardised to turn _any_ verb into HEAD or GET (again, an "altogether different request"). If your web server really wants to keep everything the same it should issue 307 (302 with no verb change) or 308 (301 with no verb change).


Now I'm really confused.

<blockquote>A very common pattern is to POST to some endpoint, receive a 302, and GET the location from the 302. You obviously want to strip the body in that case.</blockquote>

What is the purpose of this pattern?

From the perspective of the client, requests is doing something completely different from what it was asked to do, without giving any notice about this.

<blockquote>Status codes 301, 302 are standardised to turn POST into GET</blockquote>

Where is that standardized? On http://www.w3.org/Protocols/rfc2616/rfc2616-sec10.html it says this is erroneous behavior:

<blockquote>Note: When automatically redirecting a POST request after receiving a 301 status code, some existing HTTP/1.0 user agents will erroneously change it into a GET request.</blockquote>

Also, if the current behavior is expected for 301 and 302 (which I think it isn't), aren't you contradicting your own answer in this thread: https://github.com/kennethreitz/requests/issues/2590#issuecomment-98685804


RFC 2616 is _not_ the current HTTP/1.1 standard. In this case, you want RFC 7231 which has the following blocks.

In [section 6.4](https://tools.ietf.org/html/rfc7231#section-6.4):

> Note: In HTTP/1.0, the status codes 301 (Moved Permanently) and 302 (Found) were defined for the first type of redirect ([RFC1945], Section 9.3).  Early user agents split on whether the method applied to the redirect target would be the same as the original request or would be rewritten as GET.  Although HTTP originally defined the former semantics for 301 and 302 (to match its original implementation at CERN), and defined 303 (See Other) to match the latter semantics, prevailing practice gradually converged on the latter semantics for 301 and 302 as well.  The first revision of HTTP/1.1 added 307 (Temporary Redirect) to indicate the former semantics without being impacted by divergent practice.  Over 10 years later, most user agents still do method rewriting for 301 and 302; therefore, this specification makes that behavior conformant when the original request is POST.

And in [section 6.4.2](https://tools.ietf.org/html/rfc7231#section-6.4.2), on the 301 status code:

> Note: For historical reasons, a user agent MAY change the request method from POST to GET for the subsequent request.  If this behavior is undesired, the 307 (Temporary Redirect) status code can be used instead.

And in [section 6.4.3](https://tools.ietf.org/html/rfc7231#section-6.4.3) on the 302 status code:

> Note: For historical reasons, a user agent MAY change the request method from POST to GET for the subsequent request.  If this behavior is undesired, the 307 (Temporary Redirect) status code can be used instead.

And in [section 6.4.4](https://tools.ietf.org/html/rfc7231#section-6.4.4) on the 303 status code:

> A user agent can perform a retrieval request targeting that URI (a GET or HEAD request if using HTTP), which might also be redirected, and present the eventual result as an answer to the original request. 

And in [section 6.4.7](https://tools.ietf.org/html/rfc7231#section-6.4.7) on the 307 status code:

> Note: This status code is similar to 302 (Found), except that it does not allow changing the request method from POST to GET.  This specification defines no equivalent counterpart for 301 (Moved Permanently) ([RFC7238], however, defines the status code 308 (Permanent Redirect) for this purpose).

You asked:

> What is the purpose of this pattern?

It's usually used for form submission. A HTTP form is _usually_ submitted using a POST, but most servers will not return a page that way. Instead, they take the POST and then issue a redirect to some GET-able resource that the browser can display. Requests will appear to be broken if it doesn't behave the same way as browsers do, so we need to follow that logic.

> Also, if the current behavior is expected for 301 and 302 (which I think it isn't), aren't you contradicting your own answer in this thread: #2590 (comment)

No, I'm not. =) I was a) answering your question about stripping bodies, and b) explaining that plenty of redirect codes do generate entirely different requests from the redirect. However, I do not believe that 301 to PATCH should do that, expressly because the spec doesn't expect so either. I think there's still a bug here, don't you worry! =)


That explains it, thanks a lot. Also, I somehow overlooked this issue is only about PATCH. Sorry about that.


No problem at all. =)


This happens with PUT requests returning 301, too. I see you mentioned 

> Arguably, a 301 for a non-POST should also behave the same way

Is it correct behaviour for PUT as well?

Thanks!


@borbamartin I believe so, yes. I think really _only_ the logic we use with POST is wrong here.


Thanks @Lukasa !


So I think I have a PR ready for this, but I have a couple of clarifying questions.

>In #1084 we allowed 307 and 308 to keep their bodies, but all other redirects lose theirs. Arguably, a 301 for a non-POST should also behave the same way.

I'm reading this as "a 301 for a non-POST should also behave like a 307/308" which seems to be consistent with RFC 7231. However, the last few comments are making me wonder if I'm interpreting this wrong.

>This happens with PUT requests returning 301, too. [...] Is it correct behaviour for PUT as well?

> I believe so, yes. 

Are you saying PUT should be converted to a GET like POST? That seems reasonable, but I can't find any examples of this happening in the wild, or even in the RFC.

>I think really *only* the logic we use with POST is wrong here.

POST requests are converted to a GET like browsers currently do. Isn't that the behaviour we want here? I've been reading this issue as the logic for everything *except* POST (and GET/HEAD) needs to be adjusted here.

Yeah, sorry, I think 301 to PUT should also stay PUT.

This will be resolved in 3.0.0 with #3757.