Repository navigation
Force GET Redirect on PUT and POST on HTTP Status Code 300-303 #28998
Description
Activity
This behavior should also be in place for PUT and DELETE considering they are not safe functions either
Is there an HTTP-related specification somewhere that dictates that?
We generally follow RFC 7231. Section 6.4 covers behaviors for redirection: https://tools.ietf.org/html/rfc7231.html#section-6.4
Currently, GET request is just forced for POST requests with a 300-303 response.
RFC 7231 describes the behavior for POST redirects and indicates it should transform to GET.
This behavior should also be in place for PUT and DELETE considering they are not safe functions either. RestSharp does it like that.
However, there are no described behaviors in the RFC for transforming PUT or DELETE. If RestSharp is doing that, then it is inventing a new behavior.
What do browsers do in these cases?
Alright. You are right the RFC's are not clear. I tried tools like postman that did the mapping to GET even for PUT and DELETE. My understanding is that due to the confusion 307 and 308 were made to ensure request method are not rewritten. For 301 and 302 the behavior varies across tools. 303 maps to GET in most cases even for PUT and DELETE.
https://developer.mozilla.org/en-US/docs/Web/HTTP/Redirections#Temporary_redirections.
For 303 the rewriting should happen in my opinion. 301 and 302 are debatable.
It should be noted that forcing a GET on 303 for a PUT is the behavior of System.Net.Http in .NET Framework from what I can tell.
Reacted by Roderic Bos, Timothy Byrd and AraHaanTriage: We should match .NET Framework in cases where RFC is not clear.
Is this stackoverflow answer wrong?
303: Redirect for undefined reason. Typically, 'Operation has completed, continue elsewhere.' Clients making subsequent requests for this resource should not use the new URI. Clients should follow the redirect for POST/PUT/DELETE requests, but use GET for the follow-up request.I just converted a utility from Net Framework to Net 5.0, and the results were not pretty.
I'm doing a PUT to
https://www.example.com/thing/12345and the server returns a 303 with the same URI in the Location header, as a way to say "if you want the new value of this expensive thing, come and GET it".In Net Framework, the retry does a GET on
https://www.example.com/thing/12345and the server returns a 200 result and all is well.In Net 5.0, the retry does another PUT to
https://www.example.com/thing/12345and the server returns 303 again.
This repeats for the default 50 retries, and then I finally get the 303 back to do something with.I'm not clear on what use case this is desirable behaviour.
Since I didn't want to completely disable retries for my GETs, too, the smallest way to compensate seemed to be to set MaxAutomaticRedirections = 1 then manually handle the 303 for the PUT.
Here's my take on the RFC language, taken from https://tools.ietf.org/html/rfc7231#section-6.4
300 Multiple Choicesis a weird one and not really used in practice. The RFC is unclear here. I would suggest we not make any change here.301 Moved Permanentlyand302 Foundboth explicitly state:For historical reasons, a user agent MAY change the request method from POST to GET for the subsequent request.
So I think our current behavior here (only change POST) is correct.
303 See Otherexplicitly states:This status code is applicable to any HTTP method.
So, I think we should change the behavior for 303 to change the method to GET for all methods (not just PUT). I believe this matches both .NET Framework behavior and common practice.
@scalablecory @stephentoub Any concerns here?
Reacted by Marie Píchová and AraHaanIs there a place I can check the Net Framework code to make sure the behaviour is matching?
If there are places where the RFC is unclear, I suggest compatibility with the existing Net Framework.
Otherwise, I can't trust porting code that has worked for years.@stephentoub Any concerns here?
If it brings us better in line with the spec and netfx, sounds good.
@TimothyByrd best option is to write a test for each case and try it out. Code is here: https://referencesource.microsoft.com/
If there are places where the RFC is unclear, I suggest compatibility with the existing Net Framework.
Otherwise, I can't trust porting code that has worked for years.That is our high-level desire as well -- see #28998 (comment).
However note, that we do not have plans to be bug-by-bug compatible with .NET Framework (as it is technically impossible and extremely limits innovation and improvements). If we decide that a behavior on .NET Framework was buggy / undesirable / not conformant with RFC, it might be acceptable to make a breaking change in .NET Core in certain circumstances.
Of course, we do not want to break majority of customers intentionally, so for "bugs" which many customers depend on, we will likely keep them also in .NET Core to make everyone's life easier.Fair enough.
In cases where the RFC is not clear, the code still needs to do something, so figuring out the most sensible something is important.I have updated the PR such that:
- The behaviour will only change for 303/SeeOther, not for any of the other 30x responses.
- A redirect on a 303 will change to a GET for POST (existing behaviour), PUT, DELETE, PATCH and OPTIONS
- GET is already GET and HEAD should stay HEAD
- You said "all" methods - but I kind of think OPTIONS should stay OPTIONS, too.
@karelz @stephentoub Is that what you meant?
A redirect on a 303 will change to a GET for POST (existing behaviour), PUT, DELETE, PATCH and OPTIONS
Why not all methods?
I think special casing HEAD is probably reasonable; HEAD is sort of a special case of GET, and if you did a HEAD in the first place it's pretty weird for that to result in a response body.
But for other methods, including OPTIONS and any unrecognized method, the RFC seems pretty clear that 303 applies to this. The server chose to send a 303. That means it wants the client to follow the redirect.
Why not all methods?
Well, we know the RFC is unclear, and that since HEAD stays HEAD, "all" doesn't really mean "all".
(This paragraph is how I was imagining things.) POST/PUT/PATCH all create/update a resource, so having a redirect to a GET makes sense for the case where assembling the resource is expensive. OPTIONS sounds like it retrieves metadata about a resource, which makes it seem more like HEAD in a way.
To figure things out, I took a brute force approach and checked what Firefox, Edge/Chrome and Net Framework do.
It looks like the only major discrepancy is that Net Framework changes "all" methods (except HEAD) to GET when redirecting on a 300, and doesn't follow the redirect on a 308. I don't suggest doing that, but I do think it should be documented as change between Net Framework and Net.
And I've updated the pull request to change all methods (except HEAD) to GET.
It seems like compatibility with Framework could be useful if RFC is ambiguous. Would it make sense to add AppContext switch @geoffkizer for legacy behavior?
Would it make sense to add AppContext switch @geoffkizer for legacy behavior?
You mean, to match current behavior? I think our current behavior here is pretty broken, so I don't think there's a need for a compat switch.
So to conclude the discussion, here's where we have landed:
(1) 303 should change all methods to GET, except HEAD. This matches .NET Framework and common browser behavior.
(2) 300 behavior is inconsistent across browsers/.NET Framework; we will not make any changes here.If anything looks wrong, please yell now.
BTW @TimothyByrd thanks for doing the research re browser behavior here, that's very helpful.
Reacted by Timothy Byrd- added a commit that references this issue
on Mar 9, 2021 - ghost locked as resolved and limited conversation to collaborators
on Apr 9, 2021
Currently, GET request is just forced for POST requests with a 300-303 response. This behavior should also be in place for PUT and DELETE considering they are not safe functions either. RestSharp does it like that.
If not, I couldn't even figure out a way to keep the current behavior and just handling it differently for these methods. From the looks of it, I'll have to implement the complete redirection logic to achieve the desired behavior.