Skip to content

Commit

Permalink
[fix] Ignore the threshold option if context takeover is enabled
Browse files Browse the repository at this point in the history
When context takeover is enabled, compress messages even if their size
is below the value of `threshold` option.

Refs: #1950
  • Loading branch information
lpinca committed Sep 29, 2021
1 parent 474aa36 commit 41ae563
Show file tree
Hide file tree
Showing 6 changed files with 159 additions and 142 deletions.
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ const wss = new WebSocketServer({
// Below options specified as default values.
concurrencyLimit: 10, // Limits zlib concurrency for perf.
threshold: 1024 // Size (in bytes) below which messages
// should not be compressed.
// should not be compressed if context takeover is disabled.
}
});
```
Expand Down
4 changes: 2 additions & 2 deletions doc/ws.md
Original file line number Diff line number Diff line change
Expand Up @@ -147,8 +147,8 @@ value). If an object is provided then that is extension parameters:
zlib on deflate.
- `zlibInflateOptions` {Object} [Additional options][zlib-options] to pass to
zlib on inflate.
- `threshold` {Number} Payloads smaller than this will not be compressed.
Defaults to 1024 bytes.
- `threshold` {Number} Payloads smaller than this will not be compressed if
context takeover is disabled. Defaults to 1024 bytes.
- `concurrencyLimit` {Number} The number of concurrent calls to zlib. Calls
above this limit will be queued. Default 10. You usually won't need to touch
this option. See [this issue][concurrency-limit] for more details.
Expand Down
2 changes: 1 addition & 1 deletion lib/permessage-deflate.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ class PerMessageDeflate {
* @param {Boolean} [options.serverNoContextTakeover=false] Request/accept
* disabling of server context takeover
* @param {Number} [options.threshold=1024] Size (in bytes) below which
* messages should not be compressed
* messages should not be compressed if context takeover is disabled
* @param {Object} [options.zlibDeflateOptions] Options to pass to zlib on
* deflate
* @param {Object} [options.zlibInflateOptions] Options to pass to zlib on
Expand Down
10 changes: 9 additions & 1 deletion lib/sender.js
Original file line number Diff line number Diff line change
Expand Up @@ -274,7 +274,15 @@ class Sender {

if (this._firstFragment) {
this._firstFragment = false;
if (rsv1 && perMessageDeflate) {
if (
rsv1 &&
perMessageDeflate &&
perMessageDeflate.params[
perMessageDeflate._isServer
? 'server_no_context_takeover'
: 'client_no_context_takeover'
]
) {
rsv1 = buf.length >= perMessageDeflate._threshold;
}
this._compress = rsv1;
Expand Down
23 changes: 7 additions & 16 deletions test/permessage-deflate.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -344,7 +344,7 @@ describe('PerMessageDeflate', () => {

describe('#compress and #decompress', () => {
it('works with unfragmented messages', (done) => {
const perMessageDeflate = new PerMessageDeflate({ threshold: 0 });
const perMessageDeflate = new PerMessageDeflate();
const buf = Buffer.from([1, 2, 3]);

perMessageDeflate.accept([{}]);
Expand All @@ -361,7 +361,7 @@ describe('PerMessageDeflate', () => {
});

it('works with fragmented messages', (done) => {
const perMessageDeflate = new PerMessageDeflate({ threshold: 0 });
const perMessageDeflate = new PerMessageDeflate();
const buf = Buffer.from([1, 2, 3, 4]);

perMessageDeflate.accept([{}]);
Expand All @@ -388,7 +388,6 @@ describe('PerMessageDeflate', () => {

it('works with the negotiated parameters', (done) => {
const perMessageDeflate = new PerMessageDeflate({
threshold: 0,
memLevel: 5,
level: 9
});
Expand All @@ -415,11 +414,9 @@ describe('PerMessageDeflate', () => {

it('honors the `level` option', (done) => {
const lev0 = new PerMessageDeflate({
threshold: 0,
zlibDeflateOptions: { level: 0 }
});
const lev9 = new PerMessageDeflate({
threshold: 0,
zlibDeflateOptions: { level: 9 }
});
const extensionStr =
Expand Down Expand Up @@ -459,7 +456,6 @@ describe('PerMessageDeflate', () => {

it('honors the `zlib{Deflate,Inflate}Options` option', (done) => {
const lev0 = new PerMessageDeflate({
threshold: 0,
zlibDeflateOptions: {
level: 0,
chunkSize: 256
Expand All @@ -469,7 +465,6 @@ describe('PerMessageDeflate', () => {
}
});
const lev9 = new PerMessageDeflate({
threshold: 0,
zlibDeflateOptions: {
level: 9,
chunkSize: 128
Expand Down Expand Up @@ -523,7 +518,7 @@ describe('PerMessageDeflate', () => {
});

it("doesn't use contex takeover if not allowed", (done) => {
const perMessageDeflate = new PerMessageDeflate({ threshold: 0 }, true);
const perMessageDeflate = new PerMessageDeflate({}, true);
const extensions = extension.parse(
'permessage-deflate;server_no_context_takeover'
);
Expand Down Expand Up @@ -554,7 +549,7 @@ describe('PerMessageDeflate', () => {
});

it('uses contex takeover if allowed', (done) => {
const perMessageDeflate = new PerMessageDeflate({ threshold: 0 }, true);
const perMessageDeflate = new PerMessageDeflate({}, true);
const extensions = extension.parse('permessage-deflate');
const buf = Buffer.from('foofoo');

Expand Down Expand Up @@ -583,7 +578,7 @@ describe('PerMessageDeflate', () => {
});

it('calls the callback when an error occurs (inflate)', (done) => {
const perMessageDeflate = new PerMessageDeflate({ threshold: 0 });
const perMessageDeflate = new PerMessageDeflate();
const data = Buffer.from('something invalid');

perMessageDeflate.accept([{}]);
Expand All @@ -596,11 +591,7 @@ describe('PerMessageDeflate', () => {
});

it("doesn't call the callback twice when `maxPayload` is exceeded", (done) => {
const perMessageDeflate = new PerMessageDeflate(
{ threshold: 0 },
false,
25
);
const perMessageDeflate = new PerMessageDeflate({}, false, 25);
const buf = Buffer.from('A'.repeat(50));

perMessageDeflate.accept([{}]);
Expand All @@ -616,7 +607,7 @@ describe('PerMessageDeflate', () => {
});

it('calls the callback if the deflate stream is closed prematurely', (done) => {
const perMessageDeflate = new PerMessageDeflate({ threshold: 0 });
const perMessageDeflate = new PerMessageDeflate();
const buf = Buffer.from('A'.repeat(50));

perMessageDeflate.accept([{}]);
Expand Down
Loading

0 comments on commit 41ae563

Please sign in to comment.