add --disable-indexing cli flag to completely disable directory indexing
#1329
Conversation
|
Please add a test for this. It's a fairly impactful change. |
src/listing.rs
Outdated
| if conf.disable_indexing { | ||
| return Ok(ServiceResponse::new( | ||
| req.clone(), | ||
| HttpResponse::NotFound() | ||
| .content_type(mime::TEXT_PLAIN_UTF_8) | ||
| .body("File not found."), | ||
| )); | ||
| } | ||
|
|
There was a problem hiding this comment.
Doesn't putting it here result in the index still being downloadable via requesting an archive? This seems to me like a potential security problem or at least something people might not expect. I think it might be safer to prohibit index generation entirely in main.rs.
There was a problem hiding this comment.
I wasn't aware that was a thing. I'll take a look.
There was a problem hiding this comment.
After looking at it closer, I think it'll be more clean to just move this check all the way to the top of this function. All file listings seem to go through this handler.
Plus, it seems to render a nicer looking error page when it's handled here.
|
I added a couple tests. I'm not super familiar with your codebase, so let me know if there's some better assertions I could use. |
| if conf.disable_indexing { | ||
| return Ok(ServiceResponse::new( | ||
| req.clone(), | ||
| HttpResponse::NotFound() | ||
| .content_type(mime::TEXT_PLAIN_UTF_8) | ||
| .body("File not found."), | ||
| )); | ||
| } |
There was a problem hiding this comment.
I think this is probably fine but I'm wondering: Can we just disable indexing entirely in main.rs?
There was a problem hiding this comment.
Technically yes, and if you want to see what that looks like, you can check out 7b5b1a0. Reiterating what I said in this comment I think it would be less complicated to do it like this.
|
Excellent stuff, tests are spot on, too! |
closes #1328