node-restify icon indicating copy to clipboard operation
node-restify copied to clipboard

Issue 1604: Fix 'appendRequestPath: false' scenarios for Restify.plugins.serverStatic

Open jmbeltran07 opened this issue 5 years ago • 1 comments

Pre-Submission Checklist

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

Issues

Closes:

  • Issue # https://github.com/restify/node-restify/issues/1604

Restify.plugins.serverStatic option "appendRequestPath" is not working properly. According to documentation, when appendRequestPath = false, the file we are requesting should be served directly from directory, this wasn't the case. for example:

var restify = require('restify');

const server = restify.createServer({
    name: 'myapp',
    version: '1.0.0'
  });

server.get('/endpoint/*', restify.plugins.serveStatic({
    directory: __dirname, // dirname is /Users/keithlee96/testFolder/test-api
    default: 'index.html',
    appendRequestPath: false
}));

server.listen(9090, function(){
    console.log('%s listening at %s', server.name, server.url);
});

if we make a call to /endpoint it will attempt to serve __dirname/endpoint/index.html, instead of __dirname/index.html (because we are setting appendRequestPath: false another example is that if we call /endpoint/myCoolResource.json it will also attempt to serve __dirname/endpoint/myCoolResource.json instead of __dirname/myCoolResource.json

Changes

With this PR when the user is setting appendRequestPath = false, the server will serve the requested resource specified in the endpoint (or the default static resource) which is located directly on directory. Now the behavior adequate with what's described on http://restify.com/docs/plugins-api/#servestatic

Also we are modifying the function testNoAppendPath used by the tests which were passing as false positives so it catches these errors in the future. Also, we are adding more tests to increase the coverage of Restify.plugins.serverStatic function.

jmbeltran07 avatar Oct 26 '20 05:10 jmbeltran07

Just for completeness, we also happen to have another flavor of serving static files with restify which has it's API syntax (and functionality) similar to that of expressjs.

We had seen a lot of issues with restify.plugins.serveStatic and hence at some point, we built restify.plugins.serveStaticFiles.

Here is a small example of how to use it, feel free to play around with it: https://codesandbox.io/s/gallant-raman-9x9jk?file=/app.js

rajatkumar avatar Oct 28 '20 21:10 rajatkumar