HTTP::StaticFileHandler returns 302 Found for directory requests missing trailing slash even when directory_listing: false

[cristian-lsdb]

When HTTP::StaticFileHandler is initialized with directory_listing: false, the handler behaves as expected (returning a 404 Not Found) when requesting a directory with the trailing slash /foo/. But, if the trailing slash is missing /foo, it redirects to /foo/ with a 302 Found response.

# To run this example, create a directory named `foo` in the same directory as this script.

require "http/server/handler"
require "http/client/response"

def handle(request, fallthrough = true, directory_listing = true, ignore_body = false, decompress = true)
  io = IO::Memory.new
  response = HTTP::Server::Response.new(io)
  context = HTTP::Server::Context.new(request, response)
  handler = HTTP::StaticFileHandler.new "./", fallthrough, directory_listing
  handler.call context
  response.close
  io.rewind
  HTTP::Client::Response.from_io(io, ignore_body, decompress)
end

response = handle HTTP::Request.new("GET", "/foo/"), directory_listing: false
pp response.status_code # => 404

response = handle HTTP::Request.new("GET", "/foo"), directory_listing: false
pp response.status_code # => 302
pp response.headers["Location"] # => "/foo/"

In production this can lead to an infinite redirect loop when the Crystal application is behind Nginx and Nginx is configured to remove all trailing slashes.

And I guess this can also be considered as a security risk since it is possible to check if a directory exists on the server.

[Blacksmoke16]

This would mean reverting #2544. Which I'm not really sure want to do given there seems to be support for this via https://datatracker.ietf.org/doc/html/rfc3986#section-6.2.4.

[cristian-lsdb]

These are two distinct cases.

When #2544 was opened, the directory_listing option did not exist, and it was assumed that it was enabled by default.

Since then, it is possible to disable it with directory_listing: false and this is the case that is problematic:

• The client requests /foo.
• HTTP::StaticFileHandler responds with a redirect to /foo/.
• The client requests /foo/.
• HTTP::StaticFileHandler responds with a 404 Not Found.

In my opinion, the expected behavior should be that on the first request to /foo, HTTP::StaticFileHandler directly responds with a 404 Not Found.

I do not question the redirect behavior when directory_listing is set to true.

[straight-shoota]

Yeah this looks like it should be a simple fix to disable the directory redirect when directory_listing: false. If the handler doesn't do directory listings, it shouldn't care about redirecting directory paths.

That's far from rollinb back #2544.