Decompression does not work on Node.js >= 12 if the BFINAL bit is set to 1

[jeffrson]

Description

I want ws to connect to a self written WebSocket Server based on https://github.com/sta/websocket-sharp.
While the test program works when using NodeJS 10.17.0, it doesn't with v12 or v13 - although the client connects and can send data successfully to the server, it does not show any received data. Since it does not seem to respond on "ping", the server library closes the connection after some time.

The server cannot be too wrong, since another short test with https://github.com/theturtle32/WebSocket-Node succeeds.

Reproducible in:

• version: 7.2.0
• Node.js version(s): 13.3.0, 12.13.1
• OS version(s): Windows 10

Attachments:

const WebSocket = require('ws');
const ws = new WebSocket('ws://127.0.0.1:12345/test');

ws.on('open', () => {
  ws.on('message', data => console.log(data))
  ws.on('close', error => console.log('close'))
  ws.on('error', error => console.log(`Error: ${error}`))
  ws.on('unexpected-response', error => console.log('unexpected-response'))
  ws.on('upgrade', error => console.log('upgrade'))
  ws.on('ping', () => console.log('ping'))
  ws.on('pong', data => console.log('pong'))

  let login = {
      name: "TEST",
  }

  ws.send(JSON.stringify(login))
})

[lpinca]

I don't think it's an issue in the library because given ws popularity this problem would have flooded the issue tracker.

Add a listener for the 'data' event on the internal socket and log the raw data.

ws.on('open', function() {
  ws._socket.prependListener('data', console.log);
});

Also, in your example, you should register the various event listeners outside of the handler of the 'open' event otherwise if 'open' is not emitted the listeners of 'close', 'unexpected-response', 'upgrade', etc. will never be registered.

[jeffrson]

I don't think it's an issue in the library because given ws popularity this problem would have flooded the issue tracker.

Yes, sure - it's strange, however, that it works when moving to node10.

I followed your advice and added the listener to the internal socket. Data arrives - and it's the same for Node10 and Node12. Again, Node10 sees the data, Node12 doesn't.
Then I tried another websocket server, python based, and everything goes well. I compared data, and there is a bit of a difference. So I was poking through the source of websocket-sharp and it looks like it's related to compression.
I disabled compression (perMessagedeflate: false) and it works!

Now there are two things I don't understand here. Why's there a difference with compression (websocket-sharp is using .Net's DeflateStream) and why does it work with Node10.

Edit: C# DeflateStream appears to yield the same result as https://nodejs.org/api/zlib.html#zlib_zlib_deflateraw_buffer_options_callback

[lpinca]

What are negotiated parameters? Are default values used? Are the messages fragmented? Try to bypass ws and use an inflate stream directly. Write to it the data you get from the C# DeflateStream.

This is basically what ws does:

const assert = require('assert');
const zlib = require('zlib');

const message = Buffer.from('a'.repeat(1024));
let flushed = false;

const deflateChunks = [];
const inflateChunks = [];
const deflate = zlib.createDeflateRaw();
const inflate = zlib.createInflateRaw();

deflate.on('data', function(chunk) {
  deflateChunks.push(chunk);
});

inflate.on('data', function(chunk) {
  inflateChunks.push(chunk);
});

deflate.write(message);
deflate.flush(zlib.constants.Z_SYNC_FLUSH, function() {
  inflate.write(Buffer.concat(deflateChunks));
  deflateChunks.length = 0;

  inflate.flush(function() {
    flushed = true;
    const buf = Buffer.concat(inflateChunks);

    inflateChunks.length = 0;
    assert(buf.equals(message));
  });
});

process.on('exit', function() {
  assert(flushed);
});

[jeffrson]

Thank you for keeping up! I'll continue to follow the compression path...

For the time being - this is what I can log from the C# server:

15.12.2019 10:21:25|Debug|WebSocket.acceptHandshake:865|A handshake request from 127.0.0.1:53218:
GET /test HTTP/1.1
Sec-WebSocket-Version: 13
Sec-WebSocket-Key: 5up1gUmmJQiajMaCi83U+g==
Connection: Upgrade
Upgrade: websocket
Sec-WebSocket-Extensions: permessage-deflate; client_max_window_bits
Host: 127.0.0.1:4649

15.12.2019 10:21:25|Debug|WebSocket.sendHttpResponse:2066|A response to 127.0.0.1:53218:
HTTP/1.1 101 Switching Protocols
Server: websocket-sharp/1.0
Upgrade: websocket
Connection: Upgrade
Sec-WebSocket-Accept: OxsUtcSfeneuAu+hmo7Wd3lNp7g=
Sec-WebSocket-Extensions: permessage-deflate; client_no_context_takeover; server_no_context_takeover

Assuming the server sends a string "Welcome" when the client connects. The client receives:
<Buffer c1 09 0b 4f cd 49 ce cf 4d 05 00>

It works with the python server, and receives:
<Buffer c1 09 0a 4f cd 49 ce cf 4d 05 00>
Welcome

(1 byte difference - 0b fails, 0a works)

Deflate of the "Welcome"-String however is actually <Buffer 0b 4f cd 49 ce cf 4d 05 00> (with 0b).

Does this sound familar?

[lpinca]

const zlib = require('zlib');

const message = Buffer.from('Welcome');

const chunks = [];
const deflate = zlib.createDeflateRaw();

deflate.on('data', function(chunk) {
  chunks.push(chunk);
});

deflate.write(message);
deflate.flush(zlib.constants.Z_SYNC_FLUSH, function() {
  console.log(Buffer.concat(chunks).slice(0, -4))
});

Prints <Buffer 0a 4f cd 49 ce cf 4d 05 00> and this

const zlib = require('zlib');

const chunks = [];
const inflate = zlib.createInflateRaw();

inflate.on('data', function(chunk) {
  chunks.push(chunk);
});

inflate.write(
  Buffer.from([0x0a, 0x4f, 0xcd, 0x49, 0xce, 0xcf, 0x4d, 0x05, 0x00])
);
inflate.write(Buffer.from([0x00, 0x00, 0xff, 0xff]));
inflate.flush(function() {
  const buf = Buffer.concat(chunks);

  chunks.length = 0;
  console.log(buf.toString());
});

works as expected. If the first byte is changed to 0b it no longer works. Does it work for you on Node.js 10? What version specifically?

[jeffrson]

It's the latest version - 10.17.0. Now I tried the 11 versions - it seems to work for me until 11.10.1, and stops working with 11.11.0. Have to checkout the changelog.

Now what's strange - this code

const zlib = require('zlib')

const input = 'Welcome';
zlib.deflateRaw(input, (err, buffer) => {
  if (!err) {
    console.log(buffer);

    zlib.inflateRaw(buffer, (err, output) => {
      if (!err) {
        console.log(output.toString())
      } else {
      }
    })

  } else {
  }
})

results in

<Buffer 0b 4f cd 49 ce cf 4d 05 00>
Welcome

[lpinca]

It seems a regression introduced in nodejs/node@28db96f.

If that commit is reverted my example above works even if the first byte of the buffer is 0x0b. The first byte is 0x0b if the stream is explicitly ended.

const chunks = [];
const deflate = zlib.createDeflateRaw();

deflate.on('data', function(chunk) {
  chunks.push(chunk);
});

deflate.on('end', function() {
  console.log(Buffer.concat(chunks)); // <Buffer 0b 4f cd 49 ce cf 4d 05 00>
});

deflate.end(Buffer.from('Welcome'));

This is what happens when using zlib.deflateRaw{,Sync}().

Do you want to open an issue in the Node.js repository?

[jeffrson]

Yes, that's the only change related to zlib. Great that you were able to verify this.

I would open an issue - I'm not sure though, why there's the '0x0b' instead of '0x0a'.

Thanks for analysing!

[lpinca]

I'm not sure though, why there's the '0x0b' instead of '0x0a'

That's because the BFINAL bit is set by C# DeflateStream or when using zlib.deflateRaw{,Sync}() to signal that it is the last block in compressed data.

According to the spec https://tools.ietf.org/html/rfc7692#section-7.2.1

An endpoint MAY use both DEFLATE blocks with the "BFINAL" bit set to 0 and DEFLATE blocks with the "BFINAL" bit set to 1.

both are valid.

I tested with Chrome and sent the same data ws.send('Welcome');. I received this frame c1 89 be 2d be ab b4 62 73 e2 70 e2 f3 ae be where be 2d be ab is the mask. After unmasking the actual compressed data is 0a 4f cd 49 ce cf 4d 05 00 so it seems Chrome also uses blocks with the BFINAL bit set to 0. I did not test it but if Firefox does the same it might explain why this issue has gone unnoticed for so long.

[lpinca]

Should be fixed by nodejs/node@0e89b64.