No Connection Header in Node-SPDY

‹prev | My Chain | next›

Up today, I hope to finally resolve issue #1 of express-spdy.

According to draft #2 of the SPDY specification, servers should not set Connection or Keep-Alive headers. In draft #3, that changed to must not set:

The Connection, Keep-Alive, Proxy-Connection, and Transfer-Encoding headers are not valid and MUST not be sent.

But, as issue #1 points out, the Connection header is being set and is being set to Keep-Alive.

Last night, I tracked that down to the following node-spdy code:

var Response = exports.Response = function(cframe, c) {
//...
  this._headers = {
    'Connection': 'keep-alive'
  };
//...
};

The solution seems simple enough—remove the Connection key-value pair from headers. But before doing that for real, I need to make sure that Chrome will continue to allow connections without it. This would not be the first time that Chrome failed to follow the SPDY spec.

So, I comment out that line in node-spdy, start up my test express-spdy app and see:

As always, it looks very much like a vanilla express.js app, but, checking the SPDY tab in Chrome's about:net-internals, I see a beautiful SPDY session ongoing:

t=1314238477571 [st= 47]     SPDY_SESSION_SYN_REPLY  
                             --> flags = 0
                             --> content-length: 50360
                                 content-type: text/html
                                 status: 200 OK
                                 version: HTTP/1.1
                                 x-powered-by: Express
                             --> id = 1

Nice. The Content header is definitely missing from there. Even better, subsequent requests to the sample app produce normal SPDY SYN_STREAMs and SYN_REPLYs with nary a RST_STREAM in sight:

t=1314239371362 [st=893838]     SPDY_SESSION_SYN_STREAM  
                                --> flags = 1
                                --> accept: text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8
                                    accept-charset: ISO-8859-1,utf-8;q=0.7,*;q=0.3
                                    accept-encoding: gzip,deflate,sdch
                                    accept-language: en-US,en;q=0.8
                                    host: jaynestown.local:3000
                                    method: GET
                                    referer: https://jaynestown.local:3000/one.html
                                    scheme: https
                                    url: /two.html
                                    user-agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/535.2 (KHTML, like Gecko) Chrome/15.0.854.0 Safari/535.2
                                    version: HTTP/1.1
                                --> id = 9
t=1314239371388 [st=893864]     SPDY_SESSION_SYN_REPLY  
                                --> flags = 0
                                --> accept-ranges: bytes
                                    cache-control: public, max-age=0
                                    content-length: 49489
                                    content-type: text/html; charset=UTF-8
                                    etag: "49489-1309920451000"
                                    last-modified: Wed, 06 Jul 2011 02:47:31 GMT
                                    status: 200 OK
                                    version: HTTP/1.1
                                    x-powered-by: Express

Cool. So it seems that Connection/Keep-Alive can be safely removed.

Now to drive this implementation with tests. I revert my change and create a new test/response-test.js file. For now, I am only testing the response headers, so the overall vows.js structure will be:

vows.describe('SPDY Response').
addBatch({
  'headers': {
    topic: function() {
      // Setup connection request and send response here
    },
    'it should not contain a Connection header': function(cframe) {
      // Assertions here
    }
  }
})

The assertions ought to be fairly straight forward:

'it should not contain a Connection header': function(cframe) {
      assert.equal(cframe.headers['Connection', undefined);
    }

The topic of the test requires a little more effort. The topic will actually come through the connection object's write method. So I need a connection object, a bare-bones request, and a response to send along that connection:

topic: function() {
      var callback = this.callback,
          connection = {
            zlib: spdy.createZLib(),
            write: function(cframe) { callback(null, cframe); }
          },
          req = { data: { streamID: 1 } },
          response = spdy.createResponse(req, connection);

      response._flushHead();
    }

Only, when I run this test, I get:

➜  node-spdy git:(master) ✗ vows test/response-test.js
✗

  headers
    ✗ it should not contain a Connection header
      » An unexpected error was caught: {"0":128,"1":2,"2":0,"3":2,"4":0,"5....>

The unexpected error actually goes on for a looong time. That's just the control frame itself. Nothing too interesting there, so why is it being reported as an "unexpected error"? Hrm... something seems vaguely familiar with that error. Where have I seen that before? Oh, yeah. Sheesh, I keep coming across that. I am beginning to think this may be a vows issue not a Chris issue. Anyhow, the solution, is to ensure that the callback is being called with two values:

write: function(cframe) { callback(null, cframe); }

And, with that, I have my test fixed. And by "fixed", I mean broken:

➜  node-spdy git:(master) ✗ vows test/response-test.js --spec

♢ SPDY Response

  headers
    ✗ it should not contain a Connection header
      » expected undefined,
        got      'keep-alive' (==) // response-test.js:31
 
✗ Broken » 1 broken (0.018s)

Now, I remove the "Connection" header from node-spdy's Response class for good and, just like that, I have BDD'd a fix for my issue:

➜  node-spdy git:(master) ✗ vows test/response-test.js --spec

♢ SPDY Response

  headers
    ✓ it should not contain a Connection header
 
✓ OK » 1 honored (0.005s)

Yay!

I'm not sure exactly what I'll work on tomorrow, but I'm pretty sure it's gonna be awesome.

Day #122