getjackd/jackd

Incorrect ASCII encoding enforcement

Closed this issue · 3 comments

edqallen commented

After the better part of a day debugging code, I've determined that jackd has incorrectly implemented the beanstalk protocol in a rather intrusive manner.

The tl;dr is that jackd is forcing the whole conversation with the server to be ASCII encoded when it should not be, and the fix is pretty simple: Remove the three instances of the string 'ascii' in index.js

Here's the longer explanation:

The beanstalk protocol doc states the following which I believe is the source of the issue:

The beanstalk protocol runs over TCP using ASCII encoding

If one were to stop reading here, the incorrect behavior would be easily explained. However further down the document, the following is stated:

The protocol contains two kinds of data: text lines and unstructured chunks of
data. Text lines are used for client commands and server responses. Chunks are
used to transfer job bodies and stats information. Each job body is an opaque
sequence of bytes. The server never inspects or modifies a job body and always
sends it back in its original form. It is up to the clients to agree on a
meaningful interpretation of job bodies.

I've been using the beanstalkd server for many years with a PHP app and routinely put UTF-8 and even binary data in the server and retrieve it, and it works flawlessly for this. The Jackd library is incorrectly interpreting the documentation and as a result, corrupting data.

divmgl commented

Good call. I'll remove it altogether. I wasn't aware this was happening and it makes sense. We typically just want to send over the bytes and let the caller figure out how they want to parse the bytes. I'll make this change now. Cheers.

  • 👍1
edqallen commented

Thanks! I've just been using a modified version in the meantime. You can close this or not at your leisure. I didn't want to myself in case you want to keep it open until the change is made.

divmgl commented

I appreciate you making this issue by the way. I'm going to make this change immediately as it doesn't adhere to the beanstalkd protocol, which was one of my core motivations behind this project.

This is a breaking change so I'll likely need to bump this library up to version 2. I'll keep you posted once this is finished.