Skip to content

Commit

Permalink
Change default empty to 204. Closes #3919
Browse files Browse the repository at this point in the history
  • Loading branch information
hueniverse committed Sep 16, 2019
1 parent 2cc43c0 commit cf81fc8
Show file tree
Hide file tree
Showing 14 changed files with 123 additions and 98 deletions.
2 changes: 1 addition & 1 deletion lib/config.js
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ internals.routeBase = Joi.object({
plugins: Joi.object(),
response: Joi.object({
disconnectStatusCode: Joi.number().integer().min(400).default(499),
emptyStatusCode: Joi.valid(200, 204).default(200),
emptyStatusCode: Joi.valid(200, 204).default(204),
failAction: internals.failAction,
modify: Joi.boolean(),
options: Joi.object(),
Expand Down
12 changes: 10 additions & 2 deletions lib/response.js
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ exports = module.exports = internals.Response = class {
if (this.variety === 'plain' &&
this.source !== null) {

this._contentType = (typeof this.source === 'string' ? 'text/html' : 'application/json');
this._contentType = typeof this.source === 'string' ? 'text/html' : 'application/json';
}
}

Expand Down Expand Up @@ -710,7 +710,15 @@ internals.Response.Payload = class extends Stream.Readable {

this._sizeOffset = this._sizeOffset + variable.length + 7;
this._prefix = '/**/' + variable + '('; // '/**/' prefix prevents CVE-2014-4671 security exploit
this._data = (this._data === null || Buffer.isBuffer(this._data)) ? this._data : this._data.replace(/\u2028/g, '\\u2028').replace(/\u2029/g, '\\u2029');

if (this._data !== null &&
!Buffer.isBuffer(this._data)) {

this._data = this._data
.replace(/\u2028/g, '\\u2028')
.replace(/\u2029/g, '\\u2029');
}

this._suffix = ');';
}

Expand Down
2 changes: 1 addition & 1 deletion lib/transmit.js
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ internals.length = function (response) {
if (length === 0 &&
!response._statusCode &&
response.statusCode === 200 &&
request.route.settings.response.emptyStatusCode === 204) {
request.route.settings.response.emptyStatusCode !== 200) {

response.code(204);
delete response.headers['content-length'];
Expand Down
62 changes: 31 additions & 31 deletions test/auth.js
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ describe('authentication', () => {
expect(res1.statusCode).to.equal(401);

const res2 = await server.inject({ url: '/', headers: { authorization: 'Custom steve' } });
expect(res2.statusCode).to.equal(200);
expect(res2.statusCode).to.equal(204);
});

it('uses views', async () => {
Expand Down Expand Up @@ -246,7 +246,7 @@ describe('authentication', () => {
expect(res1.statusCode).to.equal(401);

const res2 = await server.inject({ url: '/', headers: { authorization: 'Custom steve' } });
expect(res2.statusCode).to.equal(200);
expect(res2.statusCode).to.equal(204);
});

it('sets default with object', async () => {
Expand All @@ -261,7 +261,7 @@ describe('authentication', () => {
expect(res1.statusCode).to.equal(401);

const res2 = await server.inject({ url: '/', headers: { authorization: 'Custom steve' } });
expect(res2.statusCode).to.equal(200);
expect(res2.statusCode).to.equal(204);
});

it('throws when setting default twice', () => {
Expand Down Expand Up @@ -482,7 +482,7 @@ describe('authentication', () => {
});

const res = await server.inject({ url: '/', headers: { authorization: 'Custom steve' } });
expect(res.statusCode).to.equal(200);
expect(res.statusCode).to.equal(204);
});

it('authenticates a request with auth strategy name config', async () => {
Expand All @@ -500,7 +500,7 @@ describe('authentication', () => {
});

const res = await server.inject({ url: '/', headers: { authorization: 'Custom steve' } });
expect(res.statusCode).to.equal(200);
expect(res.statusCode).to.equal(204);
});

it('tries to authenticate a request', async () => {
Expand Down Expand Up @@ -610,7 +610,7 @@ describe('authentication', () => {
});

const res = await server.inject({ url: '/', headers: { authorization: 'Custom steve' } });
expect(res.statusCode).to.equal(200);
expect(res.statusCode).to.equal(204);
});

it('matches scope (array to array)', async () => {
Expand All @@ -631,7 +631,7 @@ describe('authentication', () => {
});

const res = await server.inject({ url: '/', headers: { authorization: 'Custom steve' } });
expect(res.statusCode).to.equal(200);
expect(res.statusCode).to.equal(204);
});

it('matches scope (single to array)', async () => {
Expand All @@ -652,7 +652,7 @@ describe('authentication', () => {
});

const res = await server.inject({ url: '/', headers: { authorization: 'Custom steve' } });
expect(res.statusCode).to.equal(200);
expect(res.statusCode).to.equal(204);
});

it('matches scope (single to single)', async () => {
Expand All @@ -673,7 +673,7 @@ describe('authentication', () => {
});

const res = await server.inject({ url: '/', headers: { authorization: 'Custom steve' } });
expect(res.statusCode).to.equal(200);
expect(res.statusCode).to.equal(204);
});

it('matches dynamic scope (single to single)', async () => {
Expand All @@ -694,7 +694,7 @@ describe('authentication', () => {
});

const res = await server.inject({ url: '/test', headers: { authorization: 'Custom steve' } });
expect(res.statusCode).to.equal(200);
expect(res.statusCode).to.equal(204);
});

it('matches multiple required dynamic scopes', async () => {
Expand All @@ -715,7 +715,7 @@ describe('authentication', () => {
});

const res = await server.inject({ url: '/test', headers: { authorization: 'Custom steve' } });
expect(res.statusCode).to.equal(200);
expect(res.statusCode).to.equal(204);
});

it('matches multiple required dynamic scopes (mixed types)', async () => {
Expand All @@ -736,7 +736,7 @@ describe('authentication', () => {
});

const res = await server.inject({ url: '/test', headers: { authorization: 'Custom steve' } });
expect(res.statusCode).to.equal(200);
expect(res.statusCode).to.equal(204);
});

it('matches dynamic scope with multiple parts (single to single)', async () => {
Expand All @@ -757,7 +757,7 @@ describe('authentication', () => {
});

const res = await server.inject({ url: '/test/admin', headers: { authorization: 'Custom steve' } });
expect(res.statusCode).to.equal(200);
expect(res.statusCode).to.equal(204);
});

it('does not match broken dynamic scope (single to single)', async () => {
Expand Down Expand Up @@ -834,7 +834,7 @@ describe('authentication', () => {
});

const res = await server.inject({ url: '/', headers: { authorization: 'Custom steve' } });
expect(res.statusCode).to.equal(200);
expect(res.statusCode).to.equal(204);
});

it('errors on missing scope', async () => {
Expand Down Expand Up @@ -910,7 +910,7 @@ describe('authentication', () => {
expect(res1.result.message).to.equal('Insufficient scope');

const res2 = await server.inject({ url: '/', headers: { authorization: 'Custom john' } });
expect(res2.statusCode).to.equal(200);
expect(res2.statusCode).to.equal(204);
});

it('validates forbidden scope', async () => {
Expand Down Expand Up @@ -942,7 +942,7 @@ describe('authentication', () => {
expect(res1.result.message).to.equal('Insufficient scope');

const res2 = await server.inject({ url: '/', headers: { authorization: 'Custom john' } });
expect(res2.statusCode).to.equal(200);
expect(res2.statusCode).to.equal(204);
});

it('validates complex scope', async () => {
Expand Down Expand Up @@ -977,10 +977,10 @@ describe('authentication', () => {
expect(res1.result.message).to.equal('Insufficient scope');

const res2 = await server.inject({ url: '/', headers: { authorization: 'Custom john' } });
expect(res2.statusCode).to.equal(200);
expect(res2.statusCode).to.equal(204);

const res3 = await server.inject({ url: '/', headers: { authorization: 'Custom mary' } });
expect(res3.statusCode).to.equal(200);
expect(res3.statusCode).to.equal(204);

const res4 = await server.inject({ url: '/', headers: { authorization: 'Custom lucy' } });
expect(res4.statusCode).to.equal(403);
Expand Down Expand Up @@ -1035,7 +1035,7 @@ describe('authentication', () => {
});

const res = await server.inject({ url: '/', headers: { authorization: 'Custom steve' } });
expect(res.statusCode).to.equal(200);
expect(res.statusCode).to.equal(204);
});

it('matches scope (access single)', async () => {
Expand Down Expand Up @@ -1091,7 +1091,7 @@ describe('authentication', () => {
});

const res = await server.inject({ url: '/', headers: { authorization: 'Custom steve' } });
expect(res.statusCode).to.equal(200);
expect(res.statusCode).to.equal(204);
});

it('errors on matching scope (access array)', async () => {
Expand Down Expand Up @@ -1139,7 +1139,7 @@ describe('authentication', () => {
});

const res = await server.inject({ url: '/', headers: { authorization: 'Custom steve' } });
expect(res.statusCode).to.equal(200);
expect(res.statusCode).to.equal(204);
});

it('matches user entity', async () => {
Expand All @@ -1160,7 +1160,7 @@ describe('authentication', () => {
});

const res = await server.inject({ url: '/', headers: { authorization: 'Custom steve' } });
expect(res.statusCode).to.equal(200);
expect(res.statusCode).to.equal(204);
});

it('errors on missing user entity', async () => {
Expand Down Expand Up @@ -1203,7 +1203,7 @@ describe('authentication', () => {
});

const res = await server.inject({ url: '/', headers: { authorization: 'Custom client' } });
expect(res.statusCode).to.equal(200);
expect(res.statusCode).to.equal(204);
});

it('errors on missing app entity', async () => {
Expand Down Expand Up @@ -1327,7 +1327,7 @@ describe('authentication', () => {
server.route({ method: 'GET', path: '/', handler: () => null });

const res = await server.inject('/');
expect(res.statusCode).to.equal(200);
expect(res.statusCode).to.equal(204);
});
});

Expand Down Expand Up @@ -1468,7 +1468,7 @@ describe('authentication', () => {
});

const res = await server.inject({ method: 'POST', url: '/', headers: { authorization: 'Custom validPayload' } });
expect(res.statusCode).to.equal(200);
expect(res.statusCode).to.equal(204);
});

it('skips when scheme does not support it', async () => {
Expand All @@ -1486,7 +1486,7 @@ describe('authentication', () => {
});

const res = await server.inject({ method: 'POST', url: '/', headers: { authorization: 'Custom validPayload' } });
expect(res.statusCode).to.equal(200);
expect(res.statusCode).to.equal(204);
});

it('authenticates request payload (required scheme)', async () => {
Expand All @@ -1505,7 +1505,7 @@ describe('authentication', () => {
});

const res = await server.inject({ method: 'POST', url: '/', headers: { authorization: 'Custom validPayload' } });
expect(res.statusCode).to.equal(200);
expect(res.statusCode).to.equal(204);
});

it('authenticates request payload (required scheme and required route)', async () => {
Expand All @@ -1526,7 +1526,7 @@ describe('authentication', () => {
});

const res = await server.inject({ method: 'POST', url: '/', headers: { authorization: 'Custom validPayload' } });
expect(res.statusCode).to.equal(200);
expect(res.statusCode).to.equal(204);
});

it('throws when scheme requires payload authentication and route conflicts', () => {
Expand Down Expand Up @@ -1646,7 +1646,7 @@ describe('authentication', () => {
});

const res = await server.inject({ method: 'POST', url: '/', headers: { authorization: 'Custom skip' } });
expect(res.statusCode).to.equal(200);
expect(res.statusCode).to.equal(204);
});

it('skips request payload when unauthenticated', async () => {
Expand All @@ -1668,7 +1668,7 @@ describe('authentication', () => {
});

const res = await server.inject({ method: 'POST', url: '/' });
expect(res.statusCode).to.equal(200);
expect(res.statusCode).to.equal(204);
});

it('skips optional payload', async () => {
Expand All @@ -1689,7 +1689,7 @@ describe('authentication', () => {
});

const res = await server.inject({ method: 'POST', url: '/', headers: { authorization: 'Custom optionalPayload' } });
expect(res.statusCode).to.equal(200);
expect(res.statusCode).to.equal(204);
});

it('errors on missing payload when required', async () => {
Expand Down
6 changes: 3 additions & 3 deletions test/core.js
Original file line number Diff line number Diff line change
Expand Up @@ -1036,7 +1036,7 @@ describe('Core', () => {
};

const res = await server.inject(options);
expect(res.statusCode).to.equal(200);
expect(res.statusCode).to.equal(204);
expect(options.auth.credentials).to.exist();
});

Expand All @@ -1057,7 +1057,7 @@ describe('Core', () => {
};

const res = await server.inject(options);
expect(res.statusCode).to.equal(200);
expect(res.statusCode).to.equal(204);
expect(options.auth.credentials).to.exist();
});

Expand Down Expand Up @@ -1184,7 +1184,7 @@ describe('Core', () => {
server.route({ method: 'GET', path: '/', handler });

const res = await server.inject('/');
expect(res.statusCode).to.equal(200);
expect(res.statusCode).to.equal(204);
expect(res.request.app.key).to.equal('value');
});

Expand Down
Loading

0 comments on commit cf81fc8

Please sign in to comment.