Fix GH-11274: POST/PATCH request via file_get_contents + stream_context_create switches to GET after a HTTP 308 redirect#11275
Fix GH-11274: POST/PATCH request via file_get_contents + stream_context_create switches to GET after a HTTP 308 redirect#11275ndossche wants to merge 1 commit into
Conversation
…ntext_create switches to GET after a HTTP 308 redirect RFC 7231 states that status code 307 should keep the POST method upon redirect. RFC 7538 does the same for code 308. Although it's not mandated by the RFCs that PATCH is also kept (we can choose), it seems like keeping PATCH will be the most consistent and understandable behaviour. This patch also changes an existing test because it was testing for the wrong behaviour.
bukka
left a comment
bukka
left a comment
There was a problem hiding this comment.
The implementation looks good to me.
| int new_flags = HTTP_WRAPPER_REDIRECTED; | ||
| if (response_code == 307 || response_code == 308) { | ||
| /* RFC 7538 specifies that status code 308 does not allow changing the request method from POST to GET. | ||
| * RFC 7231 does the same for status code 307. |
There was a problem hiding this comment.
The question here is which RFC this code was implemented for. If it was https://datatracker.ietf.org/doc/html/rfc2616#section-10.3.8, then it's not exactly clear (at least to me). Anyway I think it should be right thing to prohibit changing of the method even for 8.1 so I'm fine with treating this as a bug. This is just a side note that we don't always treat things as a bug if they change in the later RFC.
There was a problem hiding this comment.
I see. It gets even more unclear when you read the note below "10.3.3 302 found" in the RFC you linked:
Note: [RFC 1945](https://datatracker.ietf.org/doc/html/rfc1945) and
[RFC 2068](https://datatracker.ietf.org/doc/html/rfc2068) specify that the client is not allowed
to change the method on the redirected request. However, most
existing user agent implementations treat 302 as if it were a 303
response, performing a GET on the Location field-value regardless
of the original request method. The status codes 303 and 307 have
been added for servers that wish to make unambiguously clear which
kind of reaction is expected of the client.
So it sounds like the behaviour even for 302 got changed later on (as is clear from the table in RFC 7538) because lots of clients implemented it in another way than originally intended back in the day.
Fortunately, those rules are better defined nowadays in current RFCs.
2 participants
RFC 7231 states that status code 307 should keep the POST method upon redirect. RFC 7538 does the same for code 308. Although it's not mandated by the RFCs that PATCH is also kept (we can choose), it seems like keeping PATCH will be the most consistent and understandable behaviour.
This patch also changes an existing test because it was testing for the wrong behaviour.