diff --git a/lib/client/client.js b/lib/client/client.js index e6c4ecd..bcd8c85 100644 --- a/lib/client/client.js +++ b/lib/client/client.js @@ -4,11 +4,12 @@ var EventEmitter = require('events').EventEmitter; var net = require('net'); var tls = require('tls'); var util = require('util'); + var once = require('once'); var backoff = require('backoff'); var vasync = require('vasync'); - var assert = require('assert-plus'); +var VError = require('verror').VError; var Attribute = require('../attribute'); var Change = require('../change'); @@ -986,8 +987,8 @@ Client.prototype.connect = function connect() { }); socket.ldap.parser.on('error', function onParseError(err) { - log.trace({err: err}, 'parser error event'); - self.emit('error', err); + self.emit('error', new VError(err, 'Parser error for %s', + socket.ldap.id)); self.connected = false; socket.end(); }); diff --git a/lib/messages/parser.js b/lib/messages/parser.js index 17109c8..548b433 100644 --- a/lib/messages/parser.js +++ b/lib/messages/parser.js @@ -1,10 +1,11 @@ // Copyright 2011 Mark Cavage, Inc. All rights reserved. -var assert = require('assert'); var EventEmitter = require('events').EventEmitter; var util = require('util'); +var assert = require('assert-plus'); var asn1 = require('asn1'); +var VError = require('verror').VError; var AbandonRequest = require('./abandon_request'); var AddRequest = require('./add_request'); @@ -76,12 +77,19 @@ Parser.prototype.write = function (data) { self.buffer = (self.buffer ? Buffer.concat([self.buffer, data]) : data); var ber = new BerReader(self.buffer); - if (!ber.readSequence()) - return false; - if (ber.remain < ber.length) { // ENOTENOUGH + var foundSeq = false; + try { + foundSeq = ber.readSequence(); + } catch (e) { + this.emit('error', e); + } + + if (!foundSeq || ber.remain < ber.length) { + // ENOTENOUGH return false; - } else if (ber.remain > ber.length) { // ETOOMUCH + } else if (ber.remain > ber.length) { + // ETOOMUCH // This is sort of ugly, but allows us to make miminal copies nextMessage = self.buffer.slice(ber.offset + ber.length); ber._size = ber.offset + ber.length; @@ -92,18 +100,21 @@ Parser.prototype.write = function (data) { // pointing at the next sequence of data (if it exists) self.buffer = null; - var message = this.getMessage(ber); - if (!message) - return end(); - + var message; try { + // Bail here if peer isn't speaking protocol at all + message = this.getMessage(ber); + + if (!message) { + return end(); + } message.parse(ber); } catch (e) { this.emit('error', e, message); return false; } - this.emit('message', message); + this.emit('message', message); return end(); }; @@ -113,14 +124,8 @@ Parser.prototype.getMessage = function (ber) { var self = this; - try { - var messageID = ber.readInt(); - var type = ber.readSequence(); - } catch (e) { - // Handle servers that aren't speaking the language at all - this.emit('error', e); - return false; - } + var messageID = ber.readInt(); + var type = ber.readSequence(); var Message; switch (type) { @@ -207,8 +212,7 @@ Parser.prototype.getMessage = function (ber) { default: this.emit('error', - new Error('protocolOp 0x' + - (type ? type.toString(16) : '??') + + new Error('Op 0x' + (type ? type.toString(16) : '??') + ' not supported'), new LDAPResult({ messageID: messageID, diff --git a/lib/server.js b/lib/server.js index c4e3a1d..6bc0f17 100644 --- a/lib/server.js +++ b/lib/server.js @@ -7,6 +7,7 @@ var tls = require('tls'); var util = require('util'); var asn1 = require('asn1'); +var VError = require('verror').VError; var dn = require('./dn'); var dtrace = require('./dtrace'); @@ -284,7 +285,8 @@ function Server(options) { if (c.type === 'unix') { c.remoteAddress = self.server.path; c.remotePort = c.fd; - } else if (c.socket) { // TLS + } else if (c.socket) { + // TLS c.remoteAddress = c.socket.remoteAddress; c.remotePort = c.socket.remotePort; } @@ -406,9 +408,7 @@ function Server(options) { }); c.parser.on('error', function (err, message) { - log.error('Exception happened parsing for %s: %s', - c.ldap.id, err.stack); - + self.emit('error', new VError(err, 'Parser error for %s', c.ldap.id)); if (!message) return c.destroy(); @@ -426,16 +426,7 @@ function Server(options) { if (log.trace()) log.trace('data on %s: %s', c.ldap.id, util.inspect(data)); - try { - c.parser.write(data); - } catch (e) { - log.warn('Unable to parse message [c.on(\'data\')]: %s', e.stack); - c.end(new LDAPResult({ - status: 0x02, - errorMessage: e.toString(), - connection: c - }).toBer()); - } + c.parser.write(data); }); } // end newConnection diff --git a/package.json b/package.json index 81d686d..68c176e 100644 --- a/package.json +++ b/package.json @@ -33,7 +33,8 @@ "dashdash": "1.6.0", "backoff": "2.4.0", "once": "1.3.0", - "vasync": "1.5.0" + "vasync": "1.5.0", + "verror": "~1.4.0" }, "optionalDependencies": { "dtrace-provider": "0.2.8" diff --git a/test/messages/parser.test.js b/test/messages/parser.test.js new file mode 100644 index 0000000..4648828 --- /dev/null +++ b/test/messages/parser.test.js @@ -0,0 +1,64 @@ +// Copyright 2014 Joyent, Inc. All rights reserved. + +var test = require('tape').test; +var bunyan = require('bunyan'); + +///--- Globals + +var lib; +var Parser; +var LOG = bunyan.createLogger({name: 'ldapjs-test'}); + +///--- Tests +test('load library', function (t) { + lib = require('../../lib/'); + Parser = lib.Parser; + + t.ok(Parser); + t.end(); +}); + +test('wrong protocol error', function (t) { + var p = new Parser({log: LOG}); + + p.once('error', function (err) { + t.ok(err); + t.end(); + }); + + // Send some bogus data to incur an error + p.write(new Buffer([16, 1, 4])); +}); + +test('bad protocol op', function (t) { + var p = new Parser({log: LOG}); + var message = new lib.LDAPMessage({ + protocolOp: 254 // bogus (at least today) + }); + p.once('error', function (err) { + t.ok(err); + t.ok(/not supported$/.test(err.message)); + t.end(); + }); + p.write(message.toBer()); +}); + +test('bad message structure', function (t) { + var p = new Parser({log: LOG}); + + // message with bogus structure + var message = new lib.LDAPMessage({ + protocolOp: lib.LDAP_REQ_EXTENSION + }); + message._toBer = function (writer) { + writer.writeBuffer(new Buffer([16, 1, 4]), 80); + return writer; + }; + + p.once('error', function (err) { + t.ok(err); + t.end(); + }); + + p.write(message.toBer()); +});