lwsjs / local-web-server

A lean, modular web server for rapid full-stack development.
MIT License
1.22k stars 86 forks source link

Redirect with added trailing slash if serving a directory #134

Closed bjornharrtell closed 4 years ago

bjornharrtell commented 5 years ago

Currently, if making a request against a subfolder in the root that has an index.html, without any trailing slash, the index.html will be fetched but any relative paths in that index.html will point one folder up as the browser thinks it is actually showing a file with the folder name.

I believe a better behaviour would be to redirect with added trailing slash if serving a directory.

This would also be the same behaviour as the default in Apache, see DirectorySlash directive at http://httpd.apache.org/docs/2.2/mod/mod_dir.html.

75lb commented 5 years ago

which version of local-web-server are you using, have you updated it recently? This issue sounds very similar to one which was fixed back in July..

bjornharrtell commented 5 years ago

I'm at 3.0.7. This seems to be the same issue as described at https://github.com/koajs/static/issues/130.

75lb commented 5 years ago

Ok, thanks. Ideally, this issue will be resolved upstream in Koa but I'll leave this open in the meantime with a relatively low priority.

If you have some spare time, you could add a --static.directory-slash option (or similar) to lws-static to implement the redirect and submit a PR. Obviously, implementing this option will mean, for every incoming request, hitting the disk to stat whether the requested resource is a directory - this would introduce a small performance cost so arguably should not be the default behaviour. At least not yet.

I have chimed in on the koa-static issue - will try and push for a fix upstream. Let's see what happens.

bjornharrtell commented 5 years ago

Thanks, sounds good. I will try to find out how to implement that option. :)

75lb commented 5 years ago

OK, cool.. in the wiki, read the docs on using and creating lws middleware modules - that will help get you started.

The Koa docs on how to return a 302 Redirect response from middleware are here.

After this line, you could test whether a directory is being requested. If it is, programmatically add a / to the end of the request path using ctx.request.path.

bjornharrtell commented 5 years ago

On futher investigation it appears it actually involves koa-send. There has been an issue about this exact issue there since 2017 - https://github.com/koajs/send/issues/81. The fix is simple, I've made a PR at https://github.com/koajs/send/pull/125 but activity seems very low on the repo so I don't have high hopes for it getting merged anytime soon.

I was unable to understand how to customize the existing lws-static since it it just wraps koa-static, but I did make a separate DirectorySlash middleware that could do it but it also needs to resolve the local path and stat the file of course... mabye it makes sense to finish that one anyway considering that koa-send improvements might take forever to land?

bjornharrtell commented 5 years ago

One problem with the separate middleware is that it is of course not aware of what is the root of lws-static.. :(

bjornharrtell commented 5 years ago

Found out how to respect the the --directory option so here my bid on a standalone middleware that seems to work. The path resolution logic is more or less taken from koa-send so that it should work the same.

const EventEmitter = require('events')
const resolvePath = require('resolve-path')
const fs = require('fs')
const {
  normalize,
  resolve,
  parse
} = require('path')

class DirectorySlash extends EventEmitter {
  description() {
    return 'Redirects directories without slash with slash.'
  }
  middleware(config) {
    return async(ctx, next, opts = {}) => {
      opts.root = opts.root || config.directory || process.cwd()
      let path = ctx.path
      const root = opts.root ? normalize(resolve(opts.root)) : ''
      const trailingSlash = path[path.length - 1] === '/'
      path = path.substr(parse(path).root.length)
      path = decode(path)
      if (path === -1) return ctx.throw(400, 'failed to decode')
      const localPath = resolvePath(root, path)
      const stats = fs.statSync(localPath)
      if (stats.isDirectory() && !trailingSlash)
        ctx.redirect(ctx.path + '/')
      await next()
    }
  }
}

function decode(path) {
  try {
    return decodeURIComponent(path)
  } catch (err) {
    return -1
  }
}

module.exports = DirectorySlash
75lb commented 5 years ago

OK, sounds good thanks. I think solving this as a separate middleware plugin makes sense to begin with, will give us chance to check it plays well with other middleware (e.g. lws-spa).

If you publish this, be sure to follow the instructions here to ensure it appears on the lws plugin list.