Skip to content
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

Restify.plugins.serverStatic option "appendRequestPath" is bugged #1604

Open
keithlee96 opened this issue Feb 11, 2018 · 16 comments
Open

Restify.plugins.serverStatic option "appendRequestPath" is bugged #1604

keithlee96 opened this issue Feb 11, 2018 · 16 comments
Labels
Bug Good First Issue Serve Label to identify bottlenecks for serve static

Comments

@keithlee96
Copy link

  • [ 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 keithlee96 changed the title Restify.plugins.serverStatic option "appendRequestPath" is not working Restify.plugins.serverStatic option "appendRequestPath" is bugged Feb 11, 2018
@PavelPolyakov
Copy link

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,

@retrohacker
Copy link
Member

retrohacker commented Feb 13, 2018

Hey @keithlee96,

First, thank you for the thorough bug report with detailed examples ❤️.

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
Copy link
Member

@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.

@kingnebby
Copy link

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!

@retrohacker
Copy link
Member

@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.

@hekike
Copy link
Member

hekike commented Feb 13, 2018

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.

@kingnebby
Copy link

kingnebby commented Feb 13, 2018

@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.

@phpmaps
Copy link

phpmaps commented Mar 16, 2018

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.

@irokhes
Copy link

irokhes commented Mar 24, 2018

@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!!

@kingnebby
Copy link

@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.

@irokhes
Copy link

irokhes commented Mar 25, 2018

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

Thanks!

@kingnebby
Copy link

@irokhes

For starters, here's a sample project.

@hekike hekike added the Serve Label to identify bottlenecks for serve static label Apr 10, 2018
@himbaer
Copy link

himbaer commented Jan 16, 2019

Hey,

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

If Not, I would really appreciate a deprecation msg.

Cheers and thx

@jmbeltran07
Copy link

jmbeltran07 commented Oct 26, 2020

Hi! I would like to contribute to this project!
I created PR: #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!

@kolbma
Copy link

kolbma commented May 23, 2022

See #1910

kolbma added a commit to kolbma/node-restify that referenced this issue May 26, 2022
Fixes restify#1604
There should be no dependency on the dirname for specific path.
kolbma added a commit to kolbma/node-restify that referenced this issue May 26, 2022
@deguich
Copy link

deguich commented May 15, 2023

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Good First Issue Serve Label to identify bottlenecks for serve static
Projects
None yet
Development

No branches or pull requests