michaelwittig / node-q

Q interfacing with Node.js
MIT License
52 stars 14 forks source link

Serialiser does not escape the null character in strings, causing badmsg error #43

Closed wwarby closed 5 years ago

wwarby commented 5 years ago

With the help of a consultant at Kx, I've got to the bottom of an issue we were facing where certain row upserts we were sending caused a badmsg error followed by the socket being forcibly closed by the q process. It turns out we had some strings that contain the null character which the serializer uses as a terminator between elements of the message, and those null characters are not being properly escaped by the serializer. This simple example can demonstrate the problem:

var conn = nodeq.connect('localhost', 5004, (err, conn) => {
  conn.k('show', nodeq.symbol('foo\u0000'), (err, result) => {
    if (err) { console.error(err); }
    console.log(result);
  });
});

In the above example the server shows badmsg, but the callback passed to conn.k is never called. If I use conn.ks, the connection eventually seems to get terminated by the server.

I've worked around this for now by replacing every nodeq.symbol(x) call with sanitise(nodeq.symbol(x)) where sanitise() simply removes the \u0000 from the input string. That works but really it seems as though the serialiser should be able to handle and properly escape input containing the character it uses as a delimiter. I realise the serialiser is part of c.js but since your library depends on it I'm reporting it here. I've also reported it to Kx.

michaelwittig commented 5 years ago

fixed in node-q@2.4.3