don't chunk responses to HTTP/1.0 clients, even if they send Connection: Keep-Alive...
authorMichaeljohn Clement <inimino@inimino.org>
Wed, 23 Dec 2009 20:48:14 +0000 (15:48 -0500)
committerRyan Dahl <ry@tinyclouds.org>
Wed, 23 Dec 2009 21:24:29 +0000 (22:24 +0100)
lib/http.js
test/mjsunit/test-http-wget.js [new file with mode: 0644]

index dfb59bb447492a5293a235d12cd91cca8067e3cf..abb472e7e78a62ff8879b0cda8511c2d62ade4bb 100644 (file)
@@ -180,15 +180,15 @@ OutgoingMessage.prototype.sendHeaderLines = function (first_line, headers) {
 
     message_header += field + ": " + value + CRLF;
 
-    if (connection_expression.exec(field)) {
+    if (connection_expression.test(field)) {
       sent_connection_header = true;
-      if (close_expression.exec(value)) this.closeOnFinish = true;
+      if (close_expression.test(value)) this.closeOnFinish = true;
 
-    } else if (transfer_encoding_expression.exec(field)) {
+    } else if (transfer_encoding_expression.test(field)) {
       sent_transfer_encoding_header = true;
-      if (chunk_expression.exec(value)) this.chunked_encoding = true;
+      if (chunk_expression.test(value)) this.chunked_encoding = true;
 
-    } else if (content_length_expression.exec(field)) {
+    } else if (content_length_expression.test(field)) {
       sent_content_length_header = true;
 
     }
@@ -196,7 +196,8 @@ OutgoingMessage.prototype.sendHeaderLines = function (first_line, headers) {
 
   // keep-alive logic
   if (sent_connection_header == false) {
-    if (this.should_keep_alive) {
+    if (this.should_keep_alive &&
+        (sent_content_length_header || this.use_chunked_encoding_by_default)) {
       message_header += "Connection: keep-alive\r\n";
     } else {
       this.closeOnFinish = true;
@@ -209,6 +210,9 @@ OutgoingMessage.prototype.sendHeaderLines = function (first_line, headers) {
       message_header += "Transfer-Encoding: chunked\r\n";
       this.chunked_encoding = true;
     }
+    else {
+      this.closeOnFinish = true;
+    }
   }
 
   message_header += CRLF;
@@ -248,9 +252,6 @@ OutgoingMessage.prototype.finish = function () {
 function ServerResponse (req) {
   OutgoingMessage.call(this);
 
-  this.should_keep_alive = true;
-  this.use_chunked_encoding_by_default = true;
-
   if (req.httpVersionMajor < 1 || req.httpVersionMinor < 1) {
     this.use_chunked_encoding_by_default = false;
     this.should_keep_alive = false;
diff --git a/test/mjsunit/test-http-wget.js b/test/mjsunit/test-http-wget.js
new file mode 100644 (file)
index 0000000..3073c1d
--- /dev/null
@@ -0,0 +1,65 @@
+process.mixin(require("./common"));
+tcp = require("tcp");
+http = require("http");
+
+// wget sends an HTTP/1.0 request with Connection: Keep-Alive
+//
+// Sending back a chunked response to an HTTP/1.0 client would be wrong, 
+// so what has to happen in this case is that the connection is closed 
+// by the server after the entity body if the Content-Length was not 
+// sent.
+//
+// If the Content-Length was sent, we can probably safely honor the 
+// keep-alive request, even though HTTP 1.0 doesn't say that the 
+// connection can be kept open.  Presumably any client sending this 
+// header knows that it is extending HTTP/1.0 and can handle the 
+// response.  We don't test that here however, just that if the 
+// content-length is not provided, that the connection is in fact 
+// closed.
+
+var port = 7333;
+
+var server_response = "";
+var client_got_eof = false;
+var connection_was_closed = false;
+
+var server = http.createServer(function (req, res) {
+  res.sendHeader(200, {"Content-Type": "text/plain"});
+  res.sendBody("hello ");
+  res.sendBody("world\n");
+  res.finish();
+})
+server.listen(port);
+
+var c = tcp.createConnection(port);
+
+c.setEncoding("utf8");
+
+c.addListener("connect", function () {
+  c.send( "GET / HTTP/1.0\r\n" +
+          "Connection: Keep-Alive\r\n\r\n");
+});
+
+c.addListener("receive", function (chunk) {
+  puts(chunk);
+  server_response += chunk;
+});
+
+c.addListener("eof", function () {
+  client_got_eof = true;
+  puts('got eof');
+  c.close();
+});
+
+c.addListener("close", function () {
+  connection_was_closed = true;
+  puts('got close');
+  server.close();
+});
+
+process.addListener("exit", function () {
+  var m = server_response.split("\r\n\r\n");
+  assert.equal(m[1], "hello world\n");
+  assert.ok(client_got_eof);
+  assert.ok(connection_was_closed);
+});