Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Remove custom output parsing and buffer payload chunks via write() #79

Merged
merged 8 commits into from Sep 23, 2016
Merged

Remove custom output parsing and buffer payload chunks via write() #79

Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
846a996
Remove custom parsing and buffer payload chunks
mtharrison Sep 9, 2016
ffdebb6
Use _shot namespace
mtharrison Sep 9, 2016
7e66b76
Write into null socket
mtharrison Sep 9, 2016
5ca963c
ES6 class syntax
mtharrison Sep 19, 2016
6b4b435
Object.assign()
mtharrison Sep 19, 2016
75eb823
Styling
mtharrison Sep 19, 2016
bd385e3
Remove internals.Response
mtharrison Sep 19, 2016
4bffff1
Added missing super call
mtharrison Sep 19, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
100 changes: 16 additions & 84 deletions lib/response.js
View file
Open in desktop
Expand Up @@ -14,6 +14,8 @@ exports = module.exports = internals.Response = function (req, onEnd) {

Http.ServerResponse.call(this, { method: req.method, httpVersionMajor: 1, httpVersionMinor: 1 });

this.shot = { trailers: {}, payloadChunks: [] };
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If someone feels squeamish about extending a builtin here, we could use a WeakMap instead.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't mind this. Maybe privatize it with an _? Also, any reason headers don't also live in here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The response._headers is keeping the contract with the Node object, and I didn't see the need to duplicate them.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ohhp, I see now!


this.once('finish', () => {

const res = internals.payload(this);
Expand Down Expand Up @@ -53,6 +55,7 @@ internals.Response.prototype.writeHead = function () {

internals.Response.prototype.write = function (data, encoding) {

this.shot.payloadChunks.push(new Buffer(data, encoding));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does bother me a little bit that the ServerResponse will also keep its own buffer (as long as it thinks it doesn't have a socket), so that data will be duplicated in memory.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you think of 7e66b76 ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It excites me.

Http.ServerResponse.prototype.write.call(this, data, encoding);
return true; // Write always returns false when disconnected
};
Expand All @@ -70,6 +73,14 @@ internals.Response.prototype.destroy = function () {
};


internals.Response.prototype.addTrailers = function (trailers) {

for (const key in trailers) {
this.shot.trailers[key.toLowerCase().trim()] = trailers[key].toString().trim();
}
};


internals.payload = function (response) {

// Prepare response object
Expand All @@ -84,91 +95,12 @@ internals.payload = function (response) {
trailers: {}
};

// Read payload

const raw = [];
let rawLength = 0;
for (let i = 0; i < response.output.length; ++i) {
const chunk = (response.output[i] instanceof Buffer ? response.output[i] : new Buffer(response.output[i], response.outputEncodings[i]));
raw.push(chunk);
rawLength = rawLength + chunk.length;
}

const rawBuffer = Buffer.concat(raw, rawLength);

// Parse payload

res.payload = '';

const CRLF = '\r\n';
const sep = new Buffer(CRLF + CRLF);
const parts = internals.splitBufferInTwo(rawBuffer, sep);
const payloadBuffer = parts[1];

if (!res.headers['transfer-encoding']) {
res.rawPayload = payloadBuffer;
res.payload = payloadBuffer.toString();
return res;
}

const CRLFBuffer = new Buffer(CRLF);
let rest = payloadBuffer;
let payloadBytes = [];
let size;
do {
const payloadParts = internals.splitBufferInTwo(rest, CRLFBuffer);
const next = payloadParts[1];
size = parseInt(payloadParts[0].toString(), 16);
if (size === 0) {
rest = next;
}
else {
const nextData = next.slice(0, size);
payloadBytes = payloadBytes.concat(Array.prototype.slice.call(nextData, 0));
rest = next.slice(size + 2);
}
}
while (size);

res.rawPayload = new Buffer(payloadBytes);
res.payload = res.rawPayload.toString('utf8');
// Prepare payload and trailers

// Parse trailers

const trailerLines = rest.toString().split(CRLF);
trailerLines.forEach((line) => {

const trailerParts = line.split(':');
if (trailerParts.length === 2) {
res.trailers[trailerParts[0].trim().toLowerCase()] = trailerParts[1].trim();
}
});
const rawBuffer = Buffer.concat(response.shot.payloadChunks);
res.rawPayload = rawBuffer;
res.payload = rawBuffer.toString();
res.trailers = response.shot.trailers;

return res;
};


internals.splitBufferInTwo = function (buffer, seperator) {

for (let i = 0; i < buffer.length - seperator.length; ++i) {
if (internals.bufferEqual(buffer.slice(i, i + seperator.length), seperator)) {
const part1 = buffer.slice(0, i);
const part2 = buffer.slice(i + seperator.length);
return [part1, part2];
}
}

return [buffer, new Buffer(0)];
};


internals.bufferEqual = function (a, b) {

for (let i = 0; i < a.length; ++i) {
if (a[i] !== b[i]) {
return false;
}
}

return true;
};