expressjs / serve-static

Serve static files
MIT License
1.38k stars 227 forks source link

Fix h3 Error in Nuxt 3.0.0-rc.12 #152

Closed hslee2008 closed 1 year ago

hslee2008 commented 1 year ago

Related Issue

151

Test

$ mocha --reporter spec --bail --check-leaks test/

  serveStatic()
    basic operations
      ✔ should require root path
      ✔ should require root path to be string
      ✔ should serve static files (97ms)
      ✔ should support nesting
      ✔ should set Content-Type
      ✔ should set Last-Modified
      ✔ should default max-age=0
      ✔ should support urlencoded pathnames
      ✔ should not choke on auth-looking URL
      ✔ should support index.html
      ✔ should support ../
      ✔ should support HEAD
      ✔ should skip POST requests
      ✔ should support conditional requests
      ✔ should support precondition checks
      ✔ should serve zero-length files
      ✔ should ignore hidden files
    current dir
      ✔ should be served with "."
    acceptRanges
      when false
        ✔ should not include Accept-Ranges
        ✔ should ignore Rage request header
      when true
        ✔ should include Accept-Ranges
        ✔ should obey Rage request header
    cacheControl
      when false
        ✔ should not include Cache-Control
        ✔ should ignore maxAge
      when true
        ✔ should include Cache-Control
    extensions
      ✔ should be not be enabled by default
      ✔ should be configurable
      ✔ should support disabling extensions
      ✔ should support fallbacks
      ✔ should 404 if nothing found
    fallthrough
      ✔ should default to true
      when true
        ✔ should fall-through when OPTIONS request
        ✔ should fall-through when URL malformed
        ✔ should fall-through when traversing past root
        ✔ should fall-through when URL too long (42ms)
        with redirect: true
          ✔ should fall-through when directory
          ✔ should redirect when directory without slash
        with redirect: false
          ✔ should fall-through when directory
          ✔ should fall-through when directory without slash
      when false
        ✔ should 405 when OPTIONS request
        ✔ should 400 when URL malformed
        ✔ should 403 when traversing past root
        ✔ should 404 when URL too long
        with redirect: true
          ✔ should 404 when directory
          ✔ should redirect when directory without slash
        with redirect: false
          ✔ should 404 when directory
          ✔ should 404 when directory without slash
    hidden files
      ✔ should be served when dotfiles: "allow" is given
    immutable
      ✔ should default to false
      ✔ should set immutable directive in Cache-Control
    lastModified
      when false
        ✔ should not include Last-Modifed
      when true
        ✔ should include Last-Modifed
    maxAge
      ✔ should accept string
      ✔ should be reasonable when infinite
    redirect
      ✔ should redirect directories
      ✔ should include HTML link
      ✔ should redirect directories with query string
      ✔ should not redirect to protocol-relative locations
      ✔ should ensure redirect URL is properly encoded
      ✔ should respond with default Content-Security-Policy
      ✔ should not redirect incorrectly
      when false
        ✔ should disable redirect
    setHeaders
      ✔ should reject non-functions
      ✔ should get called when sending file
      ✔ should not get called on 404
      ✔ should not get called on redirect
    when non-existent root path
      ✔ should 404 for any file
      ✔ should not allow traversal
    when traversing past root
      ✔ should catch urlencoded ../
      ✔ should not allow root path disclosure
    when request has "Range" header
      ✔ should support byte ranges
      ✔ should be inclusive
      ✔ should set Content-Range
      ✔ should support -n
      ✔ should support n-
      ✔ should respond with 206 "Partial Content"
      ✔ should set Content-Length to the # of octets transferred
      when last-byte-pos of the range is greater than current length
        ✔ is taken to be equal to one less than the current length
        ✔ should adapt the Content-Length accordingly
      when the first- byte-pos of the range is greater than the current length
        ✔ should respond with 416
        ✔ should include a Content-Range header of complete length
      when syntactically invalid
        ✔ should respond with 200 and the entire contents
    when index at mount point
      ✔ should redirect correctly
    when mounted
      ✔ should redirect relative to the originalUrl
      ✔ should not choke on auth-looking URL
    when mounted "root" as a file
      ✔ should load the file when on trailing slash
      ✔ should 404 when trailing slash
    when responding non-2xx or 304
      ✔ should respond as-is
    when index file serving disabled
      ✔ should next() on directory
      ✔ should redirect to trailing slash
      ✔ should next() on mount point
      ✔ should redirect to trailing slash mount point

  92 passing (1s)

Done in 2.24s.

Note

I'm not exactly sure if this would fix the problem or even if this will work because I have no experience in this field. I also have no idea how to test this in Nuxt. I took some notes from https://github.com/nuxt-modules/tailwindcss/pull/545

dougwilson commented 1 year ago

If this is the fix, it seems like it is what I was saying in the issue. If so, you need to call the eventHandler() in the same place in your code you are using this middleware, not in the middleware itself like in this pull request. Pulling in the h3 module breaks the Express.js usage, whixh of course we need to support as an Express.js module.

So your code should be like

var serveStatic = require('serve-static')
var { eventHandler } = require('h3')

// various setup, until...
// when you call serveStatic then wrap that result with eventHandler

eventHandler(serveStatic(/* options */))

I will note that I used eventHandler function above as that is what you used in your pull request. The changelog, to me, seems to indicate you need to use fromNodeMiddleware, but I am not familiar with the framework. It may be best to open an issue against the h3 framework to help as well if you are not sure.

My understanding, and even shown in that PR you linked, is that the function passed to eventHandler needs to be one that only takes the event argument, not the req, res, next arguments. You can see that change in the PR you link, but you didn't change it here. Since eventHandler just returns the original function, our test suite passes still (at least on the versions not broken by importing h3), but that doesn't show it would work in the h3 framework itself since no tests were added for that.