Skip to content
This repository

Response object for HTTP Upgrade/CONNECT #3036

Open
wants to merge 3 commits into from

8 participants

Stéphan Kochen Bert Belder Koichi Kobayashi Tim Caswell Adam Malcontenti-Wilson Ben Noordhuis Isaac Z. Schlueter Nodejs Jenkins
Stéphan Kochen

I started a thread on nodejs-dev about this, but it looks like my posts are still in the moderator queue. Will link it here once it gets through.

Briefly:

  • The first commit is a test showing that a pipelined Upgrade request to a Node server can cause an out-of-order response. This is because the upgrade handler gets immediate access to the socket, while other requests may still be processing async.

  • The second commit fixes this by providing a response object even for Upgrade, that is also queued properly along with other responses.

  • In addition to fixing the test, I find it makes sense to have a response here. Per the spec, CONNECT and Upgrade must be followed by a proper response, before handover.

Regarding make test, I have one failing test both before and after this patch set, namely simple/test-http-full-response. I consider this a platform irk. make test-debug plain crashes the test suite on simple/test-repl (also before and after), but a fair amount of HTTP tests do pass.

Stéphan Kochen

I have a branch of einaros/ws on top of this, with working tests and examples:

https://github.com/stephank/ws/tree/upgrade-response

Stéphan Kochen

More branches of websocket projects:

These are all examples of projects reimplementing basic ServerResponse functionality.


I noticed a recurring pattern, along the lines of:

socket.on('data', dataListener);
dataListener(upgradeHead);

Perhaps we can simply emit upgradeHead as the first data event on the socket, after the switchProtocols callback finishes? (Or do we have to do this in a nextTick? I'm not sure if that guarantees ordering of events when new data arrives quickly.)

Stéphan Kochen

I went ahead and rebased on current master, and implemented the earlier thought of simply emitting upgradeHead as a data event.

The branches of ws and faye-websocket have also been updated. (Sockjs didn't need any further updates.)

Bert Belder
Collaborator

@isaacs Could you flush the moderation queue for nodejs-dev? (or give me / @bnoordhuis admin rights)

Koichi Kobayashi
Collaborator

I think that @stephank's the new API is really nice. However, it breaks backward-compatibility. Because 'upgrade' event is widely used (by Socket.IO, especially), I do not want to change it.

Instead, I think that Node should close the connection when pipelined Upgrade/CONNECT request was detected. Because:

  • If Upgrade/CONNECT request is idempotent, client can resend the request.
  • If Upgrade/CONNECT request is not idempotent, client should not pipeline the request.
Stéphan Kochen

Thanks for pitching in! That will, of course, solve the problem as well.

You're absolutely right when it comes to the API. I was still hoping we could move mountains by showing the API can be better, and demonstrating that I was willing to put work in projects currently using upgrade. The only reason I don't have a branch of Socket.IO is because I couldn't get its test suite running on Node 0.7.7.

Is there a minimum set of projects using upgrade that we want to ensure compatibility with? Would it help if we got their maintainers on board with this API change?

Koichi Kobayashi
Collaborator

@stephank - What WebSocket's client does pipeline the Upgrade request?

Stéphan Kochen

There isn't any I'm aware of, that test is theoretical. Though it does actually work and triggers the bad HTTP server behavior on 0.6.14 and 0.7.7.

I'm in this for the response object, which can be very useful in combination with middleware. What led me to doing this was trying to add upgrade support to Connect middleware. See the related pull request for Connect, which implements it using the current Node API.

Briefly, that pull works, but a lot of middleware really wants to manipulate the response, e.g. add cookies, other headers, sometimes even abort the request (ie. basicAuth wants to reply with 401).

Koichi Kobayashi
Collaborator

@stephank - Sorry, I'm -1 to the PR. Because there is no problem in the real world, and the API is marked Stable (i.e. Backwards-compatibility is guaranteed.).
Closing.

Koichi Kobayashi koichik closed this
Koichi Kobayashi
Collaborator
koichik commented

By the discussion in the ML, we will revisit in v0.9. Reopening.

Koichi Kobayashi koichik reopened this
Tim Caswell

I just found this issue. I've been wanting this feature for a long time! What can I do to help? Basically my goal is to be able to handle websockets 100% from a handler function without needing access to the server object. This makes composing layers (like connect middlewares) possible when using websockets.

Stéphan Kochen
  • This needs to be rebased on master. Renewed interest helps; I'll see about doing that this week. :)

  • On the mailing list, @theturtle32 raised the point of sending both the HTTP response and WebSocket handshake in a single frame. This pull request basically splits the write()s, which could be a problem. It looks like (in the tests) a single frame is sent because Nagle's is enabled by default, but it's likely that users will want to disable Nagle's as soon as they get the socket. We need figure out if this is a problem and, if so, how to fix it.

  • Other than that, I'm just waiting for feedback. :)

Stéphan Kochen

Rebased. Test results still good.

Stéphan Kochen

I tried to create a test case for the issue with Nagle's. I basically disable Nagle's directly after switchProtocols, then write the response body to the HTTP Upgrade. A plain TCP client then counts data events to check for segmentation.

https://gist.github.com/3426025

The test currently passes. (ie. I only get one data event.)

Can someone with a bit more knowledge of net / libuv internals verify if this is a proper test case? I'm not sure if simply counting data events is enough verification.

Adam Malcontenti-Wilson

Any progress on this proposal getting into one of 0.9.x unstable releases?

The lack of a response object is making it hard for frameworks to implement upgrade request routing easily, meaning that upgrades are typically handed manually with a lot more work required.

As to the issue of backwards-compatibility that everyone seems to be hanging on, even with this proposed API break, wouldn't the only change required be something like this or am I missing something?

server.on('upgrade', function(req, socket, head) {
  if(!(socket instanceof net.Socket))
    socket = req.connection;

  //... legacy code that uses socket directly ...
})
Ben Noordhuis

PR no longer applies. Is this still an issue with master and if so, why?

Stéphan Kochen

@adammw: Sure, but API compatibility means things have to stay working without code changes to the libraries/applications using the API.

@bnoordhuis: It is still an issue. This is a proposal to change the upgrade/connect event API to no longer receive plain sockets, but a Response object. The PR is an implementation.

I'll take a look at another rebase on current master.

Stéphan Kochen

Here we go, rebased against c6e958d. Some things don't sit quite right with me, though:

  • Both master and this branch pass tests on a release build on my Mac. Tests on a debug build fail, however, even on plain master, so I have no comparison there. (test-debug-brk-file hangs, tried running some others manually and got assertion failures.)

  • http.js still uses old-style streams and pretty much forces this on servers using upgrade requests. (I'm guessing this is well known.) Right now, I'm still pausing/resuming the socket and injecting the head buffer back in using socket._readableState.onread() after the switch callback had its chance to install handlers. I think eventually, the onread() should be done before the switch callback, so that the callback gets an immediately readable stream?

Stéphan Kochen

Rebased on current master (8e311d2). Now using stream.push(). I succeeded in running make test-debug on my Mac this time, with the exact same results on master and this branch. (Several seemingly unrelated failures.)

Remaining points:

  • Waiting on new-style streams in http.js. (issue #4488)
  • Would still appreciate comments on separate write()s, as per this earlier comment.
Stéphan Kochen

Right, with #4488 closed, I rebased on master (926c90b) again. The socket.pause() and socket.resume() calls are gone. I now also socket.push() the upgradeHead before the switchProtocols() callback. Test results are good.

Remaining:

Tim Caswell

Should @stephank keep working on this? I'd hate to see him do all this work if there is no intention to accept it. This is a feature I would really like, but it is a change to the HTTP API which is stable now.

@stephank, is your change backwards incompatible with any existing node code? And if so, how significant is the change?

My approach of late has been to replace the http module entirely in userspace. (see web in npm).

Stéphan Kochen

@creationix Yes, it's an API change to the upgrade and connect event callbacks.

It's the difference between this server we have right now:

server.on('upgrade', function(req, sock, head) {
    sock.write('HTTP/1.1 101 Switching Protocol\r\n' +
               'Connection: Upgrade\r\n' +
               'Upgrade: Echo\r\n\r\n');

    sock.push(head);
    sock.on('readable', function() {
        var chunk = sock.read();
        sock.write(chunk);
    });
});

And this server on my branch:

server.on('upgrade', function(req, res) {
    res.writeHead(101, {
        'Connection': 'Upgrade',
        'Upgrade': 'Echo'
    });

    res.switchProtocols(function(sock) {
        sock.on('readable', function() {
            var chunk = sock.read();
            sock.write(chunk);
        });
    });
});

A server that handles both is slightly finicky, though:

server.on('upgrade', function(req, resOrSock, head) {
    var headers = {
        'Connection': 'Upgrade',
        'Upgrade': 'Echo'
    };

    if (resOrSock.switchProtocols) {
        resOrSock.writeHead(101, headers);
        resOrSock.switchProtocols(setupSock);
    }
    else {
        resOrSock.write('HTTP/1.1 101 Switching Protocol\r\n' +
                        Object.keys(headers).map(function(name) {
                            return name + ': ' + headers[name] + '\r\n';
                        }).join('') +
                        '\r\n');
        resOrSock.push(head);
        setupSock(resOrSock);
    }

    function setupSock(sock) {
        sock.on('readable', function() {
            var chunk = sock.read();
            sock.write(chunk);
        });
    }
});
Stéphan Kochen

There's now a package upgrade-ex in NPM that implements this stand-alone on top of Node.js 0.9.6.

@adammw suggested I look into backwards compatibility, but this is kind of the reverse. I can transform an 'old style' upgrade into a 'new-style' this way, but I've not yet figured out how to make 'old style' work on top of 'new style'.

Isaac Z. Schlueter
Collaborator

@stephank So, it seems like the main blocker is that now you do on('upgrade', function(req, socket, head) {}) and with the patch, you'd do on('upgrade', function(req, res) {}). Why not making the new signature on('upgrade', function(req, socket, head, res) {})? It's a bit ugly, but would keep backwards compatibility. Changing event signatures is very destabilizing.

Adam Malcontenti-Wilson

Changing event signatures is very destabilizing.

That's worrying in of itself.

on('upgrade', function(req, socket, head, res) {}) is too ugly for my liking.

My suggestion is using different event name is used in place of upgrade (upgradeEx, upgrade-req, upgrade2, etc.) like is done in the update-ex npm package. Depending on which events have listeners depends on which behaviour is taken within the http module. And since they have different names, this also has the plus of being able to be detected which is being listened to, and allowing different signatures.
In the meantime, the upgrade event can still work as before but with a warning when a listener is attached telling the user/maintainer to upgrade their package to use the new event and event signature.

I really like this proposal, and think it works a lot better than the existing upgrade event behaviour, although I worry that now it is a npm module its inclusion into the nodejs core may be questioned, even though it is an improvement/replacement for existing functionality.

And considering the breakage and updates that will be needed from modifications of streams in 0.9/0.10, I think considering this sooner would be better than later.

Nodejs Jenkins
Collaborator

Can one of the admins verify this patch?

Adam Malcontenti-Wilson

Seems like there may now be some motivation to make some changes to upgrade, but in a rather different way at #4813.

Stéphan Kochen

The last push contains a rebase on master (46da8c2), and reworded commit messages.

I had to change test-http-upgrade-client2.js, because it was doing really strange things that I'm not sure are valid on current node versions any way.

Adam Malcontenti-Wilson adammw referenced this pull request in senchalabs/connect
Closed

A proposal for handling upgrade requests. #506

Stéphan Kochen

Rebase against current master (b255f4c).

I used to have a commit that removed upgradeHead on both the HTTP server and client, but master has since deprecated it and replaced it with an empty buffer for compatibility.

So the commit was no longer necessary, and I'm no longer touching the HTTP client code. I do, however, remove the empty buffer on the HTTP server, considering this is an API change any way.

I also removed the commit that fixed test-http-upgrade-client2.js, which also seems fixed in master already.

Ben Noordhuis

@stephank What benefit does this change have over the current approach?

Stéphan Kochen

Can't see why jenkins is failing, but I believe these are not failures related to the pull.

@bnoordhuis In short: middleware. See also #4813.

Ben Noordhuis

Right. @isaacs, this one's for you.

stephank and others added some commits
Stéphan Kochen stephank http: failing test for pipelined upgrade
There are no known real world clients that actually send an upgrade
request pipelined after a regular request. Nonetheless, this tests
conformance to the standard.
59c95ff
Stéphan Kochen stephank http: upgrade/connect with response object
Introduces `switchProtocols()` on the response, which is an alternate
`end()` for responses that hijack the socket.

The common part for building a ServerResponse, shared with regular
requests, is extracted to a `createResponse()` function.
22b95bc
Stéphan Kochen stephank http: update documentation 6b7afd6
Stéphan Kochen

Rebase against current master (b26d346). You guys are really good at breaking my stuff for years on end. :(

upgradeHead removal was apparently reverted again on master. I didn't add it back to the client.

Isaac Z. Schlueter
Collaborator

@stephank Sorry for the absurd delay on this. I will review this in more detail very soon (today or Monday).

The main blocker that I can see right off the bat is that we probably can't get away with changing the server.on('upgrade') signature in a backwards-incompatible way (hence the ugly compromise in #4813). That being said, the core pipelining-related bug here definitely needs to be fixed, and #4813 is important, so I think at least most of the work you've done will be relevant and useful (even just the test is really great).

Thanks again, and my apologies for letting it linger so long. This is next in my queue after the OutgoingMessage refactoring.

Stéphan Kochen

@isaacs Thanks for taking a look.

Note though, the test is more or less theoretical. It definitely triggers bad behaviour, but I'm not aware of this ever happening in practice, with real clients.

I'm curious how you'll end up tackling #4813. Middleware is the important issue for me here.

Stéphan Kochen

If my understanding is correct, there is no intention of merging this, and I have no intention of updating it any further.

Unless you guys want to keep this issue around for a hypothetical 2.0, you may as well close it.

Path ahead seems to be a separate module and getting people to use that. I might at some point in the future revive upgradeEx, but I'm currently otherwise occupied.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Showing 3 unique commits by 2 authors.

Aug 06, 2013
Stéphan Kochen stephank http: failing test for pipelined upgrade
There are no known real world clients that actually send an upgrade
request pipelined after a regular request. Nonetheless, this tests
conformance to the standard.
59c95ff
Stéphan Kochen stephank http: upgrade/connect with response object
Introduces `switchProtocols()` on the response, which is an alternate
`end()` for responses that hijack the socket.

The common part for building a ServerResponse, shared with regular
requests, is extracted to a `createResponse()` function.
22b95bc
Stéphan Kochen stephank http: update documentation 6b7afd6
This page is out of date. Refresh to see the latest.
84 doc/api/http.markdown
Source Rendered
@@ -93,39 +93,37 @@ not be emitted.
93 93
94 94 ### Event: 'connect'
95 95
96   -`function (request, socket, head) { }`
  96 +`function (request, response) { }`
97 97
98   -Emitted each time a client requests a http CONNECT method. If this event isn't
99   -listened for, then clients requesting a CONNECT method will have their
100   -connections closed.
  98 +Emitted each time a client requests a http CONNECT method. The arguments are
  99 +the same as for the request event. If this event isn't listened for, then
  100 +clients requesting a CONNECT method will have their connections closed.
101 101
102   -* `request` is the arguments for the http request, as it is in the request
103   - event.
104   -* `socket` is the network socket between the server and client.
105   -* `head` is an instance of Buffer, the first packet of the tunneling stream,
106   - this may be empty.
  102 +The server will treat a CONNECT method as the final request on a connection.
  103 +Completing the request with `response.end()` will send the response, before
  104 +finally closing the connection.
107 105
108   -After this event is emitted, the request's socket will not have a `data`
109   -event listener, meaning you will need to bind to it in order to handle data
110   -sent to the server on that socket.
  106 +To instead continue the connection, set the response parameters as usual, then
  107 +use [switchProtocols](#http_response_switchprotocols_callback) instead of
  108 +`response.end()`. This method will give away control of the socket after
  109 +headers have been sent.
111 110
112 111 ### Event: 'upgrade'
113 112
114   -`function (request, socket, head) { }`
  113 +`function (request, response) { }`
115 114
116   -Emitted each time a client requests a http upgrade. If this event isn't
117   -listened for, then clients requesting an upgrade will have their connections
118   -closed.
  115 +Emitted each time a client requests a http upgrade. The arguments are the same
  116 +as for the request event. If this event isn't listened for, then clients
  117 +requesting an upgrade will have their connections closed.
119 118
120   -* `request` is the arguments for the http request, as it is in the request
121   - event.
122   -* `socket` is the network socket between the server and client.
123   -* `head` is an instance of Buffer, the first packet of the upgraded stream,
124   - this may be empty.
  119 +The server will treat an upgrade request as the final request on a connection.
  120 +Completing the request with `response.end()` will send the response, before
  121 +finally closing the connection.
125 122
126   -After this event is emitted, the request's socket will not have a `data`
127   -event listener, meaning you will need to bind to it in order to handle data
128   -sent to the server on that socket.
  123 +To instead continue the connection, set the response parameters as usual, then
  124 +use [switchProtocols](#http_response_switchprotocols_callback) instead of
  125 +`response.end()`. This method will give away control of the socket after
  126 +headers have been sent.
129 127
130 128 ### Event: 'clientError'
131 129
@@ -395,6 +393,14 @@ If `data` is specified, it is equivalent to calling `response.write(data, encodi
395 393 followed by `response.end()`.
396 394
397 395
  396 +### response.switchProtocols(callback)
  397 +
  398 +This method is used instead of `response.end()` when successfully completing
  399 +a request with an upgrade or the CONNECT method. The callback receives the
  400 +socket as its only argument, which will resume sending and receiving after
  401 +the HTTP headers.
  402 +
  403 +
398 404 ## http.request(options, callback)
399 405
400 406 Node maintains several connections per server to make HTTP requests.
@@ -616,16 +622,15 @@ A client server pair that show you how to listen for the `connect` event.
616 622 res.writeHead(200, {'Content-Type': 'text/plain'});
617 623 res.end('okay');
618 624 });
619   - proxy.on('connect', function(req, cltSocket, head) {
  625 + proxy.on('connect', function(req, res) {
620 626 // connect to an origin server
621 627 var srvUrl = url.parse('http://' + req.url);
622 628 var srvSocket = net.connect(srvUrl.port, srvUrl.hostname, function() {
623   - cltSocket.write('HTTP/1.1 200 Connection Established\r\n' +
624   - 'Proxy-agent: Node-Proxy\r\n' +
625   - '\r\n');
626   - srvSocket.write(head);
627   - srvSocket.pipe(cltSocket);
628   - cltSocket.pipe(srvSocket);
  629 + res.writeHead(200, { 'Proxy-agent': 'Node-Proxy' });
  630 + res.switchProtocols(function(cltSocket) {
  631 + srvSocket.pipe(cltSocket);
  632 + cltSocket.pipe(srvSocket);
  633 + });
629 634 });
630 635 });
631 636
@@ -677,13 +682,14 @@ A client server pair that show you how to listen for the `upgrade` event.
677 682 res.writeHead(200, {'Content-Type': 'text/plain'});
678 683 res.end('okay');
679 684 });
680   - srv.on('upgrade', function(req, socket, head) {
681   - socket.write('HTTP/1.1 101 Web Socket Protocol Handshake\r\n' +
682   - 'Upgrade: WebSocket\r\n' +
683   - 'Connection: Upgrade\r\n' +
684   - '\r\n');
685   -
686   - socket.pipe(socket); // echo back
  685 + srv.on('upgrade', function(req, res) {
  686 + res.writeHead(101, {
  687 + 'Connection': 'Upgrade',
  688 + 'Upgrade': 'Echo'
  689 + });
  690 + res.switchProtocols(function(socket) {
  691 + socket.pipe(socket); // echo back
  692 + });
687 693 });
688 694
689 695 // now that server is running
@@ -695,7 +701,7 @@ A client server pair that show you how to listen for the `upgrade` event.
695 701 hostname: '127.0.0.1',
696 702 headers: {
697 703 'Connection': 'Upgrade',
698   - 'Upgrade': 'websocket'
  704 + 'Upgrade': 'Echo'
699 705 }
700 706 };
701 707
105 lib/_http_server.js
@@ -287,6 +287,42 @@ function connectionListener(socket) {
287 287 var outgoing = [];
288 288 var incoming = [];
289 289
  290 + function createResponse(req, shouldKeepAlive) {
  291 + incoming.push(req);
  292 +
  293 + var res = new ServerResponse(req);
  294 +
  295 + res.shouldKeepAlive = shouldKeepAlive;
  296 + DTRACE_HTTP_SERVER_REQUEST(req, socket);
  297 + COUNTER_HTTP_SERVER_REQUEST();
  298 +
  299 + if (socket._httpMessage) {
  300 + // There are already pending outgoing res, append.
  301 + outgoing.push(res);
  302 + } else {
  303 + res.assignSocket(socket);
  304 + }
  305 +
  306 + res.on('finish', function() {
  307 + // Usually the first incoming element should be our request. it may
  308 + // be that in the case abortIncoming() was called that the incoming
  309 + // array will be empty.
  310 + assert(incoming.length == 0 || incoming[0] === req);
  311 +
  312 + incoming.shift();
  313 +
  314 + // if the user never called req.read(), and didn't pipe() or
  315 + // .resume() or .on('data'), then we call req._dump() so that the
  316 + // bytes will be pulled off the wire.
  317 + if (!req._consuming)
  318 + req._dump();
  319 +
  320 + res.detachSocket(socket);
  321 + });
  322 +
  323 + return res;
  324 + }
  325 +
290 326 function abortIncoming() {
291 327 while (incoming.length) {
292 328 var req = incoming.shift();
@@ -360,14 +396,41 @@ function connectionListener(socket) {
360 396 freeParser(parser, req);
361 397
362 398 var eventName = req.method === 'CONNECT' ? 'connect' : 'upgrade';
363   - if (EventEmitter.listenerCount(self, eventName) > 0) {
364   - var bodyHead = d.slice(bytesParsed, d.length);
365   -
366   - self.emit(eventName, req, req.socket, bodyHead);
367   - } else {
  399 + if (EventEmitter.listenerCount(self, eventName) === 0) {
368 400 // Got upgrade header or CONNECT method, but have no handler.
369 401 socket.destroy();
  402 + return;
370 403 }
  404 +
  405 + // This is start + byteParsed
  406 + var bodyHead = d.slice(bytesParsed, d.length);
  407 + socket.unshift(bodyHead);
  408 +
  409 + // 'shouldKeepAlive == false' ensures we send 'Connection: close' for
  410 + // responses that don't successfully switch protocols.
  411 + var res = createResponse(req, false);
  412 + res.useChunkedEncodingByDefault = false;
  413 +
  414 + // The alternate exit for a handler that accepts the upgrade.
  415 + var switchCb = null;
  416 + res.switchProtocols = function(fn) {
  417 + req._consuming = true;
  418 + switchCb = fn;
  419 + this.end();
  420 + };
  421 +
  422 + // When finished, either upgrade, or close the socket.
  423 + res.on('finish', function() {
  424 + socket.removeListener('close', serverSocketCloseListener);
  425 +
  426 + if (switchCb) {
  427 + switchCb(socket);
  428 + } else {
  429 + socket.destroySoon();
  430 + }
  431 + });
  432 +
  433 + self.emit(eventName, req, res);
371 434 }
372 435 };
373 436
@@ -398,39 +461,11 @@ function connectionListener(socket) {
398 461 // new message. In this callback we setup the response object and pass it
399 462 // to the user.
400 463 parser.onIncoming = function(req, shouldKeepAlive) {
401   - incoming.push(req);
402   -
403   - var res = new ServerResponse(req);
404   -
405   - res.shouldKeepAlive = shouldKeepAlive;
406   - DTRACE_HTTP_SERVER_REQUEST(req, socket);
407   - COUNTER_HTTP_SERVER_REQUEST();
408   -
409   - if (socket._httpMessage) {
410   - // There are already pending outgoing res, append.
411   - outgoing.push(res);
412   - } else {
413   - res.assignSocket(socket);
414   - }
  464 + var res = createResponse(req, shouldKeepAlive);
415 465
416 466 // When we're finished writing the response, check if this is the last
417   - // respose, if so destroy the socket.
  467 + // response, if so destroy the socket.
418 468 res.on('finish', function() {
419   - // Usually the first incoming element should be our request. it may
420   - // be that in the case abortIncoming() was called that the incoming
421   - // array will be empty.
422   - assert(incoming.length == 0 || incoming[0] === req);
423   -
424   - incoming.shift();
425   -
426   - // if the user never called req.read(), and didn't pipe() or
427   - // .resume() or .on('data'), then we call req._dump() so that the
428   - // bytes will be pulled off the wire.
429   - if (!req._consuming)
430   - req._dump();
431   -
432   - res.detachSocket(socket);
433   -
434 469 if (res._last) {
435 470 socket.destroySoon();
436 471 } else {
12 test/simple/test-http-after-connect.js
@@ -37,13 +37,15 @@ var server = http.createServer(function(req, res) {
37 37 res.end(req.url);
38 38 }, 50);
39 39 });
40   -server.on('connect', function(req, socket, firstBodyChunk) {
  40 +server.on('connect', function(req, res) {
41 41 common.debug('Server got CONNECT request');
42 42 serverConnected = true;
43   - socket.write('HTTP/1.1 200 Connection established\r\n\r\n');
44   - socket.resume();
45   - socket.on('end', function() {
46   - socket.end();
  43 + res.writeHead(200, 'Connection established');
  44 + res.switchProtocols(function(socket) {
  45 + socket.resume();
  46 + socket.on('end', function() {
  47 + socket.end();
  48 + });
47 49 });
48 50 });
49 51 server.listen(common.PORT, function() {
19 test/simple/test-http-connect.js
@@ -29,20 +29,21 @@ var clientGotConnect = false;
29 29 var server = http.createServer(function(req, res) {
30 30 assert(false);
31 31 });
32   -server.on('connect', function(req, socket, firstBodyChunk) {
  32 +server.on('connect', function(req, res) {
33 33 assert.equal(req.method, 'CONNECT');
34 34 assert.equal(req.url, 'google.com:443');
35 35 common.debug('Server got CONNECT request');
36 36 serverGotConnect = true;
37 37
38   - socket.write('HTTP/1.1 200 Connection established\r\n\r\n');
39   -
40   - var data = firstBodyChunk.toString();
41   - socket.on('data', function(buf) {
42   - data += buf.toString();
43   - });
44   - socket.on('end', function() {
45   - socket.end(data);
  38 + res.writeHead(200, 'Connection Established');
  39 + res.switchProtocols(function(socket) {
  40 + var data = '';
  41 + socket.on('data', function(buf) {
  42 + data += buf.toString();
  43 + });
  44 + socket.on('end', function() {
  45 + socket.end(data);
  46 + });
46 47 });
47 48 });
48 49 server.listen(common.PORT, function() {
79 test/simple/test-http-pipelined-upgrade.js
... ... @@ -0,0 +1,79 @@
  1 +// Copyright Joyent, Inc. and other Node contributors.
  2 +//
  3 +// Permission is hereby granted, free of charge, to any person obtaining a
  4 +// copy of this software and associated documentation files (the
  5 +// "Software"), to deal in the Software without restriction, including
  6 +// without limitation the rights to use, copy, modify, merge, publish,
  7 +// distribute, sublicense, and/or sell copies of the Software, and to permit
  8 +// persons to whom the Software is furnished to do so, subject to the
  9 +// following conditions:
  10 +//
  11 +// The above copyright notice and this permission notice shall be included
  12 +// in all copies or substantial portions of the Software.
  13 +//
  14 +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS
  15 +// OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
  16 +// MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN
  17 +// NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM,
  18 +// DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
  19 +// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
  20 +// USE OR OTHER DEALINGS IN THE SOFTWARE.
  21 +
  22 +var common = require('../common');
  23 +var assert = require('assert');
  24 +var net = require('net');
  25 +var http = require('http');
  26 +
  27 +var agent = new http.Agent({ maxSockets: 1 });
  28 +
  29 +var gotRegularResponse = false;
  30 +var gotUpgradeResponse = false;
  31 +
  32 +var server = http.createServer(function(req, res) {
  33 + common.debug('Server got regular request');
  34 + setTimeout(function() {
  35 + common.debug('Server finished regular request');
  36 + res.writeHead(200, {'Content-Type': 'text/plain'});
  37 + res.end('Hello World\n');
  38 + }, 500);
  39 +});
  40 +server.on('upgrade', function(req, res) {
  41 + common.debug('Server got upgrade request');
  42 + res.writeHead(101, {
  43 + 'Upgrade': 'dummy',
  44 + 'Connection': 'Upgrade'
  45 + });
  46 + res.switchProtocols(function(sock) {
  47 + common.debug('Server finished update request');
  48 + sock.end();
  49 + server.close();
  50 + });
  51 +});
  52 +server.listen(common.PORT, function() {
  53 + var conn = net.createConnection(common.PORT);
  54 +
  55 + conn.on('connect', function() {
  56 + conn.write('GET / HTTP/1.1\r\n' +
  57 + '\r\n' +
  58 + 'GET / HTTP/1.1\r\n' +
  59 + 'Upgrade: WebSocket\r\n' +
  60 + 'Connection: Upgrade\r\n' +
  61 + '\r\n');
  62 + });
  63 +
  64 + conn.setEncoding('utf8');
  65 + var data = '';
  66 + conn.on('data', function(chunk) {
  67 + data += chunk;
  68 + });
  69 +
  70 + conn.on('end', function() {
  71 + conn.end();
  72 +
  73 + var regularIndex = data.indexOf('Hello World');
  74 + var upgradeIndex = data.indexOf('Upgrade');
  75 + assert.notEqual(regularIndex, -1);
  76 + assert.notEqual(upgradeIndex, -1);
  77 + assert.ok(regularIndex < upgradeIndex);
  78 + });
  79 +});
19 test/simple/test-http-upgrade-client2.js
@@ -26,12 +26,19 @@ var http = require('http');
26 26 var CRLF = '\r\n';
27 27
28 28 var server = http.createServer();
29   -server.on('upgrade', function(req, socket, head) {
30   - socket.write('HTTP/1.1 101 Ok' + CRLF +
31   - 'Connection: Upgrade' + CRLF +
32   - 'Upgrade: Test' + CRLF + CRLF + 'head');
33   - socket.on('end', function() {
34   - socket.end();
  29 +server.on('upgrade', function(req, res) {
  30 + res.writeHead(101, {
  31 + 'Connection': 'Upgrade',
  32 + 'Upgrade': 'Test'
  33 + });
  34 + res.switchProtocols(function(socket) {
  35 + socket.write('head');
  36 + socket.on('readable', function() {
  37 + socket.read();
  38 + });
  39 + socket.on('end', function() {
  40 + socket.end();
  41 + });
35 42 });
36 43 });
37 44
33 test/simple/test-http-upgrade-server.js
@@ -49,22 +49,23 @@ function testServer() {
49 49 res.end();
50 50 });
51 51
52   - server.on('upgrade', function(req, socket, upgradeHead) {
53   - socket.write('HTTP/1.1 101 Web Socket Protocol Handshake\r\n' +
54   - 'Upgrade: WebSocket\r\n' +
55   - 'Connection: Upgrade\r\n' +
56   - '\r\n\r\n');
57   -
58   - request_upgradeHead = upgradeHead;
59   -
60   - socket.ondata = function(d, start, end) {
61   - var data = d.toString('utf8', start, end);
62   - if (data == 'kill') {
63   - socket.end();
64   - } else {
65   - socket.write(data, 'utf8');
66   - }
67   - };
  52 + server.on('upgrade', function(req, res) {
  53 + res.writeHead(101, {
  54 + 'Upgrade': 'WebSocket',
  55 + 'Connection': 'Upgrade'
  56 + });
  57 + res.switchProtocols(function(socket) {
  58 + request_upgradeHead = socket.read();
  59 +
  60 + socket.ondata = function(d, start, end) {
  61 + var data = d.toString('utf8', start, end);
  62 + if (data == 'kill') {
  63 + socket.end();
  64 + } else {
  65 + socket.write(data, 'utf8');
  66 + }
  67 + };
  68 + });
68 69 });
69 70 }
70 71
2  test/simple/test-http-upgrade-server2.js
@@ -29,7 +29,7 @@ var server = http.createServer(function(req, res) {
29 29 throw new Error('This shouldn\'t happen.');
30 30 });
31 31
32   -server.on('upgrade', function(req, socket, upgradeHead) {
  32 +server.on('upgrade', function(req, res) {
33 33 common.error('got upgrade event');
34 34 // test that throwing an error from upgrade gets
35 35 // is uncaught

Tip: You can add notes to lines in a file. Hover to the left of a line to make a note

Something went wrong with that request. Please try again.