Skip to content

Commit 45897f5

Browse files
ndosschendossche
committed
Merge branch 'PHP-8.1' into PHP-8.2
* PHP-8.1: Fix GH-11274: POST/PATCH request via file_get_contents + stream_context_create switches to GET after a HTTP 308 redirect
2 parents 3769505 + 1ede313 commit 45897f5

File tree

4 files changed

+80
-7
lines changed

4 files changed

+80
-7
lines changed

‎NEWS‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,8 @@ PHP NEWS
5757
- Standard:
5858
. Fixed bug GH-11138 (move_uploaded_file() emits open_basedir warning for
5959
source file). (ilutov)
60+
. Fixed bug GH-11274 (POST/PATCH request switches to GET after a HTTP 308
61+
redirect). (nielsdos)
6062

6163
- Streams:
6264
. Fixed bug GH-10031 ([Stream] STREAM_NOTIFY_PROGRESS over HTTP emitted

‎ext/standard/http_fopen_wrapper.c‎

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,7 @@
7979

8080
#define HTTP_WRAPPER_HEADER_INIT 1
8181
#define HTTP_WRAPPER_REDIRECTED 2
82+
#define HTTP_WRAPPER_KEEP_METHOD 4
8283

8384
static inline void strip_header(char *header_bag, char *lc_header_bag,
8485
const char *lc_header_name)
@@ -140,6 +141,7 @@ static php_stream *php_stream_url_wrap_http_ex(php_stream_wrapper *wrapper,
140141
char *user_headers = NULL;
141142
int header_init = ((flags & HTTP_WRAPPER_HEADER_INIT) != 0);
142143
int redirected = ((flags & HTTP_WRAPPER_REDIRECTED) != 0);
144+
int redirect_keep_method = ((flags & HTTP_WRAPPER_KEEP_METHOD) != 0);
143145
bool follow_location = 1;
144146
php_stream_filter *transfer_encoding = NULL;
145147
int response_code;
@@ -363,8 +365,8 @@ static php_stream *php_stream_url_wrap_http_ex(php_stream_wrapper *wrapper,
363365
if (context && (tmpzval = php_stream_context_get_option(context, "http", "method")) != NULL) {
364366
if (Z_TYPE_P(tmpzval) == IS_STRING && Z_STRLEN_P(tmpzval) > 0) {
365367
/* As per the RFC, automatically redirected requests MUST NOT use other methods than
366-
* GET and HEAD unless it can be confirmed by the user */
367-
if (!redirected
368+
* GET and HEAD unless it can be confirmed by the user. */
369+
if (!redirected || redirect_keep_method
368370
|| zend_string_equals_literal(Z_STR_P(tmpzval), "GET")
369371
|| zend_string_equals_literal(Z_STR_P(tmpzval), "HEAD")
370372
) {
@@ -458,7 +460,7 @@ static php_stream *php_stream_url_wrap_http_ex(php_stream_wrapper *wrapper,
458460
zend_str_tolower(ZSTR_VAL(tmp), ZSTR_LEN(tmp));
459461
t = ZSTR_VAL(tmp);
460462

461-
if (!header_init) {
463+
if (!header_init && !redirect_keep_method) {
462464
/* strip POST headers on redirect */
463465
strip_header(user_headers, t, "content-length:");
464466
strip_header(user_headers, t, "content-type:");
@@ -606,7 +608,7 @@ static php_stream *php_stream_url_wrap_http_ex(php_stream_wrapper *wrapper,
606608
* see bug #44603 for details. Since Content-Type maybe part of user's headers we need to do this check first.
607609
*/
608610
if (
609-
header_init &&
611+
(header_init || redirect_keep_method) &&
610612
context &&
611613
!(have_header & HTTP_HEADER_CONTENT_LENGTH) &&
612614
(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,
624626
}
625627

626628
/* Request content, such as for POST requests */
627-
if (header_init && context &&
629+
if ((header_init || redirect_keep_method) && context &&
628630
(tmpzval = php_stream_context_get_option(context, "http", "content")) != NULL &&
629631
Z_TYPE_P(tmpzval) == IS_STRING && Z_STRLEN_P(tmpzval) > 0) {
630632
if (!(have_header & HTTP_HEADER_CONTENT_LENGTH)) {
@@ -913,9 +915,16 @@ static php_stream *php_stream_url_wrap_http_ex(php_stream_wrapper *wrapper,
913915
CHECK_FOR_CNTRL_CHARS(resource->pass);
914916
CHECK_FOR_CNTRL_CHARS(resource->path);
915917
}
918+
int new_flags = HTTP_WRAPPER_REDIRECTED;
919+
if (response_code == 307 || response_code == 308) {
920+
/* RFC 7538 specifies that status code 308 does not allow changing the request method from POST to GET.
921+
* RFC 7231 does the same for status code 307.
922+
* 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. */
923+
new_flags |= HTTP_WRAPPER_KEEP_METHOD;
924+
}
916925
stream = php_stream_url_wrap_http_ex(
917926
wrapper, new_path, mode, options, opened_path, context,
918-
--redirect_max, HTTP_WRAPPER_REDIRECTED, response_header STREAMS_CC);
927+
--redirect_max, new_flags, response_header STREAMS_CC);
919928
} else {
920929
php_stream_wrapper_log_error(wrapper, options, "HTTP request failed! %s", tmp_line);
921930
}

‎ext/standard/tests/http/bug67430.phpt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ POST / HTTP/1.1
4141
Host: %s:%d
4242
Connection: close
4343

44-
GET /foo HTTP/1.1
44+
POST /foo HTTP/1.1
4545
Host: %s:%d
4646
Connection: close
4747

‎ext/standard/tests/http/gh11274.phpt‎

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
--TEST--
2+
GH-11274 (POST/PATCH request via file_get_contents + stream_context_create switches to GET after a HTTP 308 redirect)
3+
--INI--
4+
allow_url_fopen=1
5+
--CONFLICTS--
6+
server
7+
--FILE--
8+
<?php
9+
$serverCode = <<<'CODE'
10+
$uri = $_SERVER['REQUEST_URI'];
11+
if (isset($_GET["desired_status"]) && $uri[strlen($uri) - 1] !== '/') {
12+
$desired_status = (int) $_GET["desired_status"];
13+
http_response_code($desired_status);
14+
header("Location: $uri/");
15+
exit;
16+
}
17+
18+
echo "method: ", $_SERVER['REQUEST_METHOD'], "; body: ", file_get_contents('php://input'), "\n";
19+
CODE;
20+
21+
include __DIR__."/../../../../sapi/cli/tests/php_cli_server.inc";
22+
php_cli_server_start($serverCode, null, []);
23+
24+
foreach ([null, 301, 302, 307, 308] as $status) {
25+
if (is_null($status)) {
26+
echo "-- Testing unredirected request --\n";
27+
} else {
28+
echo "-- Testing redirect status code $status --\n";
29+
}
30+
$suffix = $status ? "?desired_status=$status" : "";
31+
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'])]]));
32+
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'])]]));
33+
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'])]]));
34+
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'])]]));
35+
}
36+
?>
37+
--EXPECT--
38+
-- Testing unredirected request --
39+
method: POST; body: hello=world
40+
method: PATCH; body: hello=world
41+
method: POST; body: hello=world
42+
method: PATCH; body: hello=world
43+
-- Testing redirect status code 301 --
44+
method: GET; body:
45+
method: GET; body:
46+
method: GET; body:
47+
method: GET; body:
48+
-- Testing redirect status code 302 --
49+
method: GET; body:
50+
method: GET; body:
51+
method: GET; body:
52+
method: GET; body:
53+
-- Testing redirect status code 307 --
54+
method: POST; body: hello=world
55+
method: PATCH; body: hello=world
56+
method: POST; body: hello=world
57+
method: PATCH; body: hello=world
58+
-- Testing redirect status code 308 --
59+
method: POST; body: hello=world
60+
method: PATCH; body: hello=world
61+
method: POST; body: hello=world
62+
method: PATCH; body: hello=world

0 commit comments

Comments
 (0)