add an option for directory listing, default disabled

[sqs]

Thanks for the great package!

This PR makes the directory listing feature require an explicit option DirListing. This prevents users from mistakenly enabling directory listings for assets, which could be bad for users wanting assets only to be accessible to end users who know the specific path names.

I think making this feature opt-in is better because this library describes itself as provid[ing] net/http-like primitives. The net/http package's FileServer type does not provide directory listings, and directory listings are arguably not a "primitive".

[dmitshur]

Hey Quinn, thanks for the PR!

Good news, this change is very much in scope for this package. The reason it exists is to add gzip-compression behavior to http.FileServer, but also to make it customizable. So having an option to control directory listing is a great fit.

However, I'm not completely sure what its default value should be. I see some tradeoffs. But first, let me ask about something you said:

The net/http package's FileServer type does not provide directory listings

Can you tell me what makes you say that? As far as I know, http.FileServer most definitely serves directory listings (and has no way to turn that off). This behavior is responsible for the vast majority of the code for http.FileServer. Serving directory listings is more involved than just serving files, since you need to render some HTML, etc. In code, see https://github.com/golang/go/blob/a35377515f6a232305a52f89c164/src/net/http/fs.go#L611-L620.

So, here are the tradeoffs between the two options:

• Directory listing on by default is consistent with net/http and its http.FileServer. The goal of httpgzip.FileServer to be a drop-in replacement, so matching behavior with zero options makes sense.

• Hiding directory listing may provide a false sense of security. Unless you take care to ensure names of files in a directory are equivalent to passwords in terms of their length and content, someone can still find them without a directory listing by guessing. So, making it disabled by default may not be helpful.

• Displaying directory listing may be somewhat expensive, if the directory is large, or if listing all files is an expensive operation (in case of a dynamic virtual filesystem). It may not be neccessary for a production use, so having it off by default may potentially help reduce load on the server. However, displaying directory listing may be helpful during development. (I would prefer to optimize the defaults for production use, however.)

• As you said, 'directory listings are arguably not a "primitive"', and I agree. How to serve a file has a fairly common answer, but how to serve a directory listing is completely custom code that the user might want to customize (e.g., add header/footer to match the rest of website theme, set meta tags, etc.) or disable. Another API to consider is like FileServerOptions.ServeError, where the user could provide a custom handler to serve directories, and httpgzip could supply a default one.

• Hiding directory listing by default would be a breaking behavior API change, which means all existing users of httpgzip would potentially need to be updated.

So, at this point, it's a bit unclear whether it should be made on or off by default. Making it off would be a breaking behavior and users need to be updated, but I don't mind making those fixes and sending PRs in clients if the API is in fact better. But, at this point, I'm not convinced which default is actually better. Thoughts?

[sqs]

Thanks for the thoughtful reply. I apparently was mistaken about the standard library’s http package not offering directory listings. I personally think it introduces an additional security concern and should be off by default (everywhere, even Apache httpd, etc.), but it’s your call.

[dmitshur]

I personally think it introduces an additional security concern and should be off by default (everywhere, even Apache httpd, etc.)

Fair enough. Just checking, but what did you think of my point that it might offer a false sense of security unless filenames are treated as sensitive tokens, i.e., they need to generated using a cryptographically secure source of randomness, and be long enough, otherwise they can be guessed?

[sqs]

I see the argument but I personally don’t agree. Usually the argument comes up when one considers whether to add something that marginally increases perceived security, not when one considers whether to remove something that marginally decreases actual security. I don’t begrudge anyone who disagrees though! Both ways are valid.

[tsenart]

We have the need to disable this again. Instead of forking and using our fork, would it be OK to merge this if the default remained true?