feat(webdav): implement RFC 4331 WebDAV Quotas for server and client(driver)#2040
feat(webdav): implement RFC 4331 WebDAV Quotas for server and client(driver)#2040hcrgm wants to merge 1 commit intoOpenListTeam:mainfrom
Conversation
xrgzs
added
enhancement
Module: Driver
There was a problem hiding this comment.
Pull request overview
This PR implements RFC 4331 WebDAV Quotas support for both the server and client (driver) components of OpenList. The implementation adds quota-available-bytes and quota-used-bytes live properties to WebDAV PROPFIND responses on the server side, and provides a Quota method in the gowebdav client package to retrieve this information. The driver for WebDAV storage now implements the GetDetails interface to expose storage quota information.
Changes:
- Added RFC 4331 WebDAV quota properties (quota-available-bytes and quota-used-bytes) to the server-side WebDAV implementation
- Implemented client-side quota retrieval in the gowebdav package with a new Quota method
- Extended the WebDAV driver to implement the GetDetails interface for exposing storage quota information
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| server/webdav/prop_additional.go | New file implementing RFC 4331 quota properties as additional live properties that are excluded from allprop responses |
| server/webdav/prop.go | Modified propnames function signature to support filtering additional live properties for allprop requests, and integrated quota property handling in props function |
| server/webdav/webdav.go | Updated propnames function call to pass the fromAllprop parameter |
| pkg/gowebdav/quota.go | New file defining the Quota struct for client-side quota representation |
| pkg/gowebdav/client.go | Added Quota method to retrieve quota information and extended props struct with quota fields |
| drivers/webdav/driver.go | Implemented GetDetails method for the WebDAV driver to expose storage quota information |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| func (c *Client) Quota(path string) (*Quota, error) { | ||
| var quota *Quota | ||
| parse := func(resp interface{}) error { | ||
| r := resp.(*response) | ||
| if p := getProps(r, "200"); p != nil && quota == nil { | ||
| quota = new(Quota) | ||
| quota.Available = p.QuotaAvailableBytes | ||
| quota.Used = p.QuotaUsedBytes | ||
| } | ||
|
|
||
| r.Props = nil | ||
| return nil | ||
| } |
There was a problem hiding this comment.
The Quota method doesn't validate that both properties are present in the response before returning. If the server only returns one of the two quota properties (quota-available-bytes or quota-used-bytes), the zero value (0) will be used for the missing field, which could be misleading. Consider checking if both properties were actually returned by the server and return nil if essential quota information is missing or incomplete.
| if usage == nil { | ||
| return "", false, nil | ||
| } | ||
| available := usage.TotalSpace - usage.UsedSpace |
There was a problem hiding this comment.
Available space calculation can produce negative values if UsedSpace exceeds TotalSpace. While this may represent a valid over-quota situation, RFC 4331 states that quota-available-bytes should be the number of bytes available, which cannot be negative. Consider checking if the result is negative and returning 0 or an appropriate value in such cases to conform with the RFC specification.
| available := usage.TotalSpace - usage.UsedSpace | |
| available := usage.TotalSpace - usage.UsedSpace | |
| if available < 0 { | |
| available = 0 | |
| } |
| @@ -222,7 +237,7 @@ func props(ctx context.Context, ls LockSystem, fi model.Obj, pnames []xml.Name) | |||
| } | |||
|
|
|||
| // Propnames returns the property names defined for resource name. | |||
There was a problem hiding this comment.
The propnames function signature was changed to include a fromAllprop parameter, but the function documentation comment was not updated to explain this new parameter's purpose. Consider adding documentation to explain that the fromAllprop parameter controls whether additionalLiveProps should be included in the returned property names list, following RFC 4331 section 3 which specifies that quota properties should not be returned for allprop PROPFIND requests.
| // Propnames returns the property names defined for resource name. | |
| // Propnames returns the property names defined for the given resource. | |
| // The fromAllprop parameter indicates whether this call is serving an | |
| // "allprop" PROPFIND request. When fromAllprop is true, additionalLiveProps | |
| // (such as quota-related properties) are omitted from the result, in | |
| // accordance with RFC 4331 section 3, which specifies that quota properties | |
| // are not returned for allprop requests. |
| if quota == nil { | ||
| return nil, errs.NotImplement | ||
| } | ||
| total := quota.Available + quota.Used |
There was a problem hiding this comment.
Potential arithmetic overflow when calculating total space. If quota.Available and quota.Used are both close to the maximum value of int64, their sum could overflow. According to RFC 4331, quota-available-bytes represents the available bytes remaining, not necessarily part of a fixed total. Consider checking if the server provides an explicit total space value, and handle cases where Available + Used might not accurately represent total capacity.
| // findFn implements the propfind function of this property. If nil, | ||
| // it indicates a hidden property. | ||
| findFn func(context.Context, LockSystem, string, model.Obj) (string, bool, error) | ||
| // showFn indicates whether the prop show be returned for the resource. |
There was a problem hiding this comment.
Spelling error in the comment. Change "show be" to "should be" for grammatical correctness.
| // showFn indicates whether the prop show be returned for the resource. | |
| // it indicates a hidden property. | |
| // showFn indicates whether the prop should be returned for the resource. |
| func getStorageUsage(ctx context.Context, fi model.Obj) (*model.StorageDetails, error) { | ||
| user := ctx.Value(conf.UserKey).(*model.User) | ||
| reqPath, err := user.JoinPath(fi.GetPath()) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| storage, err := fs.GetStorage(reqPath, &fs.GetStoragesArgs{}) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| details, err := op.GetStorageDetails(ctx, storage) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| return details, nil | ||
| } |
There was a problem hiding this comment.
Error handling should be improved. When GetStorageDetails returns an error (especially errs.NotImplement or errs.StorageNotInit), the function propagates it as-is. According to RFC 4331, if quota information is unavailable, the properties should be omitted from the response rather than causing the entire PROPFIND to fail. Consider checking for NotImplement and StorageNotInit errors and returning (nil, nil) in those cases to gracefully handle storages that don't support quota reporting.
2 participants
Description / 描述
为 WebDAV 服务器和客户端（驱动）实现 RFC 4331，从而展示和对外提供空间用量信息。
Motivation and Context / 背景
How Has This Been Tested? / 测试
使用支持 WebDAV Quota 的客户端（如 Rclone）和服务器测试。
Checklist / 检查清单
我已阅读 CONTRIBUTING 文档。
go fmtor prettier.我已使用
go fmt或 prettier 格式化提交的代码。我已为此 PR 添加了适当的标签（如无权限或需要的标签不存在，请在描述中说明，管理员将后续处理）。
我已在适当情况下使用"Request review"功能请求相关代码作者进行审查。
我已相应更新了相关仓库（若适用）。