Skip to content

Commit

Permalink
fix(server): fallthrough non GET and HEAD request to routes… (#2374)
Browse files Browse the repository at this point in the history
Fix a bug introduced in cee700d where the serveIndex feature where always replying instead of forwarding requests to the next middleware.
  • Loading branch information
jvasseur authored and hiroppy committed Dec 29, 2019
1 parent f4c8f94 commit ebe8eca
Show file tree
Hide file tree
Showing 3 changed files with 34 additions and 6 deletions.
18 changes: 16 additions & 2 deletions lib/Server.js
Original file line number Diff line number Diff line change
Expand Up @@ -390,13 +390,27 @@ class Server {

if (Array.isArray(contentBase)) {
contentBase.forEach((item) => {
this.app.use(contentBasePublicPath, serveIndex(item));
this.app.use(contentBasePublicPath, (req, res, next) => {
// serve-index doesn't fallthrough non-get/head request to next middleware
if (req.method !== 'GET' && req.method !== 'HEAD') {
return next();
}

serveIndex(item)(req, res, next);
});
});
} else if (
typeof contentBase !== 'number' &&
!isAbsoluteUrl(String(contentBase))
) {
this.app.use(contentBasePublicPath, serveIndex(contentBase));
this.app.use(contentBasePublicPath, (req, res, next) => {
// serve-index doesn't fallthrough non-get/head request to next middleware
if (req.method !== 'GET' && req.method !== 'HEAD') {
return next();
}

serveIndex(contentBase)(req, res, next);
});
}
}

Expand Down
14 changes: 14 additions & 0 deletions test/server/after-option.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,10 @@ describe('after option', () => {
appArg.get('/after/some/path', (_, response) => {
response.send('after');
});

appArg.post('/after/some/path', (_, response) => {
response.send('after POST');
});
},
port,
},
Expand All @@ -48,4 +52,14 @@ describe('after option', () => {
expect(response.text).toBe('after');
});
});

it('should handle POST requests to after route', () => {
return req
.post('/after/some/path')
.expect('Content-Type', 'text/html; charset=utf-8')
.expect(200)
.then((response) => {
expect(response.text).toBe('after POST');
});
});
});
8 changes: 4 additions & 4 deletions test/server/contentBasePublicPath-option.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -277,19 +277,19 @@ describe('contentBasePublicPath option', () => {
});

it('POST request', (done) => {
req.post(`${contentBasePublicPath}/`).expect(405, done);
req.post(`${contentBasePublicPath}/`).expect(404, done);
});

it('PUT request', (done) => {
req.put(`${contentBasePublicPath}/`).expect(405, done);
req.put(`${contentBasePublicPath}/`).expect(404, done);
});

it('DELETE request', (done) => {
req.delete(`${contentBasePublicPath}/`).expect(405, done);
req.delete(`${contentBasePublicPath}/`).expect(404, done);
});

it('PATCH request', (done) => {
req.patch(`${contentBasePublicPath}/`).expect(405, done);
req.patch(`${contentBasePublicPath}/`).expect(404, done);
});
});
});

0 comments on commit ebe8eca

Please sign in to comment.