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

Restify.plugins.serverStatic option "appendRequestPath" is bugged

Open keithlee96 opened this issue 7 years ago • 16 comments

  • [ X ] Used appropriate template for the issue type
  • [ X ] Searched both open and closed issues for duplicates of this issue
  • [ X ] Title adequately and concisely reflects the feature or the bug

Bug Report

Restify.plugins.serverStatic option "appendRequestPath" is not working properly.

Restify Version

v6.3.4

Node.js Version

v8.91

Expected behaviour

I expected appendRequestPath = false; to make it so that the request path is not appended to the file path. (As stated in the documentation at http://restify.com/docs/plugins-api/#servestatic)

Actual behaviour

The request path was appended to the file path, despite the fact that appendRequestPath was set to false.

Repro case

index.js

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);
});

index.html was in the same folder.

I ran the file with node index.js When I made a get request to localhost:9090/endpoint in postman, I got the error:

{
    "code": "ResourceNotFound",
    "message": "/endpoint; caused by Error: ENOENT: no such file or directory, stat '/Users/keithlee96/testFolder/test-api/endpoint'"
}

Expected the targeted file path to be: '/Users/keithlee96/testFolder/test-api/index.html'

Are you willing and able to fix this?

No, I'm no familiar with typescript, so I don't know how to fix the error myself. I just want it to behave as the documentation http://restify.com/docs/plugins-api/#servestatic says it should. Any help on this would be greatly appreciated.

keithlee96 avatar Feb 11 '18 23:02 keithlee96

I agree, just met the same unexpected behaviour. I assume it's because of dirBasename === reqpathBasename condition, which forces you to name the route folder name as it is in the directory which you try to serve.

Why it is there?

I assume it has little sense.

Regards,

PavelPolyakov avatar Feb 12 '18 12:02 PavelPolyakov

Hey @keithlee96,

First, thank you for the thorough bug report with detailed examples :heart:.

Currently the serveStatic plugin isn't well supported. There are quite a few open bugs for it that are looking for contributors. From my understanding, it was introduced a long time ago and none of the current active maintainers use it.

As it stands, I wouldn't recommend using this plugin and instead would recommend either:

  1. Fronting your Node.js process with a webserver like NGinx for serving static content
  2. Wiring an endpoint up to fs.createReadStream

retrohacker avatar Feb 13 '18 00:02 retrohacker

@hekike @DonutEspresso

serveStatic keeps coming up again and again, but it doesn't seem like we ever get momentum on fixing this plugin. Would it be worth considering removing it from our plugins? Having it in a half-working state is causing confusion.

retrohacker avatar Feb 13 '18 00:02 retrohacker

Agree, just battled with this for longer than I should have since I was trusting the documentation. All I was trying to do was connect a Swagger-UI documentation feature to my rest api.

Please either deprecate or provide good advice for alternatives. Thanks!

kingnebby avatar Feb 13 '18 16:02 kingnebby

@kingnebby sorry this was such a painful experience for you. I believe we should deprecate this unless someone is willing to step-up and maintain this plugin.

Based on the activity I've seen on this since I've joined the project, I suspect there is a low probability of this becoming actively maintained so my vote is to deprecate it entirely with a note about alternative solutions.

retrohacker avatar Feb 13 '18 21:02 retrohacker

I'm okay with removing this as it's a REST framework, however, may some user want to serve Swagger spec as a static file.

hekike avatar Feb 13 '18 21:02 hekike

@retrohacker - I can't complain tbh, this and many other projects have saved me probably trillions of hours of work. :)

Also, I think providing a guide on the 'fs' option is better than telling people to use NGinx. Idk, it feels wrong to say, "Just go use a different technology to configure your server" as a solution to a framework shortcoming.

As far as swagger goes, that might be a good use case/tutorial. I finally got it all working with some funkiness:

  • Swagger-UI stuff in dist/docs.
  • Swagger Spec in dist/

Wired with this code.

// Serve the swagger-ui pages.
server.get(/\/doc\/.*/, restify.plugins.serveStatic({
  directory: './dist',
  default: 'index.html',
}))
// Serves just the Swagger spec for various consumption, like
// the Swagger-UI, and the online Swagger-Editor.
server.get(/^\/swagger$/, restify.plugins.serveStatic({
  directory: './dist',
  file: 'swagger.yaml',
}))
// Helper to reroute people who don't type exaclty the write URL.
server.get(/(^\/$|^\/doc$)/, (req, res, next) => {
  res.redirect('/doc/', next)
})

Who knows, maybe this will help someone.

kingnebby avatar Feb 13 '18 21:02 kingnebby

Ouch. Me too. Hitting this issue. I really like severStatic b/ its a simple way to spin up a webserver and focus work on client side dev. Moreover, from the same server I could have the client files talk to the backend restful endpoints.

phpmaps avatar Mar 16 '18 23:03 phpmaps

@kingnebby can you provide a more detailed example of the swagger configuration? I've been trying to add swagger documentation to my restify api with no luck.

Thanks in advance!!

irokhes avatar Mar 24 '18 22:03 irokhes

@irokhes I was actually considering doing a full tutorial, this is a good incentive :) Maybe I'll try to put it on Medium next week.

kingnebby avatar Mar 25 '18 00:03 kingnebby

@kingnebby that would be awesome! If you need someone to review your initial draft I'll be more than happy!!

Thanks!

irokhes avatar Mar 25 '18 00:03 irokhes

@irokhes

For starters, here's a sample project.

kingnebby avatar Mar 26 '18 23:03 kingnebby

Hey,

do you know, when/If this will be fixed at all?

If Not, I would really appreciate a deprecation msg.

Cheers and thx

himbaer avatar Jan 16 '19 14:01 himbaer

Hi! I would like to contribute to this project! I created PR: https://github.com/restify/node-restify/pull/1860 that attempts to fix this issue. With that PR now Restify.plugins.serverStatic will behave as in the documentation http://restify.com/docs/plugins-api/#servestatic when appendRequestPath is set to false. For the example code above:

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);
});

The PR will be serving index.html which is located on __dirname, and it will not try to search it on __dirname/endpoint. Also this applies for any resource, not only to retrieve the default. i.e: /endpoint/anotherResourse.html will serve __dirname/anotherResource.html in we set: server.get('/endpoint/*, ...

I hope this helps you guys, please let me know any feedback!

jmbeltran07 avatar Oct 26 '20 05:10 jmbeltran07

See https://github.com/restify/node-restify/issues/1910

kolbma avatar May 23 '22 15:05 kolbma

Error still exists in last restify version 11.1.0.

Example:

server.get(
  "/doc/*",
  restify.plugins.serveStatic({
    directory:"./api-doc",
    default: "index.html",
    maxAge: 1,
    appendRequestPath: false
  })
);

Access to /doc/ produce error:

ENOENT: no such file or directory, stat ".../api-doc/doc"

Access to /doc/index.html works

deguich avatar May 15 '23 08:05 deguich