honeymoose/angular-cn
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

refactor(aio): unify error messages for invalid requests to upload-server

Browse Source 
Previously, there was a distinction between GET requests to invalid URLs and all
other requests. This was mainly because the upload-server only accepts GET
requests, but that is not a hard limitation and may change in the future.

Thus, it makes sense to return a 404 response for requests to invalid URLs
regardless of the method used.
This commit is contained in:
Georgios Kalpakas 2017-06-27 20:14:41 +03:00 committed by Pete Bacon Darwin
parent 11647e4c78
commit 8cfc2e2ec0
3 changed files with 28 additions and 46 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
aio/aio-builds-setup/dockerbuild/scripts-js/lib/upload-server/upload-server-factory.ts
View File

@ -105,8 +105,7 @@ class UploadServerFactory {
} }
}); });
middleware.get(/^\/health-check\/?$/, (_req, res) => res.sendStatus(200)); middleware.get(/^\/health-check\/?$/, (_req, res) => res.sendStatus(200));
middleware.get('*', req => this.throwRequestError(404, 'Unknown resource', req)); middleware.all('*', req => this.throwRequestError(404, 'Unknown resource', req));
middleware.all('*', req => this.throwRequestError(405, 'Unsupported method', req));
middleware.use((err: any, _req: any, res: express.Response, _next: any) => this.respondWithError(res, err)); middleware.use((err: any, _req: any, res: express.Response, _next: any) => this.respondWithError(res, err));
return middleware; return middleware;

28
aio/aio-builds-setup/dockerbuild/scripts-js/lib/verify-setup/upload-server.e2e.ts
View File

@ -26,13 +26,13 @@ describe('upload-server (on HTTP)', () => {
it('should disallow non-GET requests', done => { it('should disallow non-GET requests', done => {
const url = `http://${host}/create-build/${pr}/${sha9}`; const url = `http://${host}/create-build/${pr}/${sha9}`;
const bodyRegex = /^Unsupported method/; const bodyRegex = /^Unknown resource/;
Promise.all([ Promise.all([
h.runCmd(`curl -iLX PUT ${url}`).then(h.verifyResponse(405, bodyRegex)), h.runCmd(`curl -iLX PUT ${url}`).then(h.verifyResponse(404, bodyRegex)),
h.runCmd(`curl -iLX POST ${url}`).then(h.verifyResponse(405, bodyRegex)), h.runCmd(`curl -iLX POST ${url}`).then(h.verifyResponse(404, bodyRegex)),
h.runCmd(`curl -iLX PATCH ${url}`).then(h.verifyResponse(405, bodyRegex)), h.runCmd(`curl -iLX PATCH ${url}`).then(h.verifyResponse(404, bodyRegex)),
h.runCmd(`curl -iLX DELETE ${url}`).then(h.verifyResponse(405, bodyRegex)), h.runCmd(`curl -iLX DELETE ${url}`).then(h.verifyResponse(404, bodyRegex)),
]).then(done); ]).then(done);
}); });
@ -376,25 +376,17 @@ describe('upload-server (on HTTP)', () => {
describe(`${host}/*`, () => { describe(`${host}/*`, () => {
it('should respond with 404 for GET requests to unknown URLs', done => { it('should respond with 404 for requests to unknown URLs', done => {
const bodyRegex = /^Unknown resource/; const bodyRegex = /^Unknown resource/;
Promise.all([ Promise.all([
h.runCmd(`curl -iL http://${host}/index.html`).then(h.verifyResponse(404, bodyRegex)), h.runCmd(`curl -iL http://${host}/index.html`).then(h.verifyResponse(404, bodyRegex)),
h.runCmd(`curl -iL http://${host}/`).then(h.verifyResponse(404, bodyRegex)), h.runCmd(`curl -iL http://${host}/`).then(h.verifyResponse(404, bodyRegex)),
h.runCmd(`curl -iL http://${host}`).then(h.verifyResponse(404, bodyRegex)), h.runCmd(`curl -iL http://${host}`).then(h.verifyResponse(404, bodyRegex)),
]).then(done); h.runCmd(`curl -iLX PUT http://${host}`).then(h.verifyResponse(404, bodyRegex)),
}); h.runCmd(`curl -iLX POST http://${host}`).then(h.verifyResponse(404, bodyRegex)),
h.runCmd(`curl -iLX PATCH http://${host}`).then(h.verifyResponse(404, bodyRegex)),
h.runCmd(`curl -iLX DELETE http://${host}`).then(h.verifyResponse(404, bodyRegex)),
it('should respond with 405 for non-GET requests to any URL', done => {
const bodyRegex = /^Unsupported method/;
Promise.all([
h.runCmd(`curl -iLX PUT http://${host}`).then(h.verifyResponse(405, bodyRegex)),
h.runCmd(`curl -iLX POST http://${host}`).then(h.verifyResponse(405, bodyRegex)),
h.runCmd(`curl -iLX PATCH http://${host}`).then(h.verifyResponse(405, bodyRegex)),
h.runCmd(`curl -iLX DELETE http://${host}`).then(h.verifyResponse(405, bodyRegex)),
]).then(done); ]).then(done);
}); });

43
aio/aio-builds-setup/dockerbuild/scripts-js/test/upload-server/upload-server-factory.spec.ts
View File

@ -258,12 +258,12 @@ describe('uploadServerFactory', () => {
}); });
it('should respond with 405 for non-GET requests', done => { it('should respond with 404 for non-GET requests', done => {
verifyRequests([ verifyRequests([
agent.put(`/create-build/${pr}/${sha}`).expect(405), agent.put(`/create-build/${pr}/${sha}`).expect(404),
agent.post(`/create-build/${pr}/${sha}`).expect(405), agent.post(`/create-build/${pr}/${sha}`).expect(404),
agent.patch(`/create-build/${pr}/${sha}`).expect(405), agent.patch(`/create-build/${pr}/${sha}`).expect(404),
agent.delete(`/create-build/${pr}/${sha}`).expect(405), agent.delete(`/create-build/${pr}/${sha}`).expect(404),
], done); ], done);
}); });
@ -418,12 +418,12 @@ describe('uploadServerFactory', () => {
}); });
it('should respond with 405 for non-GET requests', done => { it('should respond with 404 for non-GET requests', done => {
verifyRequests([ verifyRequests([
agent.put('/health-check').expect(405), agent.put('/health-check').expect(404),
agent.post('/health-check').expect(405), agent.post('/health-check').expect(404),
agent.patch('/health-check').expect(405), agent.patch('/health-check').expect(404),
agent.delete('/health-check').expect(405), agent.delete('/health-check').expect(404),
], done); ], done);
}); });
@ -442,26 +442,17 @@ describe('uploadServerFactory', () => {
}); });
describe('GET *', () => {
it('should respond with 404', done => {
const responseBody = 'Unknown resource in request: GET /some/url';
verifyRequests([agent.get('/some/url').expect(404, responseBody)], done);
});
});
describe('ALL *', () => { describe('ALL *', () => {
it('should respond with 405', done => { it('should respond with 404', done => {
const responseFor = (method: string) => `Unsupported method in request: ${method.toUpperCase()} /some/url`; const responseFor = (method: string) => `Unknown resource in request: ${method.toUpperCase()} /some/url`;
verifyRequests([ verifyRequests([
agent.put('/some/url').expect(405, responseFor('put')), agent.get('/some/url').expect(404, responseFor('get')),
agent.post('/some/url').expect(405, responseFor('post')), agent.put('/some/url').expect(404, responseFor('put')),
agent.patch('/some/url').expect(405, responseFor('patch')), agent.post('/some/url').expect(404, responseFor('post')),
agent.delete('/some/url').expect(405, responseFor('delete')), agent.patch('/some/url').expect(404, responseFor('patch')),
agent.delete('/some/url').expect(404, responseFor('delete')),
], done); ], done);
}); });