Skip to content

Commit

Permalink
Use response instead of request when marshalling (#4162)
Browse files Browse the repository at this point in the history
* Use response instead of request when marshalling. Closes #4161

* Cleanup

* Don't change internals.fail()

* Fix info.responding being set on remotely closed requests

* More cleanup
  • Loading branch information
kanongil authored Sep 24, 2020
1 parent 147b6fd commit 06d2723
Show file tree
Hide file tree
Showing 6 changed files with 64 additions and 25 deletions.
3 changes: 2 additions & 1 deletion lib/auth.js
Original file line number Diff line number Diff line change
Expand Up @@ -392,8 +392,9 @@ exports = module.exports = internals.Auth = class {
return response;
}

static async response(request) {
static async response(response) {

const request = response.request;
const auth = request._core.auth;
if (!request.auth.isAuthenticated) {
return;
Expand Down
4 changes: 2 additions & 2 deletions lib/cors.js
Original file line number Diff line number Diff line change
Expand Up @@ -154,10 +154,10 @@ internals.handler = function (request, h) {
};


exports.headers = function (request) {
exports.headers = function (response) {

const request = response.request;
const settings = request.route.settings.cors;
const response = request.response;

if (settings._origin !== false) {
response.vary('origin');
Expand Down
27 changes: 13 additions & 14 deletions lib/headers.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,9 @@ const Boom = require('@hapi/boom');
const internals = {};


exports.cache = function (request) {
exports.cache = function (response) {

const response = request.response;
const request = response.request;
if (response.headers['cache-control']) {
return;
}
Expand All @@ -32,9 +32,9 @@ exports.cache = function (request) {
};


exports.content = async function (request) {
exports.content = async function (response) {

const response = request.response;
const request = response.request;
if (response._isPayloadSupported() ||
request.method === 'head') {

Expand All @@ -59,7 +59,7 @@ exports.content = async function (request) {
response._payload = new internals.Empty(); // Set empty stream
}

exports.type(request);
exports.type(response);
}
else {

Expand All @@ -72,9 +72,9 @@ exports.content = async function (request) {
};


exports.state = async function (request) {
exports.state = async function (response) {

const response = request.response;
const request = response.request;
const states = [];

for (const stateName in request._states) {
Expand Down Expand Up @@ -118,24 +118,23 @@ exports.state = async function (request) {
};


exports.type = function (request) {
exports.type = function (response) {

const response = request.response;
const type = response.contentType;
if (type !== null && type !== response.headers['content-type']) {
response.type(type);
}
};


exports.entity = function (request) {
exports.entity = function (response) {

const request = response.request;

if (!request._entity) {
return;
}

const response = request.response;

if (request._entity.etag &&
!response.headers.etag) {

Expand All @@ -150,9 +149,9 @@ exports.entity = function (request) {
};


exports.unmodified = function (request) {
exports.unmodified = function (response) {

const response = request.response;
const request = response.request;
if (response.statusCode === 304) {
return;
}
Expand Down
3 changes: 1 addition & 2 deletions lib/security.js
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,8 @@ exports.route = function (settings) {
};


exports.headers = function (request) {
exports.headers = function (response) {

const response = request.response;
const security = response.request.route.settings.security;

if (security._hsts) {
Expand Down
12 changes: 6 additions & 6 deletions lib/transmit.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ exports.send = async function (request) {
return;
}

await internals.marshal(request);
await internals.marshal(response);
await internals.transmit(response);
}
catch (err) {
Expand All @@ -35,10 +35,10 @@ exports.send = async function (request) {
};


internals.marshal = async function (request) {
internals.marshal = async function (response) {

for (const func of request._route._marshalCycle) {
await func(request);
for (const func of response.request._route._marshalCycle) {
await func(response);
}
};

Expand All @@ -49,7 +49,7 @@ internals.fail = async function (request, boom) {
request.response = response; // Not using request._setResponse() to avoid double log

try {
await internals.marshal(request);
await internals.marshal(response);
}
catch (err) {
Bounce.rethrow(err, 'system');
Expand Down Expand Up @@ -278,7 +278,7 @@ internals.end = function (env, event, err) {
env.team = null;

if (request.raw.res.finished) {
if (event !== 'aborted') {
if (!event) {
request.info.responded = Date.now();
}

Expand Down
40 changes: 40 additions & 0 deletions test/transmit.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

const ChildProcess = require('child_process');
const Fs = require('fs');
const Net = require('net');
const Path = require('path');
const Stream = require('stream');
const Zlib = require('zlib');
Expand Down Expand Up @@ -531,6 +532,45 @@ describe('transmission', () => {
expect(res.statusCode).to.equal(200);
expect(res.headers['cache-control']).to.be.undefined();
});

it('does not crash when request is aborted', async () => {

const server = Hapi.server();
server.route({ method: 'GET', path: '/', handler: () => 'ok' });

const team = new Teamwork.Team();
const onRequest = (request, h) => {

request.events.once('disconnect', () => team.attend());
return h.continue;
};

server.ext('onRequest', onRequest);

// Use state autoValue function to intercept marshal stage

server.state('always', {
autoValue() {

client.destroy();
return team.work; // Continue once the request has been aborted
}
});

await server.start();

const log = server.events.once('response');
const client = Net.connect(server.info.port, () => {

client.write('GET / HTTP/1.1\r\n\r\n');
});

const [request] = await log;
expect(request.response.isBoom).to.be.true();
expect(request.response.output.statusCode).to.equal(499);
expect(request.info.completed).to.be.above(0);
expect(request.info.responded).to.equal(0);
});
});

describe('transmit()', () => {
Expand Down

0 comments on commit 06d2723

Please sign in to comment.