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

Plugins serveStatic fixes #1913

Open
wants to merge 8 commits into
base: 8.x
Choose a base branch
from
Open

Plugins serveStatic fixes #1913

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
e53c58b
fix(test): lints and style of bodyReader.test
kolbma May 25, 2022
5371b40
chore(tests): remove 2 doubled tests
kolbma May 25, 2022
93909da
fix(GH-1910): plugins.serverStatic path traversal
kolbma May 26, 2022
d7f32ea
fix(GH-1864): plugins.serveStatic null byte crash
kolbma May 26, 2022
c85da14
fix(GH-1604): plugins.serveStatic no dep dirname
kolbma May 26, 2022
1180bb8
fix(GH-1864): plugins.serveStatic crash in Gzip
kolbma May 26, 2022
02c1497
Merge pull request #1 from kolbma/GH-1864
kolbma May 26, 2022
7f36ae1
Merge pull request #2 from kolbma/GH-1604
kolbma May 26, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
131 changes: 73 additions & 58 deletions lib/plugins/static.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,16 @@

var fs = require('fs');
var path = require('path');
var escapeRE = require('escape-regexp-component');

var assert = require('assert-plus');
var mime = require('mime');
var errors = require('restify-errors');

///--- Globals

var BadRequestError = errors.BadRequestError;
var ForbiddenError = errors.ForbiddenError;
var MethodNotAllowedError = errors.MethodNotAllowedError;
var NotAuthorizedError = errors.NotAuthorizedError;
var ResourceNotFoundError = errors.ResourceNotFoundError;

///--- Functions
Expand All @@ -24,8 +24,9 @@ var ResourceNotFoundError = errors.ResourceNotFoundError;
* @public
* @function serveStatic
* @param {Object} options - an options object
* @throws {MethodNotAllowedError} |
* @throws {NotAuthorizedError}
* @throws {BadRequestError}
* @throws {ForbiddenError}
* @throws {MethodNotAllowedError}
* @throws {ResourceNotFoundError}
* @returns {Function} Handler
* @example
Expand Down Expand Up @@ -53,6 +54,9 @@ var ResourceNotFoundError = errors.ResourceNotFoundError;
* directory that lacks a direct file match.
* You can specify additional restrictions by passing in a `match` parameter,
* which is just a `RegExp` to check against the requested file name.
* It is not matched against the request path.
* It is matched against the normalized unix file path including the
* `directory` option and depending on the other options.
* Additionally, you may set the `charSet` parameter, which will append a
* character set to the content-type detected by the plugin.
* For example, `charSet: 'utf-8'` will result in HTML being served with a
Expand Down Expand Up @@ -89,8 +93,10 @@ function serveStatic(options) {
assert.optionalString(opts.file, 'options.file');
assert.bool(opts.appendRequestPath, 'options.appendRequestPath');

var p = path.normalize(opts.directory).replace(/\\/g, '/');
var re = new RegExp('^' + escapeRE(p) + '/?.*');
var docRoot = path.normalize(opts.directory).replaceAll('\\', '/');
if (!docRoot.endsWith('/')) {
docRoot += '/';
}

function serveFileFromStats(file, err, stats, isGzip, req, res, next) {
if (typeof req.closed === 'function' && req.closed()) {
Expand Down Expand Up @@ -140,78 +146,87 @@ function serveStatic(options) {
}

function serveNormal(file, req, res, next) {
fs.stat(file, function fileStat(err, stats) {
if (!err && stats.isDirectory() && opts.default) {
// Serve an index.html page or similar
var filePath = path.join(file, opts.default);
fs.stat(filePath, function dirStat(dirErr, dirStats) {
serveFileFromStats(
filePath,
dirErr,
dirStats,
false,
req,
res,
next
);
});
} else {
serveFileFromStats(file, err, stats, false, req, res, next);
}
});
try {
fs.stat(file, function fileStat(err, stats) {
if (!err && stats.isDirectory() && opts.default) {
// Serve an index.html page or similar
var filePath = path.join(file, opts.default);
fs.stat(filePath, function dirStat(dirErr, dirStats) {
serveFileFromStats(
filePath,
dirErr,
dirStats,
false,
req,
res,
next
);
});
} else {
serveFileFromStats(file, err, stats, false, req, res, next);
}
});
} catch (err) {
next(new BadRequestError(err, '%s', req.path()));
return;
}
}

function serveGzip(file, req, res, next) {
try {
fs.stat(file + '.gz', function stat(err, stats) {
if (!err) {
res.setHeader('Content-Encoding', 'gzip');
serveFileFromStats(file, err, stats, true, req, res, next);
} else {
serveNormal(file, req, res, next);
}
});
} catch (err) {
next(new BadRequestError(err, '%s', req.path()));
return;
}
}

function serve(req, res, next) {
var file;

if (req.method !== 'GET' && req.method !== 'HEAD') {
next(new MethodNotAllowedError('%s', req.method));
return;
}

if (opts.file) {
//serves a direct file
file = path.join(opts.directory, decodeURIComponent(opts.file));
//serves a direct unchecked file
file = path.join(docRoot, opts.file);
} else if (opts.appendRequestPath) {
file = path.join(opts.directory, decodeURIComponent(req.path()));
file = path.join(docRoot, decodeURIComponent(req.path()));
} else {
var dirBasename = path.basename(opts.directory);
var reqpathBasename = path.basename(req.path());

if (
path.extname(req.path()) === '' &&
dirBasename === reqpathBasename
) {
file = opts.directory;
var reqpathBasename = decodeURIComponent(path.basename(req.path()));

if (!path.extname(reqpathBasename)) {
file = docRoot;
} else {
file = path.join(
opts.directory,
decodeURIComponent(path.basename(req.path()))
);
file = path.join(docRoot, reqpathBasename);
}
}

if (req.method !== 'GET' && req.method !== 'HEAD') {
next(new MethodNotAllowedError('%s', req.method));
return;
}

if (!re.test(file.replace(/\\/g, '/'))) {
next(new NotAuthorizedError('%s', req.path()));
// SAFETY CHECKS
var normalizedFile = path.normalize(file).replaceAll('\\', '/');
if (!normalizedFile.startsWith(docRoot)) {
next(new ForbiddenError('%s', req.path()));
return;
}

if (opts.match && !opts.match.test(file)) {
next(new NotAuthorizedError('%s', req.path()));
if (opts.match && !opts.match.test(normalizedFile)) {
next(new ForbiddenError('%s', req.path()));
return;
}

if (opts.gzip && req.acceptsEncoding('gzip')) {
fs.stat(file + '.gz', function stat(err, stats) {
if (!err) {
res.setHeader('Content-Encoding', 'gzip');
serveFileFromStats(file, err, stats, true, req, res, next);
} else {
serveNormal(file, req, res, next);
}
});
serveGzip(normalizedFile, req, res, next);
} else {
serveNormal(file, req, res, next);
serveNormal(normalizedFile, req, res, next);
}
}

Expand Down
58 changes: 35 additions & 23 deletions test/plugins/bodyReader.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -188,10 +188,10 @@ describe('body reader', function() {
});
});

it('should not add a listener for each call on same socket', done => {
it('should not add listener for each call on same socket', function(done) {
SERVER.use(restify.plugins.bodyParser());

let serverReq, serverRes, serverReqSocket;
var serverReq, serverRes, serverReqSocket;
SERVER.post('/meals', function(req, res, next) {
serverReq = req;
serverRes = res;
Expand All @@ -206,18 +206,23 @@ describe('body reader', function() {
agent: new http.Agent({ keepAlive: true })
});

CLIENT.post('/meals', { breakfast: 'pancakes' }, (err, _, res) => {
CLIENT.post('/meals', { breakfast: 'pancakes' }, function(err, _, res) {
assert.ifError(err);
assert.equal(res.statusCode, 200);

const firstReqSocket = serverReqSocket;
const numReqListeners = listenerCount(serverReq);
const numResListeners = listenerCount(serverRes);
const numReqSocketListeners = listenerCount(serverReq.socket);

// Without setImmediate, the second request will not reuse the socket.
setImmediate(() => {
CLIENT.post('/meals', { lunch: 'salad' }, (err2, __, res2) => {
var firstReqSocket = serverReqSocket;
var numReqListeners = listenerCount(serverReq);
var numResListeners = listenerCount(serverRes);
var numReqSocketListeners = listenerCount(serverReq.socket);

// Without setImmediate, the second request will not reuse
// the socket.
setImmediate(function() {
CLIENT.post('/meals', { lunch: 'salad' }, function(
err2,
__,
res2
) {
assert.ifError(err2);
assert.equal(res2.statusCode, 200);
assert.equal(
Expand All @@ -240,15 +245,15 @@ describe('body reader', function() {
});
});

it('should call next for each successful request on same socket', done => {
let nextCallCount = 0;
it('should call next for successful requests on socket', function(done) {
var nextCallCount = 0;
SERVER.use(restify.plugins.bodyParser());
SERVER.use((req, res, next) => {
SERVER.use(function(req, res, next) {
nextCallCount += 1;
next();
});

let serverReqSocket;
var serverReqSocket;
SERVER.post('/meals', function(req, res, next) {
res.send();
next();
Expand All @@ -260,15 +265,20 @@ describe('body reader', function() {
agent: new http.Agent({ keepAlive: true })
});

CLIENT.post('/meals', { breakfast: 'waffles' }, (err, _, res) => {
CLIENT.post('/meals', { breakfast: 'waffles' }, function(err, _, res) {
assert.ifError(err);
assert.equal(res.statusCode, 200);
const firstReqSocket = serverReqSocket;
var firstReqSocket = serverReqSocket;
assert.equal(nextCallCount, 1);

// Without setImmediate, the second request will not reuse the socket.
setImmediate(() => {
CLIENT.post('/meals', { lunch: 'candy' }, (err2, __, res2) => {
// Without setImmediate, the second request will not reuse
// the socket.
setImmediate(function() {
CLIENT.post('/meals', { lunch: 'candy' }, function(
err2,
__,
res2
) {
assert.ifError(err2);
assert.equal(res2.statusCode, 200);
assert.equal(
Expand All @@ -290,9 +300,11 @@ describe('body reader', function() {
* @returns {number} - The total number of listeners across all events
*/
function listenerCount(emitter) {
let numListeners = 0;
for (const eventName of emitter.eventNames()) {
numListeners += emitter.listenerCount(eventName);
var numListeners = 0;
var eventNames = emitter.eventNames();
var emitterCount = eventNames.length;
for (var idx = 0; idx < emitterCount; idx += 1) {
numListeners += emitter.listenerCount(eventNames[idx]);
}
return numListeners;
}
Loading