Conversation
There was a problem hiding this comment.
Pull request overview
This pull request adds WebDAV support to the HTTP static file server, enabling WebDAV clients to interact with the server using standard WebDAV operations. The changes integrate the golang.org/x/net/webdav package and modify authentication and request handling to support WebDAV methods.
Changes:
- Integrated WebDAV handler using
golang.org/x/net/webdavpackage for file operations - Added new route handler for WebDAV-specific HTTP methods (OPTIONS, PROPFIND, PUT, LOCK, UNLOCK, MKCOL, MOVE, PROPPATCH)
- Modified authentication to support Basic Auth for WebDAV access alongside existing session-based authentication
- Removed OPTIONS handling from CORS middleware and updated Go version to 1.23.0
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 8 comments.
| File | Description |
|---|---|
| main.go | Removed OPTIONS request handling from CORS middleware |
| httpstaticserver.go | Added WebDAV handler initialization, new hWebdav method, canWebdavAccess authentication method, and modified hIndex and hDelete to use WebDAV handler |
| go.mod | Updated Go version to 1.23.0, promoted golang.org/x/net to direct dependency, reorganized indirect dependencies |
| go.sum | Cleaned up unused dependency checksums for various golang.org/x packages |
Comments suppressed due to low confidence (1)
httpstaticserver.go:229
- The code after the return statement on line 217 is unreachable and will never execute. Lines 218-229 contain the original delete logic including error handling that has been bypassed by the early return. This means delete operations are now handled only by the WebDAV handler without the original safety checks and error reporting.
s.Handler.ServeHTTP(w, req)
return
// TODO: path safe check
err := os.RemoveAll(realPath)
if err != nil {
pathErr, ok := err.(*os.PathError)
if ok {
http.Error(w, pathErr.Op+" "+path+": "+pathErr.Err.Error(), 500)
} else {
http.Error(w, err.Error(), 500)
}
return
}
w.Write([]byte("Success"))
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| func (c *AccessConf) canWebdavAccess(w http.ResponseWriter, r *http.Request) bool { | ||
| if len(c.Users) == 0 { | ||
| return true | ||
| } | ||
| w.Header().Set("WWW-Authenticate", `Basic realm="Restricted"`) |
There was a problem hiding this comment.
The canWebdavAccess method should only set the WWW-Authenticate header when authentication fails, not on every call. Currently, the header is set even when authentication succeeds (when Users list is empty or valid credentials are provided), which can cause clients to unnecessarily prompt for credentials. Move the header setting to just before returning false.
There was a problem hiding this comment.
@copilot open a new pull request to apply changes based on this feedback
| if !auth.canDelete(req) { | ||
| http.Error(w, "Delete forbidden", http.StatusForbidden) | ||
| return | ||
| if !auth.canWebdavAccess(w, req) { | ||
| http.Error(w, "Delete forbidden", http.StatusForbidden) | ||
| return | ||
| } |
There was a problem hiding this comment.
The authorization logic is confusing and potentially incorrect. The code checks if delete is NOT allowed, and if so, it then checks WebDAV access. This creates a fallback where if session-based delete is forbidden, WebDAV access with basic auth can still proceed. This may unintentionally allow delete operations through WebDAV when they should be forbidden. The logic should be clearer about when each authentication method applies and whether they should be alternatives or both required.
There was a problem hiding this comment.
@copilot open a new pull request to apply changes based on this feedback
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
No description provided.