Skip to content

Commit 41ac5cc

Browse files
robrichardrobrichard
committed
fix handling partial and combined chunks
1 parent 04ac283 commit 41ac5cc

File tree

4 files changed

+181
-72
lines changed

4 files changed

+181
-72
lines changed

‎src/PatchResolver.js

Lines changed: 20 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { parseMultipartHTTP } from './parseMultipartHTTP';
1+
import { parseMultipartHttp } from './parseMultipartHttp';
22

33
// recursive function to apply the patch to the previous response
44
function applyPatch(previousResponse, patchPath, patchData) {
@@ -29,35 +29,30 @@ function mergeErrors(previousErrors, patchErrors) {
2929
export function PatchResolver({ onResponse }) {
3030
this.onResponse = onResponse;
3131
this.previousResponse = null;
32-
this.chunkBuffer = '';
3332
this.processedChunks = 0;
33+
this.chunkBuffer = '';
3434
}
3535

3636
PatchResolver.prototype.handleChunk = function(data) {
37-
const results = parseMultipartHTTP(this.chunkBuffer + data);
38-
if (results === null) {
39-
// The part is not complete yet, add it to the buffer
40-
// and wait for the next chunk to arrive
41-
this.chunkBuffer += data;
42-
} else {
43-
this.chunkBuffer = ''; // Reset
44-
for (const part of results) {
45-
if (this.processedChunks === 0) {
46-
this.previousResponse = part;
47-
this.onResponse(this.previousResponse);
48-
} else {
49-
if (!(Array.isArray(part.path) && typeof part.data !== 'undefined')) {
50-
throw new Error('invalid patch format ' + JSON.stringify(part, null, 2));
51-
}
52-
this.previousResponse = {
53-
...this.previousResponse,
54-
data: applyPatch(this.previousResponse.data, part.path, part.data),
55-
errors: mergeErrors(this.previousResponse.errors, part.errors),
56-
};
57-
58-
this.onResponse(this.previousResponse);
37+
this.chunkBuffer += data;
38+
const { newBuffer, parts } = parseMultipartHttp(this.chunkBuffer);
39+
this.chunkBuffer = newBuffer;
40+
for (const part of parts) {
41+
if (this.processedChunks === 0) {
42+
this.previousResponse = part;
43+
this.onResponse(this.previousResponse);
44+
} else {
45+
if (!(Array.isArray(part.path) && typeof part.data !== 'undefined')) {
46+
throw new Error('invalid patch format ' + JSON.stringify(part, null, 2));
5947
}
60-
this.processedChunks += 1;
48+
this.previousResponse = {
49+
...this.previousResponse,
50+
data: applyPatch(this.previousResponse.data, part.path, part.data),
51+
errors: mergeErrors(this.previousResponse.errors, part.errors),
52+
};
53+
54+
this.onResponse(this.previousResponse);
6155
}
56+
this.processedChunks += 1;
6257
}
6358
};

‎src/__test__/PatchResolver.spec.js

Lines changed: 93 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ const chunk3 = [
4444
'',
4545
'{"path":["viewer","user","profile"],"data":{"displayName":"Steven Seagal"}}\n',
4646
'',
47-
'-----',
47+
'-----\r\n',
4848
].join('\r\n');
4949

5050
describe('PathResolver', function() {
@@ -86,19 +86,24 @@ describe('PathResolver', function() {
8686
onResponse,
8787
});
8888

89+
console.log('chunk1.length', chunk1.length);
90+
8991
const chunk1a = chunk1.substr(0, 35);
90-
const chunk1b = chunk1.substr(36);
92+
const chunk1b = chunk1.substr(35, 80);
93+
const chunk1c = chunk1.substr(35 + 80);
9194

9295
resolver.handleChunk(chunk1a);
9396
expect(onResponse).not.toHaveBeenCalled();
9497
resolver.handleChunk(chunk1b);
98+
expect(onResponse).not.toHaveBeenCalled();
99+
resolver.handleChunk(chunk1c);
95100
expect(onResponse).toHaveBeenCalledWith({
96101
data: { viewer: { currencies: null, user: { profile: null } } },
97102
});
98103
onResponse.mockClear();
99104

100105
const chunk2a = chunk2.substr(0, 35);
101-
const chunk2b = chunk2.substr(36);
106+
const chunk2b = chunk2.substr(35);
102107

103108
resolver.handleChunk(chunk2a);
104109
expect(onResponse).not.toHaveBeenCalled();
@@ -115,7 +120,7 @@ describe('PathResolver', function() {
115120

116121
const chunk3a = chunk3.substr(0, 10);
117122
const chunk3b = chunk3.substr(11, 20);
118-
const chunk3c = chunk3.substr(21);
123+
const chunk3c = chunk3.substr(11 + 20);
119124

120125
resolver.handleChunk(chunk3a);
121126
expect(onResponse).not.toHaveBeenCalled();
@@ -132,6 +137,89 @@ describe('PathResolver', function() {
132137
});
133138
});
134139

140+
it('should work when chunks are combined', function() {
141+
const onResponse = jest.fn();
142+
const resolver = new PatchResolver({
143+
onResponse,
144+
});
145+
146+
resolver.handleChunk(chunk1 + chunk2);
147+
expect(onResponse.mock.calls[0][0]).toEqual({
148+
data: { viewer: { currencies: null, user: { profile: null } } },
149+
});
150+
expect(onResponse.mock.calls[1][0]).toEqual({
151+
data: {
152+
viewer: {
153+
currencies: ['USD', 'GBP', 'EUR', 'CAD', 'AUD', 'CHF', 'MXN'],
154+
user: { profile: null },
155+
},
156+
},
157+
});
158+
});
159+
160+
it('should work when chunks are combined and split', function() {
161+
const onResponse = jest.fn();
162+
const resolver = new PatchResolver({
163+
onResponse,
164+
});
165+
166+
const chunk3a = chunk3.substr(0, 10);
167+
const chunk3b = chunk3.substr(11, 20);
168+
const chunk3c = chunk3.substr(11 + 20);
169+
170+
resolver.handleChunk(chunk1 + chunk2 + chunk3a);
171+
expect(onResponse.mock.calls[0][0]).toEqual({
172+
data: { viewer: { currencies: null, user: { profile: null } } },
173+
});
174+
expect(onResponse.mock.calls[1][0]).toEqual({
175+
data: {
176+
viewer: {
177+
currencies: ['USD', 'GBP', 'EUR', 'CAD', 'AUD', 'CHF', 'MXN'],
178+
user: { profile: null },
179+
},
180+
},
181+
});
182+
onResponse.mockClear();
183+
184+
resolver.handleChunk(chunk3b);
185+
expect(onResponse).not.toHaveBeenCalled();
186+
resolver.handleChunk(chunk3c);
187+
expect(onResponse).toHaveBeenCalledWith({
188+
data: {
189+
viewer: {
190+
currencies: ['USD', 'GBP', 'EUR', 'CAD', 'AUD', 'CHF', 'MXN'],
191+
user: { profile: { displayName: 'Steven Seagal' } },
192+
},
193+
},
194+
});
195+
});
196+
197+
it('should work when chunks are combined across boundaries', function() {
198+
const onResponse = jest.fn();
199+
const resolver = new PatchResolver({
200+
onResponse,
201+
});
202+
203+
const chunk2a = chunk2.substring(0, 35);
204+
const chunk2b = chunk2.substring(35);
205+
206+
resolver.handleChunk(chunk1 + chunk2a);
207+
expect(onResponse).toHaveBeenCalledWith({
208+
data: { viewer: { currencies: null, user: { profile: null } } },
209+
});
210+
onResponse.mockClear();
211+
resolver.handleChunk(chunk2b);
212+
213+
expect(onResponse).toHaveBeenCalledWith({
214+
data: {
215+
viewer: {
216+
currencies: ['USD', 'GBP', 'EUR', 'CAD', 'AUD', 'CHF', 'MXN'],
217+
user: { profile: null },
218+
},
219+
},
220+
});
221+
});
222+
135223
it('should merge errors', function() {
136224
const onResponse = jest.fn();
137225
const resolver = new PatchResolver({
@@ -166,4 +254,5 @@ describe('PathResolver', function() {
166254
errors: [{ message: 'Very Bad Error' }, { message: 'Not So Bad Error' }],
167255
});
168256
});
257+
it('should work', function() {});
169258
});

‎src/parseMultipartHTTP.js

Lines changed: 0 additions & 43 deletions
This file was deleted.

‎src/parseMultipartHttp.js

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
const boundary = '\r\n---\r\n';
2+
const terminatingBoundary = '\r\n-----\r\n';
3+
4+
function splitWithRest(string, delim) {
5+
const index = string.indexOf(delim);
6+
if (index < 0) {
7+
return [string];
8+
}
9+
return [string.substring(0, index), string.substring(index + delim.length)];
10+
}
11+
12+
export function parseMultipartHttp(buffer, previousParts = []) {
13+
let [, rest] = splitWithRest(buffer, boundary);
14+
if (!(rest && rest.length)) {
15+
// we did not finish receiving the initial boundary
16+
return {
17+
newBuffer: buffer,
18+
parts: previousParts,
19+
};
20+
}
21+
const parts = splitWithRest(rest, '\r\n\r\n');
22+
const headers = parts[0];
23+
rest = parts[1];
24+
25+
if (!(rest && rest.length)) {
26+
// we did not finish receiving the headers
27+
return {
28+
newBuffer: buffer,
29+
parts: previousParts,
30+
};
31+
}
32+
33+
const headersArr = headers.split('\r\n');
34+
const contentLengthHeader = headersArr.find(
35+
headerLine => headerLine.toLowerCase().indexOf('content-length:') >= 0
36+
);
37+
if (contentLengthHeader === undefined) {
38+
throw new Error('Invalid MultiPart Response, no content-length header');
39+
}
40+
const contentLengthArr = contentLengthHeader.split(':');
41+
let contentLength;
42+
if (contentLengthArr.length === 2 && !isNaN(parseInt(contentLengthArr[1]))) {
43+
contentLength = parseInt(contentLengthArr[1]);
44+
} else {
45+
throw new Error('Invalid MultiPart Response, could not parse content-length');
46+
}
47+
48+
// Strip out the terminating boundary
49+
rest = rest.replace(terminatingBoundary, '');
50+
51+
if (rest.length < contentLength) {
52+
// still waiting for more body to be sent;
53+
return {
54+
newBuffer: buffer,
55+
parts: previousParts,
56+
};
57+
}
58+
59+
const body = rest.substring(0, contentLength);
60+
const nextBuffer = rest.substring(contentLength);
61+
const part = JSON.parse(body);
62+
const newParts = [...previousParts, part];
63+
64+
if (nextBuffer.length) {
65+
return parseMultipartHttp(nextBuffer, newParts);
66+
}
67+
return { parts: newParts, newBuffer: '' };
68+
}

0 commit comments

Comments
 (0)