Skip to content

Commit

Permalink
Retain file between prepare and marshal phases
Browse files Browse the repository at this point in the history 
Additionally adds a 403 response for files with insufficient permissions.

Fixes #4
  • Loading branch information
@kanongil
kanongil committed Nov 26, 2014
1 parent b2227db commit dee3465
Show file tree
Hide file tree
Showing 2 changed files with 192 additions and 25 deletions.
80 changes: 58 additions & 22 deletions lib/file.js
View file
Expand Up @@ -58,25 +58,27 @@ exports.response = function (path, options, request) {
var source = {
path: Path.normalize(Hoek.isAbsolutePath(path) ? path : Path.join(request.route.files.relativeTo, path)),
settings: options,
stat: null
stat: null,
fd: null
};

return request.generateResponse(source, { variety: 'file', marshal: internals.marshal, prepare: internals.prepare });
return request.generateResponse(source, { variety: 'file', marshal: internals.marshal, prepare: internals.prepare, close: internals.close });
};


internals.prepare = function (response, callback) {

// close any leftover descriptors from previous prepare call
internals.close(response);

var path = response.source.path;
Fs.stat(path, function (err, stat) {
internals.openStat(path, 'r', function (err, fd, stat) {

if (err) {
return callback(Boom.notFound());
return callback(err);
}

if (stat.isDirectory()) {
return callback(Boom.forbidden());
}
response.source.fd = fd;

response.bytes(stat.size);

Expand Down Expand Up @@ -132,14 +134,15 @@ internals.marshal = function (response, callback) {
}

var gzFile = response.source.path + '.gz';
Fs.stat(gzFile, function (err, stat) {

if (err ||
stat.isDirectory()) {
internals.openStat(gzFile, 'r', function (err, fd, stat) {

if (err) {
return internals.openStream(response, response.source.path, callback);
}

internals.close(response);
response.source.fd = fd;

response.bytes(stat.size);
response.header('content-encoding', 'gzip');
response.vary('accept-encoding');
Expand All @@ -151,22 +154,55 @@ internals.marshal = function (response, callback) {

internals.openStream = function (response, path, callback) {

var fileStream = Fs.createReadStream(path);
Hoek.assert(response.source.fd !== null, 'file descriptor must be set');

var onError = function (err) {
var fileStream = Fs.createReadStream(path, { fd: response.source.fd });

fileStream.removeListener('open', onOpen);
return callback(err);
};
// claim descriptor
response.source.fd = null;

var onOpen = function () {
// callback immediately since descriptors are already open
return callback(null, fileStream);
};

fileStream.removeListener('error', onError);
return callback(null, fileStream);
};

fileStream.once('error', onError);
fileStream.once('open', onOpen);
internals.openStat = function (path, mode, callback) {

Fs.open(path, mode, function(err, fd) {

if (err) {
if (err.code === 'ENOENT') {
return callback(Boom.notFound());
} else if (err.code === 'EACCES') {
return callback(Boom.forbidden());
}
return callback(Boom.wrap(err, null, 'failed to open file'));
}

Fs.fstat(fd, function(err, stat) {

if (err) {
Fs.close(fd, Hoek.ignore);
return callback(Boom.wrap(err, null, 'failed to stat file'));
}

if (stat.isDirectory()) {
Fs.close(fd, Hoek.ignore);
return callback(Boom.forbidden());
}

return callback(null, fd, stat);
});
});
};


internals.close = function (response) {

if (response.source.fd !== null) {
Fs.close(response.source.fd, Hoek.ignore);
response.source.fd = null;
}
};


Expand Down
137 changes: 134 additions & 3 deletions test/file.js
View file
Expand Up @@ -271,6 +271,23 @@ describe('file', function () {
});
});

it('returns a 403 when missing file read permission', function (done) {

var filename = Hoek.uniqueFilename(Os.tmpDir()) + '.package.json';
Fs.writeFileSync(filename, 'data');
Fs.chmodSync(filename, 0);

var server = provisionServer();
server.route({ method: 'GET', path: '/', handler: { fileTest: filename } });

server.inject('/', function (res) {
Fs.unlinkSync(filename);

expect(res.statusCode).to.equal(403);
done();
});
});

it('returns a file using the build-in handler config', function (done) {

var server = provisionServer(__dirname);
Expand Down Expand Up @@ -819,9 +836,9 @@ describe('file', function () {
done();
});

it('returns error when file is removed before stream is opened', function (done) {
it('responds correctly when file is removed while processing', function (done) {

var filename = Hoek.uniqueFilename(Os.tmpDir());
var filename = Hoek.uniqueFilename(Os.tmpDir()) + '.package.json';
Fs.writeFileSync(filename, 'data');

var server = provisionServer();
Expand All @@ -834,7 +851,31 @@ describe('file', function () {

server.inject('/', function (res) {

expect(res.statusCode).to.equal(500);
expect(res.statusCode).to.equal(200);
done();
});
});

it('responds correctly when file is changed while processing', function (done) {

var filename = Hoek.uniqueFilename(Os.tmpDir()) + '.package.json';
Fs.writeFileSync(filename, 'data');

var server = provisionServer();
server.route({ method: 'GET', path: '/', handler: { fileTest: filename } });
server.ext('onPreResponse', function (request, reply) {

Fs.unlinkSync(filename);
Fs.writeFileSync(filename, 'database');
return reply.continue();
});

server.inject('/', function (res) {
Fs.unlinkSync(filename);

expect(res.statusCode).to.equal(200);
expect(res.headers['content-length']).to.equal(4);
expect(res.payload).to.equal('data');
done();
});
});
Expand All @@ -859,5 +900,95 @@ describe('file', function () {
});
});
});

it('returns error when aborted while processing', function (done) {

var filename = Hoek.uniqueFilename(Os.tmpDir()) + '.package.json';
Fs.writeFileSync(filename, 'data');

var server = provisionServer();
server.route({ method: 'GET', path: '/', handler: { fileTest: filename } });
server.ext('onPreResponse', function (request, reply) {
reply(Boom.internal('crapping out'));
});

server.inject('/', function (res) {

expect(res.statusCode).to.equal(500);
done();
});
});

it('returns error when stat fails unexpectedly', function (done) {

var filename = Hoek.uniqueFilename(Os.tmpDir()) + '.package.json';
Fs.writeFileSync(filename, 'data');

var orig = Fs.fstat;
Fs.fstat = function (fd, callback) { // can return EIO error

Fs.fstat = orig;
callback(new Error('failed'));
};


var server = provisionServer();
server.route({ method: 'GET', path: '/', handler: { fileTest: filename } });

server.inject('/', function (res) {

expect(res.statusCode).to.equal(500);
done();
});
});

it('returns error when open fails unexpectedly', function (done) {

var filename = Hoek.uniqueFilename(Os.tmpDir()) + '.package.json';
Fs.writeFileSync(filename, 'data');

var orig = Fs.open;
Fs.open = function () { // can return EIO error
Fs.open = orig;

var callback = arguments[arguments.length - 1];
callback(new Error('failed'));
};


var server = provisionServer();
server.route({ method: 'GET', path: '/', handler: { fileTest: filename } });

server.inject('/', function (res) {

expect(res.statusCode).to.equal(500);
done();
});
});
});

it('has not leaked file descriptors', function (done) {

// validate that all descriptors has been closed
var cmd = ChildProcess.spawn('lsof', ['-p', process.pid]);
var lsof = '';
cmd.stdout.on('data', function (buffer) {

lsof += buffer.toString();
});

cmd.stdout.on('end', function () {

var count = 0;
var lines = lsof.split('\n');
for (var i = 0, il = lines.length; i < il; ++i) {
count += !!lines[i].match(/package.json/);
}

expect(count).to.equal(0);
done();
});

cmd.stdin.end();
});
});

0 comments on commit dee3465

Please sign in to comment.