Thursday, August 18, 2011

Mitigation Strategy for Unstable writeUInt32 in Node.js

‹prev | My Chain | next›

Last night I found that the protocol library in node-spdy is broken on node 0.5.4. It is even worse on master / 0.5.5.

The problem occurs in writeUInt32 (and it's 16 bit equivalent). In a binary protocol like SPDY, writing integers is kind of a big deal. The endianness of those integers used to be specified by a string argument ('big' or 'little'). Now it is specified in the method name itself:
Buffer.prototype.writeInt32BE = function(value, offset, noAssert) {
//...
};
I think that my strategy for dealing with this will be to check the Buffer prototype for a writeInt32BE method. If not there, I will add my own that wraps the old writeUInt32 method.

So, first up, I search and replace all instances of UInt methods with their big or little endian equivalent. Thus, this:
result.writeUInt32(headers.streamID & 0x7fffffff, 0, 'big');
...Becomes:
result.writeUInt32BE(headers.streamID & 0x7fffffff, 0);
Since the SPDY SETTINGS frame continues to violate the spec and be little endian, I do need to be careful to replace some UInt calls with a LE equivalent:
exports.createSettingsFrame = function(zlib, settings) {
// ...
buff.writeUInt32LE(raw_key & 0xffffff, offset);
// ...
}
I also have to make similar changes to the readUInt equivalents in parser.js, but, once those changes are in place, I am able to run an express-spdy site on node 0.5.5-pre:



And the SPDY tab confirms that it is, indeed a SPDY session:



OK great. I have node-spdy working with the soon-to-be-released node 0.5.5, but what about previous versions of node like 0.5.3? (again, I am skipping 0.5.4 completely)

To handle that case, I check the Buffer prototype. If it defines writeUInt32BE, then all is well and no changes are required. If it does not, then I need to add it myself:
/**
* Compatibility with older versions of node
*/
if (!Buffer.prototype.writeUInt32BE) {
Buffer.prototype.writeUInt32BE = function(value, offset, noAssert) {
this.writeUInt32(value, offset, 'big');
};
Buffer.prototype.writeUInt32LE = function(value, offset, noAssert) {
this.writeUInt32(value, offset, 'little');
};
Buffer.prototype.writeUInt16BE = function(value, offset, noAssert) {
this.writeUInt16(value, offset, 'big');
};
}
In the case that an older version of node is being used, one that does not define writeUInt32BE, I define it myself using the same method signature used in node 0.5.5. In such a case, it must be an older version of node (because they wouldn't change it again would they?). In that case, I simply pass along the same parameters to the writeUInt32 method that existed back then.

I do the same for the readUInt equivalents over in parser.js:
/**
* Compatibility with older versions of node
*/
if (!Buffer.prototype.readUInt32BE) {
Buffer.prototype.readUInt32BE = function(offset, noAssert) {
this.readUInt32(offset, 'big');
};
Buffer.prototype.readUInt16BE = function(offset, noAssert) {
this.readUInt16(offset, 'big');
};
}
Easy peasy.

First I retry my test express-spdy app to verify that nothing broke. Nothing did break. I still have my nice SPDY sessions going on.

Now the fun part. I start up the sample express-spdy app with node 0.5.3:
➜  express-spdy-test  ~/local/node-v0.5.3/bin/node app.js
And when I access the site in Chrome... nothing. There are no errors in the console, but the browser is not doing anything other than showing the little spinny, throbber.

Well, no errors is a good sign. At least nothing is crashing. So, for the most part everything is hooked up, but maybe not writing properly. Or... reading.

Ack! Dang it, this is Javascript, not Ruby and not even Coffeescript. This is Javascript and my read statements need to return values:
Buffer.prototype.readUInt32BE = function(offset, noAssert) {
return this.readUInt32(offset, 'big');
};
Better.

With that... it actually works! Until they change writeUInt32 again. Ah well, at least I have a decent mitigation strategy in place now!

Expect a new version of node-spdy to hit npm in a day or so.


Day #117

No comments:

Post a Comment