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

Allow custom headers to be set for default error responses #34

Merged
merged 3 commits into from
Jul 5, 2018
Merged

Allow custom headers to be set for default error responses #34

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
61f650f
Add headers to error responses too
leo Jul 5, 2018
02c9623
Added test
leo Jul 5, 2018
072df14
Overwrite it in a better way
leo Jul 5, 2018
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
55 changes: 32 additions & 23 deletions src/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -192,21 +192,25 @@ const getHeaders = async (customHeaders = [], current, absolutePath, stats) => {
}
}

const defaultHeaders = {
'Last-Modified': stats.mtime.toUTCString(),
'Content-Length': stats.size,
// Default to "inline", which always tries to render in the browser,
// if that's not working, it will save the file. But to be clear: This
// only happens if it cannot find a appropiate value.
'Content-Disposition': contentDisposition(base, {
type: 'inline'
})
};
let defaultHeaders = {};
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you could also destructure to the existing const variable but that's a bit ugly 🤷‍♂️


if (stats) {
defaultHeaders = {
'Last-Modified': stats.mtime.toUTCString(),
'Content-Length': stats.size,
// Default to "inline", which always tries to render in the browser,
// if that's not working, it will save the file. But to be clear: This
// only happens if it cannot find a appropiate value.
'Content-Disposition': contentDisposition(base, {
type: 'inline'
})
};

const contentType = mime.contentType(base);
const contentType = mime.contentType(base);

if (contentType) {
defaultHeaders['Content-Type'] = contentType;
if (contentType) {
defaultHeaders['Content-Type'] = contentType;
}
}

return Object.assign(defaultHeaders, related);
Expand Down Expand Up @@ -422,7 +426,7 @@ const renderDirectory = async (current, acceptsJSON, handlers, methods, config,
return {directory: output};
};

const sendError = async (response, acceptsJSON, current, handlers, config, spec) => {
const sendError = async (absolutePath, response, acceptsJSON, current, handlers, config, spec) => {
const {err: original, message, code, statusCode} = spec;

/* istanbul ignore next */
Expand Down Expand Up @@ -454,7 +458,7 @@ const sendError = async (response, acceptsJSON, current, handlers, config, spec)
} catch (err) {
if (err.code !== 'ENOENT') {
// eslint-disable-next-line no-use-before-define
return internalError(response, acceptsJSON, current, handlers, config, err);
return internalError(absolutePath, response, acceptsJSON, current, handlers, config, err);
}
}

Expand All @@ -468,7 +472,12 @@ const sendError = async (response, acceptsJSON, current, handlers, config, spec)
return;
}

response.setHeader('Content-Type', 'text/html; charset=utf-8');
const headers = await getHeaders(config.headers, current, absolutePath, null);

response.writeHead(statusCode, Object.assign(headers, {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why assign? You are modifying the headers anyway if you do this. Do you want to modify the original object?
Perhaps you want to do:
Object.assign({}, headers, {...})

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated!

'Content-Type': 'text/html; charset=utf-8'
}));

response.end(errorTemplate({statusCode, message}));
};

Expand Down Expand Up @@ -501,7 +510,7 @@ module.exports = async (request, response, config = {}, methods = {}) => {
try {
relativePath = decodeURIComponent(url.parse(request.url).pathname);
} catch (err) {
return sendError(response, acceptsJSON, current, handlers, config, {
return sendError('/', response, acceptsJSON, current, handlers, config, {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The absolutePath is not defined here yet, so we need to supply a placeholder.

statusCode: 400,
code: 'bad_request',
message: 'Bad Request'
Expand All @@ -513,7 +522,7 @@ module.exports = async (request, response, config = {}, methods = {}) => {
// Prevent path traversal vulnerabilities. We could do this
// by ourselves, but using the package covers all the edge cases.
if (!isPathInside(absolutePath, current)) {
return sendError(response, acceptsJSON, current, handlers, config, {
return sendError(absolutePath, response, acceptsJSON, current, handlers, config, {
statusCode: 400,
code: 'bad_request',
message: 'Bad Request'
Expand Down Expand Up @@ -551,7 +560,7 @@ module.exports = async (request, response, config = {}, methods = {}) => {
stats = await handlers.stat(absolutePath);
} catch (err) {
if (err.code !== 'ENOENT') {
return internalError(response, acceptsJSON, current, handlers, config, err);
return internalError(absolutePath, response, acceptsJSON, current, handlers, config, err);
}
}
}
Expand All @@ -567,7 +576,7 @@ module.exports = async (request, response, config = {}, methods = {}) => {
}
} catch (err) {
if (err.code !== 'ENOENT') {
return internalError(response, acceptsJSON, current, handlers, config, err);
return internalError(absolutePath, response, acceptsJSON, current, handlers, config, err);
}
}
}
Expand All @@ -577,7 +586,7 @@ module.exports = async (request, response, config = {}, methods = {}) => {
stats = await handlers.stat(absolutePath);
} catch (err) {
if (err.code !== 'ENOENT') {
return internalError(response, acceptsJSON, current, handlers, config, err);
return internalError(absolutePath, response, acceptsJSON, current, handlers, config, err);
}
}
}
Expand All @@ -599,7 +608,7 @@ module.exports = async (request, response, config = {}, methods = {}) => {
}
} catch (err) {
if (err.code !== 'ENOENT') {
return internalError(response, acceptsJSON, current, handlers, config, err);
return internalError(absolutePath, response, acceptsJSON, current, handlers, config, err);
}
}

Expand All @@ -621,7 +630,7 @@ module.exports = async (request, response, config = {}, methods = {}) => {
}

if (!stats) {
return sendError(response, acceptsJSON, current, handlers, config, {
return sendError(absolutePath, response, acceptsJSON, current, handlers, config, {
statusCode: 404,
code: 'not_found',
message: 'The requested path could not be found'
Expand Down
26 changes: 26 additions & 0 deletions test/integration.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -972,3 +972,29 @@ test('error for request with malformed URI', async t => {
t.is(text, content);
});

test('error responses get custom headers', async t => {
const url = await getUrl({
'public': path.join(fixturesTarget, 'single-directory'),
'headers': [{
source: '**',
headers: [{
key: 'who',
value: 'me'
}]
}]
});

const response = await fetch(`${url}/non-existing`);
const text = await response.text();

t.is(response.status, 404);
t.is(response.headers.get('who'), 'me');

const content = errorTemplate({
statusCode: 404,
message: 'The requested path could not be found'
});

t.is(text, content);
});