From dc7ffd50c131cbd8cf61fe319edeec11ceb19f88 Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Fri, 19 May 2023 12:55:39 +0200 Subject: [PATCH] Fix GH-11274: POST/PATCH request via file_get_contents + stream_context_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. --- ext/standard/http_fopen_wrapper.c | 21 ++++++--- ext/standard/tests/http/bug67430.phpt | 2 +- ext/standard/tests/http/gh11274.phpt | 62 +++++++++++++++++++++++++++ 3 files changed, 78 insertions(+), 7 deletions(-) create mode 100644 ext/standard/tests/http/gh11274.phpt diff --git a/ext/standard/http_fopen_wrapper.c b/ext/standard/http_fopen_wrapper.c index fa0dcb5e6890..f6a4a094b425 100644 --- a/ext/standard/http_fopen_wrapper.c +++ b/ext/standard/http_fopen_wrapper.c @@ -79,6 +79,7 @@ #define HTTP_WRAPPER_HEADER_INIT 1 #define HTTP_WRAPPER_REDIRECTED 2 +#define HTTP_WRAPPER_KEEP_METHOD 4 static inline void strip_header(char *header_bag, char *lc_header_bag, const char *lc_header_name) @@ -140,6 +141,7 @@ static php_stream *php_stream_url_wrap_http_ex(php_stream_wrapper *wrapper, char *user_headers = NULL; int header_init = ((flags & HTTP_WRAPPER_HEADER_INIT) != 0); int redirected = ((flags & HTTP_WRAPPER_REDIRECTED) != 0); + int redirect_keep_method = ((flags & HTTP_WRAPPER_KEEP_METHOD) != 0); bool follow_location = 1; php_stream_filter *transfer_encoding = NULL; int response_code; @@ -363,8 +365,8 @@ static php_stream *php_stream_url_wrap_http_ex(php_stream_wrapper *wrapper, if (context && (tmpzval = php_stream_context_get_option(context, "http", "method")) != NULL) { if (Z_TYPE_P(tmpzval) == IS_STRING && Z_STRLEN_P(tmpzval) > 0) { /* As per the RFC, automatically redirected requests MUST NOT use other methods than - * GET and HEAD unless it can be confirmed by the user */ - if (!redirected + * GET and HEAD unless it can be confirmed by the user. */ + if (!redirected || redirect_keep_method || zend_string_equals_literal(Z_STR_P(tmpzval), "GET") || zend_string_equals_literal(Z_STR_P(tmpzval), "HEAD") ) { @@ -458,7 +460,7 @@ static php_stream *php_stream_url_wrap_http_ex(php_stream_wrapper *wrapper, zend_str_tolower(ZSTR_VAL(tmp), ZSTR_LEN(tmp)); t = ZSTR_VAL(tmp); - if (!header_init) { + if (!header_init && !redirect_keep_method) { /* strip POST headers on redirect */ strip_header(user_headers, t, "content-length:"); strip_header(user_headers, t, "content-type:"); @@ -606,7 +608,7 @@ static php_stream *php_stream_url_wrap_http_ex(php_stream_wrapper *wrapper, * see bug #44603 for details. Since Content-Type maybe part of user's headers we need to do this check first. */ if ( - header_init && + (header_init || redirect_keep_method) && context && !(have_header & HTTP_HEADER_CONTENT_LENGTH) && (tmpzval = php_stream_context_get_option(context, "http", "content")) != NULL && @@ -624,7 +626,7 @@ static php_stream *php_stream_url_wrap_http_ex(php_stream_wrapper *wrapper, } /* Request content, such as for POST requests */ - if (header_init && context && + if ((header_init || redirect_keep_method) && context && (tmpzval = php_stream_context_get_option(context, "http", "content")) != NULL && Z_TYPE_P(tmpzval) == IS_STRING && Z_STRLEN_P(tmpzval) > 0) { if (!(have_header & HTTP_HEADER_CONTENT_LENGTH)) { @@ -913,9 +915,16 @@ static php_stream *php_stream_url_wrap_http_ex(php_stream_wrapper *wrapper, CHECK_FOR_CNTRL_CHARS(resource->pass); CHECK_FOR_CNTRL_CHARS(resource->path); } + 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. + * To keep consistency between POST and PATCH requests, we'll also not change the request method from PATCH to GET, even though it's allowed it's not mandated by the RFC. */ + new_flags |= HTTP_WRAPPER_KEEP_METHOD; + } stream = php_stream_url_wrap_http_ex( wrapper, new_path, mode, options, opened_path, context, - --redirect_max, HTTP_WRAPPER_REDIRECTED, response_header STREAMS_CC); + --redirect_max, new_flags, response_header STREAMS_CC); } else { php_stream_wrapper_log_error(wrapper, options, "HTTP request failed! %s", tmp_line); } diff --git a/ext/standard/tests/http/bug67430.phpt b/ext/standard/tests/http/bug67430.phpt index e72e419fc02a..1a515537e660 100644 --- a/ext/standard/tests/http/bug67430.phpt +++ b/ext/standard/tests/http/bug67430.phpt @@ -41,7 +41,7 @@ POST / HTTP/1.1 Host: %s:%d Connection: close -GET /foo HTTP/1.1 +POST /foo HTTP/1.1 Host: %s:%d Connection: close diff --git a/ext/standard/tests/http/gh11274.phpt b/ext/standard/tests/http/gh11274.phpt new file mode 100644 index 000000000000..fc125bfc494c --- /dev/null +++ b/ext/standard/tests/http/gh11274.phpt @@ -0,0 +1,62 @@ +--TEST-- +GH-11274 (POST/PATCH request via file_get_contents + stream_context_create switches to GET after a HTTP 308 redirect) +--INI-- +allow_url_fopen=1 +--CONFLICTS-- +server +--FILE-- + ['method' => 'POST', 'header' => 'Content-type: application/x-www-form-urlencoded', 'content' => http_build_query(['hello' => 'world'])]])); + echo file_get_contents("http://" . PHP_CLI_SERVER_ADDRESS . "/test$suffix", false, stream_context_create(['http' => ['method' => 'PATCH', 'header' => 'Content-type: application/x-www-form-urlencoded', 'content' => http_build_query(['hello' => 'world'])]])); + echo file_get_contents("http://" . PHP_CLI_SERVER_ADDRESS . "/test/$suffix", false, stream_context_create(['http' => ['method' => 'POST', 'header' => 'Content-type: application/x-www-form-urlencoded', 'content' => http_build_query(['hello' => 'world'])]])); + echo file_get_contents("http://" . PHP_CLI_SERVER_ADDRESS . "/test/$suffix", false, stream_context_create(['http' => ['method' => 'PATCH', 'header' => 'Content-type: application/x-www-form-urlencoded', 'content' => http_build_query(['hello' => 'world'])]])); +} +?> +--EXPECT-- +-- Testing unredirected request -- +method: POST; body: hello=world +method: PATCH; body: hello=world +method: POST; body: hello=world +method: PATCH; body: hello=world +-- Testing redirect status code 301 -- +method: GET; body: +method: GET; body: +method: GET; body: +method: GET; body: +-- Testing redirect status code 302 -- +method: GET; body: +method: GET; body: +method: GET; body: +method: GET; body: +-- Testing redirect status code 307 -- +method: POST; body: hello=world +method: PATCH; body: hello=world +method: POST; body: hello=world +method: PATCH; body: hello=world +-- Testing redirect status code 308 -- +method: POST; body: hello=world +method: PATCH; body: hello=world +method: POST; body: hello=world +method: PATCH; body: hello=world