Skip to content

Commit 41ae563

Browse files
lpincalpinca
committedSep 29, 2021
[fix] Ignore the threshold option if context takeover is enabled
When context takeover is enabled, compress messages even if their size is below the value of `threshold` option. Refs: #1950
·
8.18.18.2.3
1 parent 474aa36 commit 41ae563

File tree

6 files changed

+159
-142
lines changed

6 files changed

+159
-142
lines changed
 

‎README.md

+1-1
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,7 @@ const wss = new WebSocketServer({
118118
// Below options specified as default values.
119119
concurrencyLimit: 10, // Limits zlib concurrency for perf.
120120
threshold: 1024 // Size (in bytes) below which messages
121-
// should not be compressed.
121+
// should not be compressed if context takeover is disabled.
122122
}
123123
});
124124
```

‎doc/ws.md

+2-2
Original file line numberDiff line numberDiff line change
@@ -147,8 +147,8 @@ value). If an object is provided then that is extension parameters:
147147
zlib on deflate.
148148
- `zlibInflateOptions` {Object} [Additional options][zlib-options] to pass to
149149
zlib on inflate.
150-
- `threshold` {Number} Payloads smaller than this will not be compressed.
151-
Defaults to 1024 bytes.
150+
- `threshold` {Number} Payloads smaller than this will not be compressed if
151+
context takeover is disabled. Defaults to 1024 bytes.
152152
- `concurrencyLimit` {Number} The number of concurrent calls to zlib. Calls
153153
above this limit will be queued. Default 10. You usually won't need to touch
154154
this option. See [this issue][concurrency-limit] for more details.

‎lib/permessage-deflate.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ class PerMessageDeflate {
4141
* @param {Boolean} [options.serverNoContextTakeover=false] Request/accept
4242
* disabling of server context takeover
4343
* @param {Number} [options.threshold=1024] Size (in bytes) below which
44-
* messages should not be compressed
44+
* messages should not be compressed if context takeover is disabled
4545
* @param {Object} [options.zlibDeflateOptions] Options to pass to zlib on
4646
* deflate
4747
* @param {Object} [options.zlibInflateOptions] Options to pass to zlib on

‎lib/sender.js

+9-1
Original file line numberDiff line numberDiff line change
@@ -274,7 +274,15 @@ class Sender {
274274

275275
if (this._firstFragment) {
276276
this._firstFragment = false;
277-
if (rsv1 && perMessageDeflate) {
277+
if (
278+
rsv1 &&
279+
perMessageDeflate &&
280+
perMessageDeflate.params[
281+
perMessageDeflate._isServer
282+
? 'server_no_context_takeover'
283+
: 'client_no_context_takeover'
284+
]
285+
) {
278286
rsv1 = buf.length >= perMessageDeflate._threshold;
279287
}
280288
this._compress = rsv1;

‎test/permessage-deflate.test.js

+7-16
Original file line numberDiff line numberDiff line change
@@ -344,7 +344,7 @@ describe('PerMessageDeflate', () => {
344344

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

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

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

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

389389
it('works with the negotiated parameters', (done) => {
390390
const perMessageDeflate = new PerMessageDeflate({
391-
threshold: 0,
392391
memLevel: 5,
393392
level: 9
394393
});
@@ -415,11 +414,9 @@ describe('PerMessageDeflate', () => {
415414

416415
it('honors the `level` option', (done) => {
417416
const lev0 = new PerMessageDeflate({
418-
threshold: 0,
419417
zlibDeflateOptions: { level: 0 }
420418
});
421419
const lev9 = new PerMessageDeflate({
422-
threshold: 0,
423420
zlibDeflateOptions: { level: 9 }
424421
});
425422
const extensionStr =
@@ -459,7 +456,6 @@ describe('PerMessageDeflate', () => {
459456

460457
it('honors the `zlib{Deflate,Inflate}Options` option', (done) => {
461458
const lev0 = new PerMessageDeflate({
462-
threshold: 0,
463459
zlibDeflateOptions: {
464460
level: 0,
465461
chunkSize: 256
@@ -469,7 +465,6 @@ describe('PerMessageDeflate', () => {
469465
}
470466
});
471467
const lev9 = new PerMessageDeflate({
472-
threshold: 0,
473468
zlibDeflateOptions: {
474469
level: 9,
475470
chunkSize: 128
@@ -523,7 +518,7 @@ describe('PerMessageDeflate', () => {
523518
});
524519

525520
it("doesn't use contex takeover if not allowed", (done) => {
526-
const perMessageDeflate = new PerMessageDeflate({ threshold: 0 }, true);
521+
const perMessageDeflate = new PerMessageDeflate({}, true);
527522
const extensions = extension.parse(
528523
'permessage-deflate;server_no_context_takeover'
529524
);
@@ -554,7 +549,7 @@ describe('PerMessageDeflate', () => {
554549
});
555550

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

@@ -583,7 +578,7 @@ describe('PerMessageDeflate', () => {
583578
});
584579

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

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

598593
it("doesn't call the callback twice when `maxPayload` is exceeded", (done) => {
599-
const perMessageDeflate = new PerMessageDeflate(
600-
{ threshold: 0 },
601-
false,
602-
25
603-
);
594+
const perMessageDeflate = new PerMessageDeflate({}, false, 25);
604595
const buf = Buffer.from('A'.repeat(50));
605596

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

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

622613
perMessageDeflate.accept([{}]);

‎test/sender.test.js

+139-121
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
const assert = require('assert');
44

55
const PerMessageDeflate = require('../lib/permessage-deflate');
6+
const extension = require('../lib/extension');
67
const Sender = require('../lib/sender');
78

89
class MockSocket {
@@ -49,7 +50,7 @@ describe('Sender', () => {
4950

5051
describe('#send', () => {
5152
it('compresses data if compress option is enabled', (done) => {
52-
const perMessageDeflate = new PerMessageDeflate({ threshold: 0 });
53+
const perMessageDeflate = new PerMessageDeflate();
5354
let count = 0;
5455
const mockSocket = new MockSocket({
5556
write: (data) => {
@@ -71,139 +72,156 @@ describe('Sender', () => {
7172
sender.send('hi', options);
7273
});
7374

74-
it('does not compress data for small payloads', (done) => {
75-
const perMessageDeflate = new PerMessageDeflate();
76-
const mockSocket = new MockSocket({
77-
write: (data) => {
78-
assert.notStrictEqual(data[0] & 0x40, 0x40);
79-
done();
80-
}
81-
});
82-
const sender = new Sender(mockSocket, {
83-
'permessage-deflate': perMessageDeflate
84-
});
85-
86-
perMessageDeflate.accept([{}]);
75+
describe('when context takeover is disabled', () => {
76+
it('honors the compression threshold', (done) => {
77+
const perMessageDeflate = new PerMessageDeflate();
78+
const mockSocket = new MockSocket({
79+
write: (data) => {
80+
assert.notStrictEqual(data[0] & 0x40, 0x40);
81+
done();
82+
}
83+
});
84+
const sender = new Sender(mockSocket, {
85+
'permessage-deflate': perMessageDeflate
86+
});
87+
const extensions = extension.parse(
88+
'permessage-deflate; client_no_context_takeover'
89+
);
8790

88-
sender.send('hi', { compress: true, fin: true });
89-
});
91+
perMessageDeflate.accept(extensions['permessage-deflate']);
9092

91-
it('compresses all frames in a fragmented message', (done) => {
92-
const chunks = [];
93-
const perMessageDeflate = new PerMessageDeflate({ threshold: 3 });
94-
const mockSocket = new MockSocket({
95-
write: (chunk) => {
96-
chunks.push(chunk);
97-
if (chunks.length !== 4) return;
98-
99-
assert.strictEqual(chunks[0].length, 2);
100-
assert.strictEqual(chunks[0][0] & 0x40, 0x40);
101-
assert.strictEqual(chunks[1].length, 9);
102-
103-
assert.strictEqual(chunks[2].length, 2);
104-
assert.strictEqual(chunks[2][0] & 0x40, 0x00);
105-
assert.strictEqual(chunks[3].length, 4);
106-
done();
107-
}
108-
});
109-
const sender = new Sender(mockSocket, {
110-
'permessage-deflate': perMessageDeflate
93+
sender.send('hi', { compress: true, fin: true });
11194
});
11295

113-
perMessageDeflate.accept([{}]);
114-
115-
sender.send('123', { compress: true, fin: false });
116-
sender.send('12', { compress: true, fin: true });
117-
});
118-
119-
it('compresses no frames in a fragmented message', (done) => {
120-
const chunks = [];
121-
const perMessageDeflate = new PerMessageDeflate({ threshold: 3 });
122-
const mockSocket = new MockSocket({
123-
write: (chunk) => {
124-
chunks.push(chunk);
125-
if (chunks.length !== 4) return;
126-
127-
assert.strictEqual(chunks[0].length, 2);
128-
assert.strictEqual(chunks[0][0] & 0x40, 0x00);
129-
assert.strictEqual(chunks[1].length, 2);
130-
131-
assert.strictEqual(chunks[2].length, 2);
132-
assert.strictEqual(chunks[2][0] & 0x40, 0x00);
133-
assert.strictEqual(chunks[3].length, 3);
134-
done();
135-
}
136-
});
137-
const sender = new Sender(mockSocket, {
138-
'permessage-deflate': perMessageDeflate
96+
it('compresses all fragments of a fragmented message', (done) => {
97+
const chunks = [];
98+
const perMessageDeflate = new PerMessageDeflate({ threshold: 3 });
99+
const mockSocket = new MockSocket({
100+
write: (chunk) => {
101+
chunks.push(chunk);
102+
if (chunks.length !== 4) return;
103+
104+
assert.strictEqual(chunks[0].length, 2);
105+
assert.strictEqual(chunks[0][0] & 0x40, 0x40);
106+
assert.strictEqual(chunks[1].length, 9);
107+
108+
assert.strictEqual(chunks[2].length, 2);
109+
assert.strictEqual(chunks[2][0] & 0x40, 0x00);
110+
assert.strictEqual(chunks[3].length, 4);
111+
done();
112+
}
113+
});
114+
const sender = new Sender(mockSocket, {
115+
'permessage-deflate': perMessageDeflate
116+
});
117+
const extensions = extension.parse(
118+
'permessage-deflate; client_no_context_takeover'
119+
);
120+
121+
perMessageDeflate.accept(extensions['permessage-deflate']);
122+
123+
sender.send('123', { compress: true, fin: false });
124+
sender.send('12', { compress: true, fin: true });
139125
});
140126

141-
perMessageDeflate.accept([{}]);
142-
143-
sender.send('12', { compress: true, fin: false });
144-
sender.send('123', { compress: true, fin: true });
145-
});
146-
147-
it('compresses empty buffer as first fragment', (done) => {
148-
const chunks = [];
149-
const perMessageDeflate = new PerMessageDeflate({ threshold: 0 });
150-
const mockSocket = new MockSocket({
151-
write: (chunk) => {
152-
chunks.push(chunk);
153-
if (chunks.length !== 4) return;
154-
155-
assert.strictEqual(chunks[0].length, 2);
156-
assert.strictEqual(chunks[0][0] & 0x40, 0x40);
157-
assert.strictEqual(chunks[1].length, 5);
158-
159-
assert.strictEqual(chunks[2].length, 2);
160-
assert.strictEqual(chunks[2][0] & 0x40, 0x00);
161-
assert.strictEqual(chunks[3].length, 6);
162-
done();
163-
}
164-
});
165-
const sender = new Sender(mockSocket, {
166-
'permessage-deflate': perMessageDeflate
127+
it('does not compress any fragments of a fragmented message', (done) => {
128+
const chunks = [];
129+
const perMessageDeflate = new PerMessageDeflate({ threshold: 3 });
130+
const mockSocket = new MockSocket({
131+
write: (chunk) => {
132+
chunks.push(chunk);
133+
if (chunks.length !== 4) return;
134+
135+
assert.strictEqual(chunks[0].length, 2);
136+
assert.strictEqual(chunks[0][0] & 0x40, 0x00);
137+
assert.strictEqual(chunks[1].length, 2);
138+
139+
assert.strictEqual(chunks[2].length, 2);
140+
assert.strictEqual(chunks[2][0] & 0x40, 0x00);
141+
assert.strictEqual(chunks[3].length, 3);
142+
done();
143+
}
144+
});
145+
const sender = new Sender(mockSocket, {
146+
'permessage-deflate': perMessageDeflate
147+
});
148+
const extensions = extension.parse(
149+
'permessage-deflate; client_no_context_takeover'
150+
);
151+
152+
perMessageDeflate.accept(extensions['permessage-deflate']);
153+
154+
sender.send('12', { compress: true, fin: false });
155+
sender.send('123', { compress: true, fin: true });
167156
});
168157

169-
perMessageDeflate.accept([{}]);
170-
171-
sender.send(Buffer.alloc(0), { compress: true, fin: false });
172-
sender.send('data', { compress: true, fin: true });
173-
});
174-
175-
it('compresses empty buffer as last fragment', (done) => {
176-
const chunks = [];
177-
const perMessageDeflate = new PerMessageDeflate({ threshold: 0 });
178-
const mockSocket = new MockSocket({
179-
write: (chunk) => {
180-
chunks.push(chunk);
181-
if (chunks.length !== 4) return;
182-
183-
assert.strictEqual(chunks[0].length, 2);
184-
assert.strictEqual(chunks[0][0] & 0x40, 0x40);
185-
assert.strictEqual(chunks[1].length, 10);
186-
187-
assert.strictEqual(chunks[2].length, 2);
188-
assert.strictEqual(chunks[2][0] & 0x40, 0x00);
189-
assert.strictEqual(chunks[3].length, 1);
190-
done();
191-
}
192-
});
193-
const sender = new Sender(mockSocket, {
194-
'permessage-deflate': perMessageDeflate
158+
it('compresses empty buffer as first fragment', (done) => {
159+
const chunks = [];
160+
const perMessageDeflate = new PerMessageDeflate({ threshold: 0 });
161+
const mockSocket = new MockSocket({
162+
write: (chunk) => {
163+
chunks.push(chunk);
164+
if (chunks.length !== 4) return;
165+
166+
assert.strictEqual(chunks[0].length, 2);
167+
assert.strictEqual(chunks[0][0] & 0x40, 0x40);
168+
assert.strictEqual(chunks[1].length, 5);
169+
170+
assert.strictEqual(chunks[2].length, 2);
171+
assert.strictEqual(chunks[2][0] & 0x40, 0x00);
172+
assert.strictEqual(chunks[3].length, 6);
173+
done();
174+
}
175+
});
176+
const sender = new Sender(mockSocket, {
177+
'permessage-deflate': perMessageDeflate
178+
});
179+
const extensions = extension.parse(
180+
'permessage-deflate; client_no_context_takeover'
181+
);
182+
183+
perMessageDeflate.accept(extensions['permessage-deflate']);
184+
185+
sender.send(Buffer.alloc(0), { compress: true, fin: false });
186+
sender.send('data', { compress: true, fin: true });
195187
});
196188

197-
perMessageDeflate.accept([{}]);
198-
199-
sender.send('data', { compress: true, fin: false });
200-
sender.send(Buffer.alloc(0), { compress: true, fin: true });
189+
it('compresses empty buffer as last fragment', (done) => {
190+
const chunks = [];
191+
const perMessageDeflate = new PerMessageDeflate({ threshold: 0 });
192+
const mockSocket = new MockSocket({
193+
write: (chunk) => {
194+
chunks.push(chunk);
195+
if (chunks.length !== 4) return;
196+
197+
assert.strictEqual(chunks[0].length, 2);
198+
assert.strictEqual(chunks[0][0] & 0x40, 0x40);
199+
assert.strictEqual(chunks[1].length, 10);
200+
201+
assert.strictEqual(chunks[2].length, 2);
202+
assert.strictEqual(chunks[2][0] & 0x40, 0x00);
203+
assert.strictEqual(chunks[3].length, 1);
204+
done();
205+
}
206+
});
207+
const sender = new Sender(mockSocket, {
208+
'permessage-deflate': perMessageDeflate
209+
});
210+
const extensions = extension.parse(
211+
'permessage-deflate; client_no_context_takeover'
212+
);
213+
214+
perMessageDeflate.accept(extensions['permessage-deflate']);
215+
216+
sender.send('data', { compress: true, fin: false });
217+
sender.send(Buffer.alloc(0), { compress: true, fin: true });
218+
});
201219
});
202220
});
203221

204222
describe('#ping', () => {
205223
it('works with multiple types of data', (done) => {
206-
const perMessageDeflate = new PerMessageDeflate({ threshold: 0 });
224+
const perMessageDeflate = new PerMessageDeflate();
207225
let count = 0;
208226
const mockSocket = new MockSocket({
209227
write: (data) => {
@@ -235,7 +253,7 @@ describe('Sender', () => {
235253

236254
describe('#pong', () => {
237255
it('works with multiple types of data', (done) => {
238-
const perMessageDeflate = new PerMessageDeflate({ threshold: 0 });
256+
const perMessageDeflate = new PerMessageDeflate();
239257
let count = 0;
240258
const mockSocket = new MockSocket({
241259
write: (data) => {
@@ -292,7 +310,7 @@ describe('Sender', () => {
292310
});
293311

294312
it('should consume all data before closing', (done) => {
295-
const perMessageDeflate = new PerMessageDeflate({ threshold: 0 });
313+
const perMessageDeflate = new PerMessageDeflate();
296314

297315
let count = 0;
298316
const mockSocket = new MockSocket({

0 commit comments

Comments
 (0)
Please sign in to comment.