librespeed / speedtest-go

Go backend for LibreSpeed
GNU Lesser General Public License v3.0
706 stars 153 forks source link

return 404 Not Found instead of 403 Forbidden #6

Closed xiruizhao closed 3 years ago

xiruizhao commented 3 years ago

Currently, GET requests return 404 only for non-existent html and js files and 403 for other non-existent uri. Since nothing confidential is stored in assets, handler for /* should be a simple file sever rooted at assets.

maddie commented 3 years ago

I'm not quite sure about this PR, listing the directory to the outside might not be a good idea, even if it doesn't contain sensitive data. The users might not be aware that the assets directory gets listed and might accidentally put sensitive files into it.

It was implemented as a simple HTTP file server initially, but I changed it to return 403 instead.

But your idea is totally valid, it should return 404 instead of 403. I'll leave this here for now and try find a way to implement this properly.

Thanks for your patch.

xiruizhao commented 3 years ago

I changed it to return 404 on all requests other than for html and js files. Curiously, WriteHeader(http.StatusNotFound) appends a body of 404 page not found (tested with curl) while WriteHeader(http.StatusForbidden) leaves an empty body (tested with curl; browser will display a blank page with no indication of error; tested on Safari and Firefox on macOS), which is why I disliked it.

maddie commented 3 years ago

I think that's chi's behavior. I'll take a look at it later tonight.

maddie commented 3 years ago

I have pushed commit 6970d87 to solve this problem. Let me know if the new behavior is OK for you.

I'll close this PR for now. Thank you!