Skip to content

Commit

Permalink
Add path data to error events. Closes #111
Browse files Browse the repository at this point in the history
  • Loading branch information
kanongil committed Jan 15, 2018
1 parent bd48207 commit c2a2703
Show file tree
Hide file tree
Showing 7 changed files with 68 additions and 35 deletions.
14 changes: 14 additions & 0 deletions README.md
Expand Up @@ -29,6 +29,7 @@ file based resources.
- [`h.file(path, [options])`](#hfilepath-options)
- [The `file` handler](#the-file-handler)
- [The `directory` handler](#the-directory-handler)
- [Errors](#errors)

## Examples

Expand Down Expand Up @@ -258,3 +259,16 @@ object with the following options:
- `false` - Disable ETag computation.
- `defaultExtension` - optional string, appended to file requests if the requested file is
not found. Defaults to no extension.

### Errors

Any file access errors are signalled using appropriate [Boom](https://github.com/hapijs/boom)
errors. These are `Boom.notFound()` for missing or hidden files, and `Boom.forbidden()` for
files that exist, but can't otherwise be accessed.

The error can contain an `err.data.path` property, which is the path that the error failed for.
This property *does not always exist* if the response was generated without a file system lookup,
and for the directory handler it will be the last tested non-index path.

If an unexpected configuration or processing errors occurs, `Boom.internal()` and `'system'`
errors can also be thrown.
46 changes: 22 additions & 24 deletions lib/directory.js
Expand Up @@ -54,7 +54,7 @@ internals.resolvePathOption = function (basePath, result) {
paths = result;
}
else {
throw Boom.badImplementation('Invalid path function');
throw Boom.internal('Invalid path function');
}

return internals.normalizePaths(basePath, paths);
Expand Down Expand Up @@ -84,7 +84,7 @@ exports.handler = function (route, options) {
!settings.showHidden &&
internals.isFileHidden(selection)) {

throw Boom.notFound();
throw Boom.notFound(null, {});
}

// Generate response
Expand Down Expand Up @@ -116,26 +116,15 @@ exports.handler = function (route, options) {
// Handle Not found

if (internals.isNotFound(error)) {
if (settings.defaultExtension) {
if (hasTrailingSlash) {
path = path.slice(0, -1);
}

try {
return await File.load(path + '.' + settings.defaultExtension, request, fileOptions);
}
catch (err) {
Bounce.ignore(err, 'boom');

// Propagate non-directory errors
if (!settings.defaultExtension) {
throw error;
}

if (!internals.isNotFound(err)) {
throw err;
}
}
if (hasTrailingSlash) {
path = path.slice(0, -1);
}

return;
return await File.load(path + '.' + settings.defaultExtension, request, fileOptions);
}

// Handle Directory
Expand Down Expand Up @@ -175,13 +164,22 @@ exports.handler = function (route, options) {
};

for (let i = 0; i < paths.length; ++i) {
const response = await each(paths[i]);
if (response !== undefined) {
return response;
try {
return await each(paths[i]);
}
catch (err) {
Bounce.ignore(err, 'boom');

// Propagate any non-404 errors

if (!internals.isNotFound(err) ||
i === paths.length - 1) {
throw err;
}
}
}

throw Boom.notFound();
throw Boom.notFound(null, {});
};

return handler;
Expand Down Expand Up @@ -242,5 +240,5 @@ internals.isNotFound = function (boom) {

internals.isDirectory = function (boom) {

return boom.output.statusCode === 403 && boom.data === 'EISDIR';
return boom.output.statusCode === 403 && boom.data.code === 'EISDIR';
};
2 changes: 1 addition & 1 deletion lib/etag.js
Expand Up @@ -85,7 +85,7 @@ internals.hashFile = async function (response) {
}
catch (err) {
Bounce.rethrow(err, 'system');
throw Boom.boomify(err, { message: 'Failed to hash file' });
throw Boom.boomify(err, { message: 'Failed to hash file', data: { path: response.source.path } });
}
};

Expand Down
2 changes: 1 addition & 1 deletion lib/file.js
Expand Up @@ -101,7 +101,7 @@ internals.prepare = async function (response) {
const path = response.source.path;

if (path === null) {
throw Boom.forbidden(null, 'EACCES');
throw Boom.forbidden(null, { code: 'EACCES' });
}

const file = response.source.file = new Fs.File(path);
Expand Down
15 changes: 9 additions & 6 deletions lib/fs.js
Expand Up @@ -34,23 +34,26 @@ exports.File.prototype.open = async function (mode) {
this.fd = await exports.open(this.path, mode);
}
catch (err) {
const data = { path: this.path };

if (this.path.indexOf('\u0000') !== -1 || err.code === 'ENOENT') {
throw Boom.notFound();
throw Boom.notFound(null, data);
}

if (err.code === 'EACCES' || err.code === 'EPERM') {
throw Boom.forbidden(null, err.code);
data.code = err.code;
throw Boom.forbidden(null, data);
}

throw Boom.boomify(err, { message: 'Failed to open file' });
throw Boom.boomify(err, { message: 'Failed to open file', data });
}
};


exports.File.prototype.close = function () {

if (this.fd !== null) {
exports.close(this.fd).then(null, Hoek.ignore);
Bounce.background(exports.close(this.fd));
this.fd = null;
}
};
Expand All @@ -64,7 +67,7 @@ exports.File.prototype.stat = async function () {
const stat = await exports.fstat(this.fd);

if (stat.isDirectory()) {
throw Boom.forbidden(null, 'EISDIR');
throw Boom.forbidden(null, { code: 'EISDIR', path: this.path });
}

return stat;
Expand All @@ -73,7 +76,7 @@ exports.File.prototype.stat = async function () {
this.close(this.fd);

Bounce.rethrow(err, ['boom', 'system']);
throw Boom.boomify(err, { message: 'Failed to stat file' });
throw Boom.boomify(err, { message: 'Failed to stat file', data: { path: this.path } });
}
};

Expand Down
13 changes: 12 additions & 1 deletion test/directory.js
Expand Up @@ -49,13 +49,14 @@ describe('directory', () => {

const res = await server.inject('/directory/');
expect(res.statusCode).to.equal(403);
expect(res.request.response._error.data.path).to.equal(__dirname);
});

it('returns a 403 when requesting a path containing \'..\'', async () => {

const forbidden = (request, h) => {

return h.response().code(403);
return Boom.forbidden(null, {});
};

const server = await provisionServer();
Expand All @@ -64,6 +65,7 @@ describe('directory', () => {

const res = await server.inject('/directory/..');
expect(res.statusCode).to.equal(403);
expect(res.request.response._error.data.path).to.not.exist();
});

it('returns a 404 when requesting an unknown file within a directory', async () => {
Expand All @@ -73,6 +75,7 @@ describe('directory', () => {

const res = await server.inject('/directory/xyz');
expect(res.statusCode).to.equal(404);
expect(res.request.response._error.data.path).to.equal(Path.join(__dirname, 'xyz'));
});

it('returns a file when requesting a file from the directory', async () => {
Expand Down Expand Up @@ -129,6 +132,7 @@ describe('directory', () => {

const res = await server.inject('/directoryfn/index.js');
expect(res.statusCode).to.equal(404);
expect(res.request.response._error.data.path).to.not.exist();
});

it('returns the correct file when requesting a file from a child directory', async () => {
Expand Down Expand Up @@ -218,6 +222,7 @@ describe('directory', () => {

const res = await server.inject('/directoryx/');
expect(res.statusCode).to.equal(403);
expect(res.request.response._error.data.path).to.equal(Path.join(__dirname, '..'));
});

it('returns a list of files when listing is enabled', async () => {
Expand Down Expand Up @@ -297,6 +302,7 @@ describe('directory', () => {

const res = await server.inject('/directoryIndex/');
expect(res.statusCode).to.equal(403);
expect(res.request.response._error.data.path).to.equal(Path.join(__dirname, 'directory'));
});

it('returns a 403 when listing is disabled and an array of index files is specified but none were found', async () => {
Expand All @@ -306,6 +312,7 @@ describe('directory', () => {

const res = await server.inject('/directoryIndex/');
expect(res.statusCode).to.equal(403);
expect(res.request.response._error.data.path).to.equal(Path.join(__dirname, 'directory'));
});

it('returns the index when served from a hidden folder', async () => {
Expand Down Expand Up @@ -537,6 +544,7 @@ describe('directory', () => {

const res = await server.inject('/directory/directory/none');
expect(res.statusCode).to.equal(404);
expect(res.request.response._error.data.path).to.equal(Path.join(__dirname, 'directory/none.html'));
});

it('appends default extension and errors on file', async () => {
Expand All @@ -546,6 +554,7 @@ describe('directory', () => {

const res = await server.inject('/directory/directory/index');
expect(res.statusCode).to.equal(403);
expect(res.request.response._error.data.path).to.equal(Path.join(__dirname, 'directory/index.dir'));
});

it('does not append default extension when directory exists', async () => {
Expand Down Expand Up @@ -694,6 +703,7 @@ describe('directory', () => {
}

expect(res.statusCode).to.equal(403);
expect(res.request.response._error.data.path).to.equal(filename);
});

it('returns error when a file open fails', async () => {
Expand All @@ -720,6 +730,7 @@ describe('directory', () => {

const res = await server.inject('/index%00.html');
expect(res.statusCode).to.equal(404);
expect(res.request.response._error.data.path).to.equal(Path.join(__dirname, 'index\u0000.html'));
});

it('only stats the file system once when requesting a file', async () => {
Expand Down
11 changes: 9 additions & 2 deletions test/file.js
Expand Up @@ -260,12 +260,14 @@ describe('file', () => {

it('returns a 404 when the file is not found', async () => {

const server = await provisionServer({ routes: { files: { relativeTo: '/no/such/path/x1' } } });
const basePath = Path.join(process.platform === 'win32' ? 'C://' : '/', 'no/such/path/x1');
const server = await provisionServer({ routes: { files: { relativeTo: basePath } } });

server.route({ method: 'GET', path: '/filenotfound', handler: { file: 'nopes' } });

const res = await server.inject('/filenotfound');
expect(res.statusCode).to.equal(404);
expect(res.request.response._error.data.path).to.equal(Path.join(basePath, 'nopes'));
});

it('returns a 403 when the file is a directory', async () => {
Expand All @@ -276,6 +278,7 @@ describe('file', () => {

const res = await server.inject('/filefolder');
expect(res.statusCode).to.equal(403);
expect(res.request.response._error.data.path).to.equal(Path.join(__dirname, '..', 'lib'));
});

it('returns a file using the built-in handler config', async () => {
Expand Down Expand Up @@ -614,7 +617,8 @@ describe('file', () => {
it('return a 500 on hashing errors', async () => {

const server = await provisionServer();
server.route({ method: 'GET', path: '/file', handler: { file: Path.join(__dirname, '..', 'package.json') } });
const filepath = Path.join(__dirname, '..', 'package.json');
server.route({ method: 'GET', path: '/file', handler: { file: filepath } });

// Prepare complicated mocking setup to fake an io error

Expand All @@ -634,6 +638,7 @@ describe('file', () => {
const res = await server.inject('/file');
expect(res.statusCode).to.equal(500);
expect(res.request.response._error).to.be.an.error(/^Failed to hash file/);
expect(res.request.response._error.data.path).to.equal(filepath);
});

it('handles multiple simultaneous request hashing errors', async () => {
Expand Down Expand Up @@ -1072,6 +1077,7 @@ describe('file', () => {
const res = await server.inject('/');
expect(res.statusCode).to.equal(500);
expect(res.request.response._error).to.be.an.error('Failed to stat file: failed');
expect(res.request.response._error.data.path).to.equal(filename);
});

it('returns error when open fails unexpectedly', async () => {
Expand All @@ -1092,6 +1098,7 @@ describe('file', () => {
const res = await server.inject('/');
expect(res.statusCode).to.equal(500);
expect(res.request.response._error).to.be.an.error('Failed to open file: failed');
expect(res.request.response._error.data.path).to.equal(filename);
});

it('returns a 403 when missing file read permission', async () => {
Expand Down

0 comments on commit c2a2703

Please sign in to comment.