fix(static): avoid user-provided data in Error messages being interpr…

…eted as sprintf codes (#1384)



The 'static' plugin had a few cases where the path in a request would be
passed as the first ("message") field to a RestError constructor.
RestError uses verror.WError, which uses extsprintf to render the given
arguments. If the "message" includes "%...s" or similar printf codes,
then it will error output.

Also bump to 4.3.1.

* drop ver bump changes, per review comments

* update test name and drop comment (unnecessary with issue ref) per review comments

CHANGES.md

@@ -2,7 +2,8 @@
 
 ## not yet released
 
-(nothing yet)
+- #1382 Fix "static" plugin to not throw on a 404 for a path with sprintf-like
+  percent escape codes.
 
 ## 4.3.0
 

lib/plugins/static.js

@@ -44,8 +44,7 @@ function serveStatic(opts) {
 
     function serveFileFromStats(file, err, stats, isGzip, req, res, next) {
         if (err) {
-            next(new ResourceNotFoundError(err,
-                req.path()));
+            next(new ResourceNotFoundError(err, '%s', req.path()));
             return;
         } else if (!stats.isFile()) {
             next(new ResourceNotFoundError('%s does not exist', req.path()));
@@ -120,17 +119,17 @@ function serveStatic(opts) {
         }
 
         if (req.method !== 'GET' && req.method !== 'HEAD') {
-            next(new MethodNotAllowedError(req.method));
+            next(new MethodNotAllowedError('%s', req.method));
             return;
         }
 
         if (!re.test(file.replace(/\\/g, '/'))) {
-            next(new NotAuthorizedError(req.path()));
+            next(new NotAuthorizedError('%s', req.path()));
             return;
         }
 
         if (opts.match && !opts.match.test(file)) {
-            next(new NotAuthorizedError(req.path()));
+            next(new NotAuthorizedError('%s', req.path()));
             return;
         }
 

test/plugins.test.js

@@ -984,6 +984,37 @@ 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();
+    });
+});
+
+test('GH-1382 static responds 404 for missing file with percent-codes',
+        function (t) {
+    var p = '/public/no-%22such-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();
+    });
+});
+
 
 test('audit logger timer test', function (t) {
     // Dirty hack to capture the log record using a ring buffer.