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.

Closes GH-11275.

Author: nielsdos

NEWS

@@ -46,6 +46,8 @@ PHP                                                                        NEWS
 - Standard:
   . Fixed bug GH-11138 (move_uploaded_file() emits open_basedir warning for
     source file). (ilutov)
+  . Fixed bug GH-11274 (POST/PATCH request switches to GET after a HTTP 308
+    redirect). (nielsdos)
 
 - Streams:
   . Fixed bug GH-10031 ([Stream] STREAM_NOTIFY_PROGRESS over HTTP emitted

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);
 		}

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
 

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--
+<?php
+$serverCode = <<<'CODE'
+$uri = $_SERVER['REQUEST_URI'];
+if (isset($_GET["desired_status"]) && $uri[strlen($uri) - 1] !== '/') {
+    $desired_status = (int) $_GET["desired_status"];
+    http_response_code($desired_status);
+    header("Location: $uri/");
+    exit;
+}
+
+echo "method: ", $_SERVER['REQUEST_METHOD'], "; body: ", file_get_contents('php://input'), "\n";
+CODE;
+
+include __DIR__."/../../../../sapi/cli/tests/php_cli_server.inc";
+php_cli_server_start($serverCode, null, []);
+
+foreach ([null, 301, 302, 307, 308] as $status) {
+    if (is_null($status)) {
+        echo "-- Testing unredirected request --\n";
+    } else {
+        echo "-- Testing redirect status code $status --\n";
+    }
+    $suffix = $status ? "?desired_status=$status" : "";
+    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'])]]));
+    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