Skip to content

Commit 8983a38

Browse files
committed
Request non-keep-alive connections by default in HTTP 1.1 requests.
As noted in FR #65634, at present we don't send a Connection request header when the protocol version is set to 1.1, which means that RFC-compliant Web servers should respond with keep-alive connections. Since there's no way of reusing the HTTP connection at present, this simply means that PHP will appear to hang until the remote server hits its connection timeout, which may be quite some time. This commit sends a "Connection: close" header by default when HTTP 1.1 (or later) is requested by the user via the context options. It can be overridden by specifying a Connection header in the context options. It isn't possible to disable sending of the Connection header, but given "Connection: keep-alive" is the same as the default HTTP 1.1 behaviour, I don't see this as a significant issue — users who want to opt in for that still can. As a note, although I've removed an efree(protocol_version), this doesn't result in a memory leak: protocol_version is freed in the out: block at the end of the function anyway, and there are no returns between the removed efree() and the later call. Yes, I ran the tests with valgrind to check that. ☺ Implements FR #65634 (HTTP wrapper is very slow with protocol_version 1.1).
1 parent fba290c commit 8983a38

File tree

4 files changed

+105
-2
lines changed

4 files changed

+105
-2
lines changed

NEWS

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,4 +44,8 @@ PHP NEWS
4444
improvements based on this.
4545
(RFC: https://wiki.php.net/rfc/operator_overloading_gmp). (Nikita)
4646

47+
- Standard:
48+
. Implemented FR #65634 (HTTP wrapper is very slow with protocol_version
49+
1.1). (Adam)
50+
4751
<<< NOTE: Insert NEWS from last stable release here prior to actual release! >>>

UPGRADING

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,3 +98,7 @@ PHP X.Y UPGRADE NOTES
9898

9999
- File upload:
100100
Uploads equal or greater than 2GB in size are now accepted.
101+
102+
- HTTP stream wrapper:
103+
HTTP 1.1 requests now include a Connection: close header unless explicitly
104+
overridden by setting a Connection header via the header context option.

ext/standard/http_fopen_wrapper.c

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,7 @@
8080
#define HTTP_HEADER_FROM 8
8181
#define HTTP_HEADER_CONTENT_LENGTH 16
8282
#define HTTP_HEADER_TYPE 32
83+
#define HTTP_HEADER_CONNECTION 64
8384

8485
#define HTTP_WRAPPER_HEADER_INIT 1
8586
#define HTTP_WRAPPER_REDIRECTED 2
@@ -386,8 +387,6 @@ php_stream *php_stream_url_wrap_http_ex(php_stream_wrapper *wrapper, const char
386387
strlcat(scratch, " HTTP/", scratch_len);
387388
strlcat(scratch, protocol_version, scratch_len);
388389
strlcat(scratch, "\r\n", scratch_len);
389-
efree(protocol_version);
390-
protocol_version = NULL;
391390
} else {
392391
strlcat(scratch, " HTTP/1.0\r\n", scratch_len);
393392
}
@@ -490,6 +489,11 @@ php_stream *php_stream_url_wrap_http_ex(php_stream_wrapper *wrapper, const char
490489
*(s-1) == '\t' || *(s-1) == ' ')) {
491490
have_header |= HTTP_HEADER_TYPE;
492491
}
492+
if ((s = strstr(tmp, "connection:")) &&
493+
(s == tmp || *(s-1) == '\r' || *(s-1) == '\n' ||
494+
*(s-1) == '\t' || *(s-1) == ' ')) {
495+
have_header |= HTTP_HEADER_CONNECTION;
496+
}
493497
/* remove Proxy-Authorization header */
494498
if (use_proxy && use_ssl && (s = strstr(tmp, "proxy-authorization:")) &&
495499
(s == tmp || *(s-1) == '\r' || *(s-1) == '\n' ||
@@ -563,6 +567,16 @@ php_stream *php_stream_url_wrap_http_ex(php_stream_wrapper *wrapper, const char
563567
}
564568
}
565569

570+
/* Send a Connection: close header when using HTTP 1.1 or later to avoid
571+
* hanging when the server interprets the RFC literally and establishes a
572+
* keep-alive connection, unless the user specifically requests something
573+
* else by specifying a Connection header in the context options. */
574+
if (protocol_version &&
575+
((have_header & HTTP_HEADER_CONNECTION) == 0) &&
576+
(strncmp(protocol_version, "1.0", MIN(protocol_version_len, 3)) > 0)) {
577+
php_stream_write_string(stream, "Connection: close\r\n");
578+
}
579+
566580
if (context &&
567581
php_stream_context_get_option(context, "http", "user_agent", &ua_zval) == SUCCESS &&
568582
Z_TYPE_PP(ua_zval) == IS_STRING) {

ext/standard/tests/http/bug65634.phpt

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
--TEST--
2+
Bug #65634 (HTTP wrapper is very slow with protocol_version 1.1)
3+
--INI--
4+
allow_url_fopen=1
5+
--SKIPIF--
6+
<?php require 'server.inc'; http_server_skipif('tcp://127.0.0.1:12342'); ?>
7+
--FILE--
8+
<?php
9+
require 'server.inc';
10+
11+
function do_test($version, $connection) {
12+
$options = [
13+
'http' => [
14+
'protocol_version' => $version,
15+
],
16+
];
17+
18+
if ($connection) {
19+
$options['http']['header'] = "Connection: $connection";
20+
}
21+
22+
$ctx = stream_context_create($options);
23+
24+
$responses = ["data://text/plain,HTTP/$version 204 No Content\r\n\r\n"];
25+
$pid = http_server('tcp://127.0.0.1:12342', $responses, $output);
26+
27+
$fd = fopen('http://127.0.0.1:12342/', 'rb', false, $ctx);
28+
fseek($output, 0, SEEK_SET);
29+
echo stream_get_contents($output);
30+
31+
http_server_kill($pid);
32+
}
33+
34+
echo "HTTP/1.0, default behaviour:\n";
35+
do_test('1.0', null);
36+
37+
echo "HTTP/1.0, connection: close:\n";
38+
do_test('1.0', 'close');
39+
40+
echo "HTTP/1.0, connection: keep-alive:\n";
41+
do_test('1.0', 'keep-alive');
42+
43+
echo "HTTP/1.1, default behaviour:\n";
44+
do_test('1.1', null);
45+
46+
echo "HTTP/1.1, connection: close:\n";
47+
do_test('1.1', 'close');
48+
49+
echo "HTTP/1.1, connection: keep-alive:\n";
50+
do_test('1.1', 'keep-alive');
51+
?>
52+
--EXPECT--
53+
HTTP/1.0, default behaviour:
54+
GET / HTTP/1.0
55+
Host: 127.0.0.1:12342
56+
57+
HTTP/1.0, connection: close:
58+
GET / HTTP/1.0
59+
Host: 127.0.0.1:12342
60+
Connection: close
61+
62+
HTTP/1.0, connection: keep-alive:
63+
GET / HTTP/1.0
64+
Host: 127.0.0.1:12342
65+
Connection: keep-alive
66+
67+
HTTP/1.1, default behaviour:
68+
GET / HTTP/1.1
69+
Host: 127.0.0.1:12342
70+
Connection: close
71+
72+
HTTP/1.1, connection: close:
73+
GET / HTTP/1.1
74+
Host: 127.0.0.1:12342
75+
Connection: close
76+
77+
HTTP/1.1, connection: keep-alive:
78+
GET / HTTP/1.1
79+
Host: 127.0.0.1:12342
80+
Connection: keep-alive
81+

0 commit comments

Comments
 (0)