fix NotAuthorized error when serving static files while directory var…

[unsol]

Pre-Submission Checklist

• Opened an issue discussing these changes before opening the PR
• Ran the linter and tests via make prepush
• Included comprehensive and convincing tests for changes

Issues

Closes:

• Issue 549

Changes

This is to resolve issue #549, so I did not create a new issue for this pull request.

The serveStatic function sets the appendRequestPath option to true by default. This works the majority of the time, but if the directory option is set to "./", then the regex test (

node-restify/lib/plugins/static.js

Line 194 in e663699

if (!re.test(file.replace(/\\/g, '/'))) {

) fails, which results in NotAuthorizedError. This test fails, as it is trying to compare a slash, against a full path. By moving the "p", and "re" variable declarations to the serve function, after the file path is joined with opts.directory, we are then able to normalize the path, run that output through escapeRE, and test the expected expression.

The "p", and "re" variables are not used anywhere else in this file -- only within the serve function.

I tested this solution with the following routes:

    server.get("/firsttest", restify.plugins.serveStatic({
        directory: "./app",
        file: "firsttest.html",
        appendRequestPath: false,
    }));

    server.get("/secondtest", restify.plugins.serveStatic({
        directory: "./app",
        file: "secondtest().html",
        appendRequestPath: false,
    }));

    server.get("/docs/*", restify.plugins.serveStatic({
        directory: "./",
        default: "index.html",
        appendRequestPath: true,
    }));

My folder structure for the tests was something along the lines of:
 /
  docs/
   v1/
    index.html
   v2/
    index.html
  app/
   firsttest.html
   secondtest().html
   (contains source code)
  dist/
   contains typescript build output

I had the first/second test routes set to serve files from the ./app directory, as they just didn't want to serve from docs (kept getting a not found event). I'm assuming that has something to do with the wild card route taking precedence for some reason.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[unsol]

Bad stale bot! Bad!

Has anyone had a chance to test this out? I've been using this patch in production since August, and have had no issues.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[unsol]

Bumping it up again.

Still no issues with daily use.

[unsol]

Been running on 7.4.0. Doesn't look like 7.5.0 will have any effect on the serveStatic changes.