Skip to content

Fix #70361: HTTP stream wrapper doesn't close keep-alive connections #1490

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
kelunik wants to merge 2 commits into from
Closed

Fix #70361: HTTP stream wrapper doesn't close keep-alive connections #1490

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ee1b6e7
Fix #70361: HTTP stream wrapper doesn't close keep-alive connections
kelunik Aug 25, 2015
1aab6f4
Add NEWS entry, fix HTTP tests
kelunik Aug 25, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions NEWS
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,9 @@ PHP NEWS
- Reflection:
. Fixed bug causing bogus traces for ReflectionGenerator::getTrace(). (Bob)

- Streams:
. Fixed bug #70361 (HTTP stream wrapper doesn't close keep-alive connections) (Niklas Keller)

20 Aug 2015, PHP 7.0.0 RC 1

- Core:
Expand Down
14 changes: 7 additions & 7 deletions ext/standard/http_fopen_wrapper.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -579,13 +579,13 @@ php_stream *php_stream_url_wrap_http_ex(php_stream_wrapper *wrapper,
}
}

/* Send a Connection: close header when using HTTP 1.1 or later to avoid
* hanging when the server interprets the RFC literally and establishes a
* keep-alive connection, unless the user specifically requests something
* else by specifying a Connection header in the context options. */
if (protocol_version &&
((have_header & HTTP_HEADER_CONNECTION) == 0) &&
(strncmp(protocol_version, "1.0", MIN(protocol_version_len, 3)) > 0)) {
/* Send a Connection: close header to avoid hanging when the server
* interprets the RFC literally and establishes a keep-alive connection,
* unless the user specifically requests something else by specifying a
* Connection header in the context options. Send that header even for
* HTTP/1.0 to avoid issues when the server respond with a HTTP/1.1
* keep-alive response, which is the preferred response type. */
if ((have_header & HTTP_HEADER_CONNECTION) == 0) {
php_stream_write_string(stream, "Connection: close\r\n");
}

Expand Down
26 changes: 19 additions & 7 deletions ext/standard/tests/http/bug38802.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,44 +86,52 @@ array(7) {
string(15) "HTTP/1.0 200 OK"
}
string(5) "done."
string(195) "GET /foo/bar HTTP/1.0
string(%d) "GET /foo/bar HTTP/1.0
Host: 127.0.0.1:12342
Connection: close

GET /foo/bar2 HTTP/1.0
Host: 127.0.0.1:12342
Connection: close

GET /foo/bar3 HTTP/1.0
Host: 127.0.0.1:12342
Connection: close

GET /foo/bar4 HTTP/1.0
Host: 127.0.0.1:12342
Connection: close

"
-- Test: fail after 2 redirections --

Warning: fopen(http://127.0.0.1:12342/foo/bar): failed to open stream: Redirection limit reached, aborting in %s
bool(false)
string(97) "GET /foo/bar HTTP/1.0
string(%d) "GET /foo/bar HTTP/1.0
Host: 127.0.0.1:12342
Connection: close

GET /foo/bar2 HTTP/1.0
Host: 127.0.0.1:12342
Connection: close

"
-- Test: fail at first redirection --

Warning: fopen(http://127.0.0.1:12342/foo/bar): failed to open stream: Redirection limit reached, aborting in %s
bool(false)
string(48) "GET /foo/bar HTTP/1.0
string(%d) "GET /foo/bar HTTP/1.0
Host: 127.0.0.1:12342
Connection: close

"
-- Test: fail at first redirection (2) --

Warning: fopen(http://127.0.0.1:12342/foo/bar): failed to open stream: Redirection limit reached, aborting in %s
bool(false)
string(48) "GET /foo/bar HTTP/1.0
string(%d) "GET /foo/bar HTTP/1.0
Host: 127.0.0.1:12342
Connection: close

"
-- Test: return at first redirection --
Expand All @@ -135,8 +143,9 @@ array(2) {
string(41) "Location: http://127.0.0.1:12342/foo/bar2"
}
string(1) "1"
string(48) "GET /foo/bar HTTP/1.0
string(%d) "GET /foo/bar HTTP/1.0
Host: 127.0.0.1:12342
Connection: close

"
-- Test: return at first redirection (2) --
Expand All @@ -148,8 +157,9 @@ array(2) {
string(41) "Location: http://127.0.0.1:12342/foo/bar2"
}
string(1) "1"
string(48) "GET /foo/bar HTTP/1.0
string(%d) "GET /foo/bar HTTP/1.0
Host: 127.0.0.1:12342
Connection: close

"
-- Test: return at second redirection --
Expand All @@ -165,10 +175,12 @@ array(4) {
string(41) "Location: http://127.0.0.1:12342/foo/bar3"
}
string(0) ""
string(97) "GET /foo/bar HTTP/1.0
string(%d) "GET /foo/bar HTTP/1.0
Host: 127.0.0.1:12342
Connection: close

GET /foo/bar2 HTTP/1.0
Host: 127.0.0.1:12342
Connection: close

"
8 changes: 5 additions & 3 deletions ext/standard/tests/http/bug48929.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,18 +39,20 @@ echo "-- Test: requests with 'header' as string --\n";
do_test(array('header' => "X-Foo: bar\r\nContent-Type: text/plain", 'method' => 'POST', 'content' => 'ohai'));

?>
--EXPECT--
--EXPECTF--
-- Test: requests with 'header' as array --
string(103) "POST / HTTP/1.0
string(%d) "POST / HTTP/1.0
Host: 127.0.0.1:12342
Connection: close
Content-Length: 4
X-Foo: bar
Content-Type: text/plain

ohai"
-- Test: requests with 'header' as string --
string(103) "POST / HTTP/1.0
string(%d) "POST / HTTP/1.0
Host: 127.0.0.1:12342
Connection: close
Content-Length: 4
X-Foo: bar
Content-Type: text/plain
Expand Down
9 changes: 5 additions & 4 deletions ext/standard/tests/http/bug53198.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,17 +41,18 @@ ini_set('from', '[email protected]');
do_test();

?>
--EXPECT--
--EXPECTF--
-- Test: leave default --
string(63) "GET / HTTP/1.0
string(%d) "GET / HTTP/1.0
From: [email protected]
Host: 127.0.0.1:12342
Connection: close

"
-- Test: after ini_set --
string(62) "GET / HTTP/1.0
string(%d) "GET / HTTP/1.0
From: [email protected]
Host: 127.0.0.1:12342
Connection: close

"

13 changes: 12 additions & 1 deletion ext/standard/tests/http/bug61548.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,75 +44,86 @@ Done
--EXPECT--
POST / HTTP/1.0
Host: 127.0.0.1:12342
Connection: close
First:1
Second:2
Content-type: text/plain

GET /foo HTTP/1.0
Host: 127.0.0.1:12342
Connection: close
First:1
Second:2


POST / HTTP/1.0
Host: 127.0.0.1:12342
Connection: close
First:1
Second:2
Content-type: text/plain

GET /foo HTTP/1.0
Host: 127.0.0.1:12342
Connection: close
First:1
Second:2


POST / HTTP/1.0
Host: 127.0.0.1:12342
Connection: close
First:1
Second:2
Content-type: text/plain
Third:

GET /foo HTTP/1.0
Host: 127.0.0.1:12342
Connection: close
First:1
Second:2
Third:

POST / HTTP/1.0
Host: 127.0.0.1:12342
Connection: close
First:1
Content-type:text/plain
Second:2

GET /foo HTTP/1.0
Host: 127.0.0.1:12342
Connection: close
First:1
Second:2

POST / HTTP/1.0
Host: 127.0.0.1:12342
Connection: close
First:1
Content-type:text/plain
Second:2

GET /foo HTTP/1.0
Host: 127.0.0.1:12342
Connection: close
First:1
Second:2

POST / HTTP/1.0
Host: 127.0.0.1:12342
Connection: close
First:1
Content-type:text/plain
Second:2
Third:

GET /foo HTTP/1.0
Host: 127.0.0.1:12342
Connection: close
First:1
Second:2
Third:

Done

2 changes: 1 addition & 1 deletion ext/standard/tests/http/bug65634.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ do_test('1.1', 'keep-alive');
HTTP/1.0, default behaviour:
GET / HTTP/1.0
Host: 127.0.0.1:12342
Connection: close

HTTP/1.0, connection: close:
GET / HTTP/1.0
Expand All @@ -78,4 +79,3 @@ HTTP/1.1, connection: keep-alive:
GET / HTTP/1.1
Host: 127.0.0.1:12342
Connection: keep-alive

3 changes: 3 additions & 0 deletions ext/standard/tests/http/bug67430.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,11 +39,14 @@ Done
--EXPECT--
POST / HTTP/1.0
Host: 127.0.0.1:12342
Connection: close

GET /foo HTTP/1.0
Host: 127.0.0.1:12342
Connection: close

POST / HTTP/1.0
Host: 127.0.0.1:12342
Connection: close

Done
20 changes: 13 additions & 7 deletions ext/standard/tests/http/ignore_errors.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ function do_test($context_options) {
if ($fd) {
$meta_data = stream_get_meta_data($fd);
var_dump($meta_data['wrapper_data']);

var_dump(stream_get_contents($fd));
}

Expand Down Expand Up @@ -62,16 +62,18 @@ array(2) {
string(10) "X-Foo: bar"
}
string(1) "1"
string(48) "GET /foo/bar HTTP/1.0
string(%d) "GET /foo/bar HTTP/1.0
Host: 127.0.0.1:12342
Connection: close

"

Warning: fopen(http://127.0.0.1:12342/foo/bar): failed to open stream: HTTP request failed! HTTP/1.0 404 Not found
in %s on line %d
bool(false)
string(48) "GET /foo/bar HTTP/1.0
string(%d) "GET /foo/bar HTTP/1.0
Host: 127.0.0.1:12342
Connection: close

"
-- Test: requests with ignore_errors --
Expand All @@ -83,8 +85,9 @@ array(2) {
string(10) "X-Foo: bar"
}
string(1) "1"
string(48) "GET /foo/bar HTTP/1.0
string(%d) "GET /foo/bar HTTP/1.0
Host: 127.0.0.1:12342
Connection: close

"
resource(%d) of type (stream)
Expand All @@ -95,8 +98,9 @@ array(2) {
string(10) "X-bar: baz"
}
string(1) "2"
string(48) "GET /foo/bar HTTP/1.0
string(%d) "GET /foo/bar HTTP/1.0
Host: 127.0.0.1:12342
Connection: close

"
-- Test: requests with ignore_errors (2) --
Expand All @@ -108,8 +112,9 @@ array(2) {
string(10) "X-Foo: bar"
}
string(1) "1"
string(48) "GET /foo/bar HTTP/1.0
string(%d) "GET /foo/bar HTTP/1.0
Host: 127.0.0.1:12342
Connection: close

"
resource(%d) of type (stream)
Expand All @@ -120,7 +125,8 @@ array(2) {
string(10) "X-bar: baz"
}
string(1) "2"
string(48) "GET /foo/bar HTTP/1.0
string(%d) "GET /foo/bar HTTP/1.0
Host: 127.0.0.1:12342
Connection: close

"