Fixing the 1 byte off error in http.Server's upgradeHead

miksago · ry · commit aee226b2434b · 2010-05-01T18:05:53.000-07:00
(And fix up some style issues)
diff --git a/lib/http.js b/lib/http.js
@@ -519,11 +519,11 @@ function flushMessageQueue (socket, queue) {
 
 function Server (requestListener) {
   net.Server.call(this);
-  
+
   if(requestListener){
     this.addListener("request", requestListener);
   }
-  
+
   this.addListener("connection", connectionListener);
 }
 sys.inherits(Server, net.Server);
@@ -562,11 +562,15 @@ function connectionListener (socket) {
     if (parser.incoming && parser.incoming.upgrade) {
       socket.ondata = null;
       socket.onend = null;
-      
+
       var req = parser.incoming;
-      req.upgradeHead = d.slice(start + bytesParsed, end);
+
+      // This is start + byteParsed + 1 due to the error of getting \n
+      // in the upgradeHead from the closing lines of the headers
+      var upgradeHead = d.slice(start + bytesParsed + 1, end);
+
       if(self.listeners("upgrade").length > 0) {
-        self.emit('upgrade', req, req.socket, req.upgradeHead);
+        self.emit('upgrade', req, req.socket, upgradeHead);
       } else {
         socket.end();
       }
diff --git a/test/simple/test-http-upgrade.js b/test/simple/test-http-upgrade.js
@@ -7,6 +7,7 @@ var http = require("http");
 
 var requests_recv = 0;
 var requests_sent = 0;
+var request_upgradeHead = null;
 
 function createTestServer(){
   return new testServer();
@@ -15,25 +16,26 @@ function createTestServer(){
 function testServer(){
   var server = this;
   http.Server.call(server, function(){});
-  
+
   server.addListener("connection", function(){
     requests_recv++;
   });
-  
+
   server.addListener("request", function(req, res){
     res.writeHead(200, {"Content-Type": "text/plain"});
     res.write("okay");
     res.end();
   });
-  
+
   server.addListener("upgrade", function(req, socket, upgradeHead){
-    socket.write([
-      "HTTP/1.1 101 Web Socket Protocol Handshake",
-      "Upgrade: WebSocket",
-      "Connection: Upgrade",
-      "\r\n"
-    ].join("\r\n"), "utf8");
-  
+    socket.write( "HTTP/1.1 101 Web Socket Protocol Handshake\r\n"
+                + "Upgrade: WebSocket\r\n"
+                + "Connection: Upgrade\r\n"
+                + "\r\n\r\n"
+                );
+
+    request_upgradeHead = upgradeHead;
+
     socket.ondata = function(d, start, end){
       var data = d.toString('utf8', start, end);
       if(data == "kill"){
@@ -66,23 +68,30 @@ function writeReq(socket, data, encoding){
 function test_upgrade_with_listener(_server){
   var conn = new testClient();
   var state = 0;
-  
+
   conn.addListener("connect", function () {
-    writeReq(conn, "GET / HTTP/1.1\r\nUpgrade: WebSocket\r\nConnection: Upgrade\r\n\r\n");
+    writeReq( conn
+            , "GET / HTTP/1.1\r\n"
+            + "Upgrade: WebSocket\r\n"
+            + "Connection: Upgrade\r\n"
+            + "\r\n"
+            + "WjN}|M(6"
+            );
   });
-  
+
   conn.addListener("data", function(data){
     state++;
-    
+
     if(state == 1){
       assert.equal("HTTP/1.1 101", data.substr(0, 12));
+      assert.equal("WjN}|M(6", request_upgradeHead.toString("utf8"));
       conn.write("test", "utf8");
     } else if(state == 2) {
       assert.equal("test", data);
       conn.write("kill", "utf8");
     }
   });
-  
+
   conn.addListener("end", function(){
     assert.equal(2, state);
     conn.end();
@@ -98,7 +107,7 @@ var test_upgrade_no_listener_ended = false;
 
 function test_upgrade_no_listener(){
   var conn = new testClient();
-  
+
   conn.addListener("connect", function () {
     writeReq(conn, "GET / HTTP/1.1\r\nUpgrade: WebSocket\r\nConnection: Upgrade\r\n\r\n");
   });
@@ -107,7 +116,7 @@ function test_upgrade_no_listener(){
     test_upgrade_no_listener_ended = true;
     conn.end();
   });
-  
+
   conn.addListener("close", function(){
     test_standard_http();
   });
@@ -121,12 +130,12 @@ function test_standard_http(){
   conn.addListener("connect", function () {
     writeReq(conn, "GET / HTTP/1.1\r\n\r\n");
   });
-  
+
   conn.addListener("data", function(data){
     assert.equal("HTTP/1.1 200", data.substr(0, 12));
     conn.end();
   });
-  
+
   conn.addListener("close", function(){
     server.close();
   });
@@ -138,6 +147,7 @@ server.addListener("listening", function(){
   // All tests get chained after this:
   test_upgrade_with_listener(server);
 });
+
 server.listen(PORT);
 
 
@@ -148,4 +158,4 @@ process.addListener("exit", function () {
   assert.equal(3, requests_recv);
   assert.equal(3, requests_sent);
   assert.ok(test_upgrade_no_listener_ended);
-});
+});
