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.
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
fix(static): avoid user-provided data in Error messages being interpreted as sprintf codes #1384
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -984,6 +984,39 @@ test('GH-719 serve a specific static file', function (t) { | |
serveStaticTest(t, false, '.tmp', null, true); | ||
}); | ||
|
||
test('static responds 404 for missing file', function (t) { | ||
var p = '/public/no-such-file.json'; | ||
var tmpPath = path.join(process.cwd(), '.tmp'); | ||
|
||
SERVER.get(new RegExp('/public/.*'), | ||
restify.serveStatic({directory: tmpPath})); | ||
|
||
CLIENT.get(p, function (err, req, res, obj) { | ||
t.ok(err); | ||
t.equal(err.statusCode, 404); | ||
t.equal(err.restCode, 'ResourceNotFound'); | ||
t.end(); | ||
}); | ||
}); | ||
|
||
// Earlier versons would throw: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Comment makes sense in the context of this PR, but may be confusing to future readers who don't have this context. Also, test name should begin with |
||
// > new restify.ResourceNotFoundError('/%22s not found') | ||
// Error: too few args to sprintf | ||
test('static responds 404 for missing file with percent-codes', function (t) { | ||
var p = '/public/no-%22such-file.json'; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Great test! |
||
var tmpPath = path.join(process.cwd(), '.tmp'); | ||
|
||
SERVER.get(new RegExp('/public/.*'), | ||
restify.serveStatic({directory: tmpPath})); | ||
|
||
CLIENT.get(p, function (err, req, res, obj) { | ||
t.ok(err); | ||
t.equal(err.statusCode, 404); | ||
t.equal(err.restCode, 'ResourceNotFound'); | ||
t.end(); | ||
}); | ||
}); | ||
|
||
|
||
test('audit logger timer test', function (t) { | ||
// Dirty hack to capture the log record using a ring buffer. | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch, this test was missing