Refactor WebDAV protocol into aster_forge_webdav crate with breaking changes#432
Conversation
Extracted WebDAV protocol handling, XML parsing, lock management, and HTTP utilities from the monolithic implementation into a standalone `aster_forge_webdav` crate. This consolidates reusable WebDAV contracts and reduces coupling between Drive-specific storage logic and protocol compliance.
**Breaking changes:**
- Moved core types (`DavFileSystem`, `DavLockSystem`, `DavFile`, `DavMetaData`, `DavPath`) to `aster_forge_webdav`
- Renamed `EntityType` property tracking to `DavResourceKind` with `property_target()` method
- Removed public `webdav::dav` module; consumers import from `aster_forge_webdav` directly
- Changed XML handling from `xmltree::Element` to crate-internal `DavXmlElement`
- Replaced inline HTTP range parsing with shared `aster_forge_utils::http_range`
**Implementation:**
- Reorganized `src/webdav/` structure: `fs/` → `backend/`, handlers split into `handlers/{transfer,properties,resources,locks,deltav}`
- Delegated PROPFIND/PROPPATCH/LOCK/UNLOCK protocol planning to crate functions
- Unified `DavResponse`/`DavResponseBody` conversion via `aster_forge_webdav::actix::into_response`
- Eliminated `webdav::responses` redundant builder helpers; protocol crate owns canonical responses
- Applied atomic PROPPATCH conflict detection via `plan_atomic_proppatch`
- Removed `xml_depth_probe` test (stack overflow risk now handled by crate's recursive depth limit)
**Non-breaking:**
- Preserved all WebDAV method semantics, lock semantics, and audit hooks
- Maintained Drive-specific storage adapters (`AsterDavFs`, `AsterDavFile`, `DbLockSystem`)
- Kept test coverage for path resolution, lock lifecycle, and litmus compliance
**Dependencies:**
- Added `aster_forge_webdav` with `actix` feature for request/response adaptation
- Updated `aster_forge_utils` for shared HTTP range and validation utilities
- Applied local path overrides for cross-crate development
- Replace string-based XML assertions with structured element checks - Add `contains_element` helper to recursively search parsed XML tree - Update propname test to use `DavXmlElement::parse` instead of string matching - Relax namespace declaration assertion to check element presence only - Add `aster_forge_xml` dependency to support structured XML validation
- Remove `src/webdav/protocol.rs` module (112 lines) - Remove local `completed_event`, `lock_info`, `lock_xml`, `lockdiscovery_element`, `supportedlock_element` implementations - Remove `ensure_lock_target_exists`, `ensure_parent_exists`, `ensure_unlocked`, `ensure_parent_unlocked` helpers - Remove `replace_relative_prefix` and associated tests - Remove unused response helpers (`bad_request`, `invalid_request_path`) - Remove `completed_event_uses_the_forge_request_head_without_sensitive_request_data` test Replace with shared implementations from `aster_forge_webdav`: - Use `actix::converted_headers`, `actix::protocol_error_response`, `actix::enforce_if_header_with_backends` - Use `actix::evaluate_http_etag_preconditions`, `actix::enforce_unlocked`, `actix::enforce_parent_unlocked` - Use `ensure_lock_target_exists`, `enforce_parent_collection`, `unsubmitted_lock_conflicts` - Use `dav_supported_lock_element`, `lock_discovery_element`, `replace_relative_prefix` - Use `DavEvent::completed` builder with standardized signature - Use `Depth` type directly from crate root BREAKING CHANGE: Internal protocol bridge removed in favor of shared WebDAV crate utilities
- Bump all aster_forge_* crates to commit 77bcc8e (from 19e82ac) - Remove local path patches for aster_forge_utils and aster_forge_webdav from Cargo.toml - Replace DavLockError unit errors () with typed DavLockError variants (TokenMismatch, Backend, Conflict) in unlock, refresh, and delete methods - Remove DavPropertyTarget and DavResourceKind from metadata and filesystem backend; replace get_props_many_for_targets with get_props_many using plain DavPath slices - Remove property_target field and method from AsterDavMeta and DavMetaData impl - Remove get_props_many_for_targets implementation from AsterDavFs - Switch normalize_path from String::from_utf8_lossy to DavPath::as_str - Propagate child_relative_path errors instead of ignoring them in properties and resources handlers - Update lock error handling in handle_lock and handle_unlock to match on new DavLockError variants - Update warning logs for lock deletion failures to include the error value - Fix test: split unsupported range forms (return 200) from unsatisfiable ranges (return 416) - Fix test: clamp oversized Timeout values instead of rejecting with 400 - Fix test: If header evaluates tagged-list productions with OR semantics, not AND
AptS-1547
added
Documentation
📝 WalkthroughWalkthrough本次变更将 WebDAV 协议契约、XML、请求解析、锁、属性、资源、传输和错误映射迁移至 ChangesWebDAV 协议边界迁移
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@coderabbitai review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (6)
src/webdav/mod.rs (1)
44-64: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win失败事件用
debug!记录，等于没记录。
DavEventOutcome::Failed分支落在tracing::debug!，生产环境默认级别下 4xx/5xx 的 WebDAV 操作会全部静默。另外所有DavEvent::completed(...)调用点的backend_error都传None，所以 sink 里那个backend_error = ?backend_error字段永远是None——纯装饰。♻️ 建议把失败路径提级
DavEventOutcome::Failed { status, backend_error, - } => tracing::debug!( + } => tracing::warn!( operation = ?event.operation,Also applies to: 272-277
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/webdav/mod.rs` around lines 44 - 64, Update the DavEventOutcome::Failed branch in the event sink to use an error-level tracing macro instead of debug!, while preserving its existing operation, status, backend_error, destination, and elapsed_ms fields. Do not change the successful-event logging path.src/webdav/handlers/deltav.rs (1)
22-28: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueREPORT / VERSION-CONTROL 是纯读路径，可以走 reader。
两个 handler 只做版本历史与目标解析查询，调用方（
src/webdav/mod.rs第148、157行）却传了writer_db()。这类允许短暂滞后的纯读打到写库上，纯粹给主库加压。若不是刻意要读己之写，建议改用reader_db()。按 coding guidelines：“写入、事务、读后写、配额权威判断、登录 session 签发、refresh token rotation、上传 init/chunk/complete/cancel 必须使用 DbHandles 的 writer；reader_db 仅用于允许短暂滞后的纯读路径。”
Also applies to: 125-129
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/webdav/handlers/deltav.rs` around lines 22 - 28, 将 REPORT 与 VERSION-CONTROL 两个纯读 handler 的调用方从 DbHandles::writer_db() 改为 reader_db()，包括 handle_report 及对应的 VERSION-CONTROL handler；保持其余请求处理逻辑不变，仅让版本历史和目标解析查询走 reader。Source: Coding guidelines
src/webdav/backend/lock.rs (1)
371-386: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
map_err(|_| DavLockError::Backend)把底层错误全扔了。同一个文件里
lock()每一步都老老实实tracing::warn!(error = %error, path = %path_str, ...)，unlock/refresh却一个都不留。线上 UNLOCK 返回 500 时你只能靠猜。至少把 repo 错误 warn 出来。♻️ 建议改法
- let lock = lock_repo::find_by_token(&self.db, &token_owned) - .await - .map_err(|_| DavLockError::Backend)? - .ok_or(DavLockError::TokenMismatch)?; + let lock = lock_repo::find_by_token(&self.db, &token_owned) + .await + .map_err(|error| { + tracing::warn!(error = %error, path = %path_str, "failed to look up WebDAV lock by token"); + DavLockError::Backend + })? + .ok_or(DavLockError::TokenMismatch)?;
refresh中 Line 419、426、430 同理。Also applies to: 419-431
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/webdav/backend/lock.rs` around lines 371 - 386, Update unlock and refresh to log each lock_repo error with tracing::warn!, including the underlying error and relevant path context, before converting it to DavLockError::Backend. Preserve the existing error mapping and token/scope behavior while replacing the silent map_err closures in unlock and the corresponding refresh operations.tests/webdav/protocol.rs (1)
6745-6779: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win这个 If 头测试差半步就能真正证明"跨资源 OR"。
现在匹配的那一组（
</webdav/if-tagged-a.txt>）恰好就是请求 URI 本身，所以即便实现根本没做 tagged-list 的 OR、只认了"请求 URI 对应那组"，测试照样绿。把匹配组换成b.txt、让a.txt那组故意不匹配，才是这个用例名字承诺的语义。另外 Line 6758-6762 用
201 || 204太宽松——文件在循环里已经 PUT 过了，这里确定是 204，写死能挡住状态码回归。♻️ 建议补一个反向用例
+ // 仅 b.txt 那一组匹配，请求 URI 是 a.txt —— 真正的跨资源 OR + let req = test::TestRequest::put() + .uri("/webdav/if-tagged-a.txt") + .insert_header(("Authorization", auth.clone())) + .insert_header(( + "If", + format!( + r#"</webdav/if-tagged-a.txt> (["wrong-a"]) </webdav/if-tagged-b.txt> ([{}])"#, + etags[1] + ), + )) + .set_payload("second group matches") + .to_request(); + let resp = test::call_service(&app, req).await; + assert_eq!( + resp.status(), + 204, + "a matching tagged-list production for another resource must still satisfy If" + );🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/webdav/protocol.rs` around lines 6745 - 6779, Update the cross-resource tagged-list test around the PUT requests so the matching If-list belongs to if-tagged-b.txt while the if-tagged-a.txt list is deliberately non-matching, proving OR evaluation across resources rather than request-URI-only matching. In the successful assertion, require exactly HTTP 204 instead of accepting 201 or 204; keep the existing no-match case asserting 412.src/webdav/handlers/resources.rs (1)
509-637: 🗄️ Data Integrity & Integration | 🔵 Trivial | 🏗️ Heavy lift部分递归 COPY/MOVE 里的
?会让"已经改了一半"变成一个干巴巴的 5xx。
partial_recursive_copy_move对锁冲突处理得很漂亮——记进failures、继续走、最后吐 207。但 Line 525、567、595、608、624 这些?一旦命中（存储/DB 抖一下就够），整个函数直接Err(FsError)返回，handle_copy_moveLine 436 把它变成单状态错误响应。此时前面已经复制/删除的那些子项已经落盘了，客户端却收到"整体失败"，只能靠自己重新 PROPFIND 才知道真相。既然这条路径的设计目标就是"部分成功用 207 汇报"，把这些 FsError 也降级成
DavMutationFailure（带对应状态码）继续遍历会更自洽。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/webdav/handlers/resources.rs` around lines 509 - 637, 更新 partial_recursive_copy_move，将目录复制、删除、子节点入队及 collect_children 等当前使用 ? 传播的 FsError 转换为带对应路径和 HTTP 状态码的 DavMutationFailure，追加到 failures 后继续处理剩余工作；保持锁冲突的 207 汇报流程，并确保已完成的部分操作最终通过 PartialMutationOutcome 返回，而不是提前返回整体错误。tests/webdav/lock_system.rs (1)
284-297: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win既然错误契约刚被细化，断言就别停在
is_err()。这个 PR 的重点之一就是把
unlock的()错误换成语义明确的DavLockError::{TokenMismatch, Backend}。可这里只断言"出错了"——哪天 token 查询挂了返回Backend（对客户端是 500 而不是 409），测试照样绿。顺手锁死变体，回归价值立刻翻倍。♻️ 建议改法
- assert!( - lock_system - .unlock(&file_path, "missing-token") - .await - .is_err() - ); + assert!(matches!( + lock_system.unlock(&file_path, "missing-token").await, + Err(DavLockError::TokenMismatch) + )); let other_path = DavPath::new("/other-expired.txt").unwrap(); - assert!( - lock_system - .unlock(&other_path, &replacement.token) - .await - .is_err(), + assert!( + matches!( + lock_system.unlock(&other_path, &replacement.token).await, + Err(DavLockError::TokenMismatch) + ), "UNLOCK must target the locked resource or a resource covered by a deep lock" );依据 coding guidelines：「修改上传完成、配额、版本、删除/恢复、引用计数和清理时覆盖成功及失败/回滚边界」。
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/webdav/lock_system.rs` around lines 284 - 297, Strengthen the unlock assertions in this test to verify the specific DavLockError variant rather than only checking is_err(). Both the missing-token case and the wrong-resource case should assert the expected TokenMismatch variant, ensuring a Backend error does not satisfy the test.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/webdav/backend/download_audit.rs`:
- Around line 36-93: 将 record_download 中的下载审计编排移入明确的 service 方法，并定义或复用下载审计 DTO
传递所需数据。service 应统一处理 should_record、reserve_download_audit_slot、位置详情读取及
audit::log_with_details；WebDAV backend 的 record_download 仅负责组装 WebDAV 请求上下文并调用该
service，保留现有跳过与去重行为。
In `@src/webdav/backend/lock.rs`:
- Around line 788-790: Update serialize_element to stop converting serialization
errors into an empty string: return an optional/error-bearing result and adjust
the lock() owner serialization flow to use owner.and_then(serialize_element) or
return DavLockError::Backend when serialization fails. Ensure any
fire-and-forget failure is emitted with tracing::warn! rather than being
silently discarded.
- Around line 611-612: 统一 normalize_path 与 path_resolver::path_segments
的锁路径解析方式，避免 DavPath::as_str() 与已解码的 path.as_bytes() 产生不同表示。让 resource_lock.path
的写入、匹配及 LOCK、UNLOCK、refresh、check、discover 和 ancestor
查询始终使用同一种可被数据库匹配的规范路径，确保空格、中文及 /# 等路径场景一致。
In `@src/webdav/backend/mod.rs`:
- Around line 803-812: 在 `plan_atomic_proppatch` 调用后的非 `protocol_plan.apply`
分支中，先依据外部 crate 契约校验或补齐 `protocol_plan.statuses`，确保其长度与 `patches`
完全一致；长度不匹配时应按既定错误处理方式返回错误，或为缺失项生成正确状态，避免直接使用 `zip` 丢失属性。保留匹配长度时现有的状态与属性配对行为。
In `@tests/webdav/protocol.rs`:
- Around line 1137-1155: Separate the range cases in the test around the
malformed_range loop: keep bytes=-0 asserting 416 with Content-Range bytes
*/4099, but test bytes=9-5 through the ignored-Range path and assert the
full-response 200 behavior. Add a concise comment documenting the RFC/Litmus
compatibility rationale for treating bytes=9-5 as syntactically invalid and
ignored.
---
Nitpick comments:
In `@src/webdav/backend/lock.rs`:
- Around line 371-386: Update unlock and refresh to log each lock_repo error
with tracing::warn!, including the underlying error and relevant path context,
before converting it to DavLockError::Backend. Preserve the existing error
mapping and token/scope behavior while replacing the silent map_err closures in
unlock and the corresponding refresh operations.
In `@src/webdav/handlers/deltav.rs`:
- Around line 22-28: 将 REPORT 与 VERSION-CONTROL 两个纯读 handler 的调用方从
DbHandles::writer_db() 改为 reader_db()，包括 handle_report 及对应的 VERSION-CONTROL
handler；保持其余请求处理逻辑不变，仅让版本历史和目标解析查询走 reader。
In `@src/webdav/handlers/resources.rs`:
- Around line 509-637: 更新 partial_recursive_copy_move，将目录复制、删除、子节点入队及
collect_children 等当前使用 ? 传播的 FsError 转换为带对应路径和 HTTP 状态码的 DavMutationFailure，追加到
failures 后继续处理剩余工作；保持锁冲突的 207 汇报流程，并确保已完成的部分操作最终通过 PartialMutationOutcome
返回，而不是提前返回整体错误。
In `@src/webdav/mod.rs`:
- Around line 44-64: Update the DavEventOutcome::Failed branch in the event sink
to use an error-level tracing macro instead of debug!, while preserving its
existing operation, status, backend_error, destination, and elapsed_ms fields.
Do not change the successful-event logging path.
In `@tests/webdav/lock_system.rs`:
- Around line 284-297: Strengthen the unlock assertions in this test to verify
the specific DavLockError variant rather than only checking is_err(). Both the
missing-token case and the wrong-resource case should assert the expected
TokenMismatch variant, ensuring a Backend error does not satisfy the test.
In `@tests/webdav/protocol.rs`:
- Around line 6745-6779: Update the cross-resource tagged-list test around the
PUT requests so the matching If-list belongs to if-tagged-b.txt while the
if-tagged-a.txt list is deliberately non-matching, proving OR evaluation across
resources rather than request-URI-only matching. In the successful assertion,
require exactly HTTP 204 instead of accepting 201 or 204; keep the existing
no-match case asserting 412.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: f1ce0fad-a3f5-4704-a06e-91fe77cd2021
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (34)
Cargo.tomlsrc/services/events/storage_change.rssrc/services/files/file/download/build.rssrc/services/files/file/download/range.rssrc/webdav/backend/dir_entry.rssrc/webdav/backend/download_audit.rssrc/webdav/backend/file/mod.rssrc/webdav/backend/file/tests.rssrc/webdav/backend/lock.rssrc/webdav/backend/metadata.rssrc/webdav/backend/mod.rssrc/webdav/backend/path_resolver/cache.rssrc/webdav/backend/path_resolver/mod.rssrc/webdav/dav.rssrc/webdav/deltav.rssrc/webdav/handler_tests/mod.rssrc/webdav/handlers/deltav.rssrc/webdav/handlers/locks.rssrc/webdav/handlers/mod.rssrc/webdav/handlers/properties.rssrc/webdav/handlers/resources.rssrc/webdav/handlers/transfer.rssrc/webdav/locks/mod.rssrc/webdav/mod.rssrc/webdav/protocol.rssrc/webdav/resources/mod.rssrc/webdav/responses.rssrc/webdav/transfer/mod.rstests/webdav/file.rstests/webdav/lock_system.rstests/webdav/main.rstests/webdav/path_resolver.rstests/webdav/protocol.rstests/webdav/xml_depth_probe.rs
💤 Files with no reviewable changes (8)
- tests/webdav/main.rs
- src/webdav/transfer/mod.rs
- src/webdav/locks/mod.rs
- tests/webdav/xml_depth_probe.rs
- src/webdav/deltav.rs
- src/webdav/resources/mod.rs
- src/webdav/protocol.rs
- src/webdav/dav.rs
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 5
🧹 Nitpick comments (6)
src/webdav/mod.rs (1)
44-64: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win失败事件用
debug!记录，等于没记录。
DavEventOutcome::Failed分支落在tracing::debug!，生产环境默认级别下 4xx/5xx 的 WebDAV 操作会全部静默。另外所有DavEvent::completed(...)调用点的backend_error都传None，所以 sink 里那个backend_error = ?backend_error字段永远是None——纯装饰。♻️ 建议把失败路径提级
DavEventOutcome::Failed { status, backend_error, - } => tracing::debug!( + } => tracing::warn!( operation = ?event.operation,Also applies to: 272-277
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/webdav/mod.rs` around lines 44 - 64, Update the DavEventOutcome::Failed branch in the event sink to use an error-level tracing macro instead of debug!, while preserving its existing operation, status, backend_error, destination, and elapsed_ms fields. Do not change the successful-event logging path.src/webdav/handlers/deltav.rs (1)
22-28: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueREPORT / VERSION-CONTROL 是纯读路径，可以走 reader。
两个 handler 只做版本历史与目标解析查询，调用方（
src/webdav/mod.rs第148、157行）却传了writer_db()。这类允许短暂滞后的纯读打到写库上，纯粹给主库加压。若不是刻意要读己之写，建议改用reader_db()。按 coding guidelines：“写入、事务、读后写、配额权威判断、登录 session 签发、refresh token rotation、上传 init/chunk/complete/cancel 必须使用 DbHandles 的 writer；reader_db 仅用于允许短暂滞后的纯读路径。”
Also applies to: 125-129
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/webdav/handlers/deltav.rs` around lines 22 - 28, 将 REPORT 与 VERSION-CONTROL 两个纯读 handler 的调用方从 DbHandles::writer_db() 改为 reader_db()，包括 handle_report 及对应的 VERSION-CONTROL handler；保持其余请求处理逻辑不变，仅让版本历史和目标解析查询走 reader。Source: Coding guidelines
src/webdav/backend/lock.rs (1)
371-386: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
map_err(|_| DavLockError::Backend)把底层错误全扔了。同一个文件里
lock()每一步都老老实实tracing::warn!(error = %error, path = %path_str, ...)，unlock/refresh却一个都不留。线上 UNLOCK 返回 500 时你只能靠猜。至少把 repo 错误 warn 出来。♻️ 建议改法
- let lock = lock_repo::find_by_token(&self.db, &token_owned) - .await - .map_err(|_| DavLockError::Backend)? - .ok_or(DavLockError::TokenMismatch)?; + let lock = lock_repo::find_by_token(&self.db, &token_owned) + .await + .map_err(|error| { + tracing::warn!(error = %error, path = %path_str, "failed to look up WebDAV lock by token"); + DavLockError::Backend + })? + .ok_or(DavLockError::TokenMismatch)?;
refresh中 Line 419、426、430 同理。Also applies to: 419-431
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/webdav/backend/lock.rs` around lines 371 - 386, Update unlock and refresh to log each lock_repo error with tracing::warn!, including the underlying error and relevant path context, before converting it to DavLockError::Backend. Preserve the existing error mapping and token/scope behavior while replacing the silent map_err closures in unlock and the corresponding refresh operations.tests/webdav/protocol.rs (1)
6745-6779: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win这个 If 头测试差半步就能真正证明"跨资源 OR"。
现在匹配的那一组（
</webdav/if-tagged-a.txt>）恰好就是请求 URI 本身，所以即便实现根本没做 tagged-list 的 OR、只认了"请求 URI 对应那组"，测试照样绿。把匹配组换成b.txt、让a.txt那组故意不匹配，才是这个用例名字承诺的语义。另外 Line 6758-6762 用
201 || 204太宽松——文件在循环里已经 PUT 过了，这里确定是 204，写死能挡住状态码回归。♻️ 建议补一个反向用例
+ // 仅 b.txt 那一组匹配，请求 URI 是 a.txt —— 真正的跨资源 OR + let req = test::TestRequest::put() + .uri("/webdav/if-tagged-a.txt") + .insert_header(("Authorization", auth.clone())) + .insert_header(( + "If", + format!( + r#"</webdav/if-tagged-a.txt> (["wrong-a"]) </webdav/if-tagged-b.txt> ([{}])"#, + etags[1] + ), + )) + .set_payload("second group matches") + .to_request(); + let resp = test::call_service(&app, req).await; + assert_eq!( + resp.status(), + 204, + "a matching tagged-list production for another resource must still satisfy If" + );🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/webdav/protocol.rs` around lines 6745 - 6779, Update the cross-resource tagged-list test around the PUT requests so the matching If-list belongs to if-tagged-b.txt while the if-tagged-a.txt list is deliberately non-matching, proving OR evaluation across resources rather than request-URI-only matching. In the successful assertion, require exactly HTTP 204 instead of accepting 201 or 204; keep the existing no-match case asserting 412.src/webdav/handlers/resources.rs (1)
509-637: 🗄️ Data Integrity & Integration | 🔵 Trivial | 🏗️ Heavy lift部分递归 COPY/MOVE 里的
?会让"已经改了一半"变成一个干巴巴的 5xx。
partial_recursive_copy_move对锁冲突处理得很漂亮——记进failures、继续走、最后吐 207。但 Line 525、567、595、608、624 这些?一旦命中（存储/DB 抖一下就够），整个函数直接Err(FsError)返回，handle_copy_moveLine 436 把它变成单状态错误响应。此时前面已经复制/删除的那些子项已经落盘了，客户端却收到"整体失败"，只能靠自己重新 PROPFIND 才知道真相。既然这条路径的设计目标就是"部分成功用 207 汇报"，把这些 FsError 也降级成
DavMutationFailure（带对应状态码）继续遍历会更自洽。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/webdav/handlers/resources.rs` around lines 509 - 637, 更新 partial_recursive_copy_move，将目录复制、删除、子节点入队及 collect_children 等当前使用 ? 传播的 FsError 转换为带对应路径和 HTTP 状态码的 DavMutationFailure，追加到 failures 后继续处理剩余工作；保持锁冲突的 207 汇报流程，并确保已完成的部分操作最终通过 PartialMutationOutcome 返回，而不是提前返回整体错误。tests/webdav/lock_system.rs (1)
284-297: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win既然错误契约刚被细化，断言就别停在
is_err()。这个 PR 的重点之一就是把
unlock的()错误换成语义明确的DavLockError::{TokenMismatch, Backend}。可这里只断言"出错了"——哪天 token 查询挂了返回Backend（对客户端是 500 而不是 409），测试照样绿。顺手锁死变体，回归价值立刻翻倍。♻️ 建议改法
- assert!( - lock_system - .unlock(&file_path, "missing-token") - .await - .is_err() - ); + assert!(matches!( + lock_system.unlock(&file_path, "missing-token").await, + Err(DavLockError::TokenMismatch) + )); let other_path = DavPath::new("/other-expired.txt").unwrap(); - assert!( - lock_system - .unlock(&other_path, &replacement.token) - .await - .is_err(), + assert!( + matches!( + lock_system.unlock(&other_path, &replacement.token).await, + Err(DavLockError::TokenMismatch) + ), "UNLOCK must target the locked resource or a resource covered by a deep lock" );依据 coding guidelines：「修改上传完成、配额、版本、删除/恢复、引用计数和清理时覆盖成功及失败/回滚边界」。
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/webdav/lock_system.rs` around lines 284 - 297, Strengthen the unlock assertions in this test to verify the specific DavLockError variant rather than only checking is_err(). Both the missing-token case and the wrong-resource case should assert the expected TokenMismatch variant, ensuring a Backend error does not satisfy the test.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/webdav/backend/download_audit.rs`:
- Around line 36-93: 将 record_download 中的下载审计编排移入明确的 service 方法，并定义或复用下载审计 DTO
传递所需数据。service 应统一处理 should_record、reserve_download_audit_slot、位置详情读取及
audit::log_with_details；WebDAV backend 的 record_download 仅负责组装 WebDAV 请求上下文并调用该
service，保留现有跳过与去重行为。
In `@src/webdav/backend/lock.rs`:
- Around line 788-790: Update serialize_element to stop converting serialization
errors into an empty string: return an optional/error-bearing result and adjust
the lock() owner serialization flow to use owner.and_then(serialize_element) or
return DavLockError::Backend when serialization fails. Ensure any
fire-and-forget failure is emitted with tracing::warn! rather than being
silently discarded.
- Around line 611-612: 统一 normalize_path 与 path_resolver::path_segments
的锁路径解析方式，避免 DavPath::as_str() 与已解码的 path.as_bytes() 产生不同表示。让 resource_lock.path
的写入、匹配及 LOCK、UNLOCK、refresh、check、discover 和 ancestor
查询始终使用同一种可被数据库匹配的规范路径，确保空格、中文及 /# 等路径场景一致。
In `@src/webdav/backend/mod.rs`:
- Around line 803-812: 在 `plan_atomic_proppatch` 调用后的非 `protocol_plan.apply`
分支中，先依据外部 crate 契约校验或补齐 `protocol_plan.statuses`，确保其长度与 `patches`
完全一致；长度不匹配时应按既定错误处理方式返回错误，或为缺失项生成正确状态，避免直接使用 `zip` 丢失属性。保留匹配长度时现有的状态与属性配对行为。
In `@tests/webdav/protocol.rs`:
- Around line 1137-1155: Separate the range cases in the test around the
malformed_range loop: keep bytes=-0 asserting 416 with Content-Range bytes
*/4099, but test bytes=9-5 through the ignored-Range path and assert the
full-response 200 behavior. Add a concise comment documenting the RFC/Litmus
compatibility rationale for treating bytes=9-5 as syntactically invalid and
ignored.
---
Nitpick comments:
In `@src/webdav/backend/lock.rs`:
- Around line 371-386: Update unlock and refresh to log each lock_repo error
with tracing::warn!, including the underlying error and relevant path context,
before converting it to DavLockError::Backend. Preserve the existing error
mapping and token/scope behavior while replacing the silent map_err closures in
unlock and the corresponding refresh operations.
In `@src/webdav/handlers/deltav.rs`:
- Around line 22-28: 将 REPORT 与 VERSION-CONTROL 两个纯读 handler 的调用方从
DbHandles::writer_db() 改为 reader_db()，包括 handle_report 及对应的 VERSION-CONTROL
handler；保持其余请求处理逻辑不变，仅让版本历史和目标解析查询走 reader。
In `@src/webdav/handlers/resources.rs`:
- Around line 509-637: 更新 partial_recursive_copy_move，将目录复制、删除、子节点入队及
collect_children 等当前使用 ? 传播的 FsError 转换为带对应路径和 HTTP 状态码的 DavMutationFailure，追加到
failures 后继续处理剩余工作；保持锁冲突的 207 汇报流程，并确保已完成的部分操作最终通过 PartialMutationOutcome
返回，而不是提前返回整体错误。
In `@src/webdav/mod.rs`:
- Around line 44-64: Update the DavEventOutcome::Failed branch in the event sink
to use an error-level tracing macro instead of debug!, while preserving its
existing operation, status, backend_error, destination, and elapsed_ms fields.
Do not change the successful-event logging path.
In `@tests/webdav/lock_system.rs`:
- Around line 284-297: Strengthen the unlock assertions in this test to verify
the specific DavLockError variant rather than only checking is_err(). Both the
missing-token case and the wrong-resource case should assert the expected
TokenMismatch variant, ensuring a Backend error does not satisfy the test.
In `@tests/webdav/protocol.rs`:
- Around line 6745-6779: Update the cross-resource tagged-list test around the
PUT requests so the matching If-list belongs to if-tagged-b.txt while the
if-tagged-a.txt list is deliberately non-matching, proving OR evaluation across
resources rather than request-URI-only matching. In the successful assertion,
require exactly HTTP 204 instead of accepting 201 or 204; keep the existing
no-match case asserting 412.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: f1ce0fad-a3f5-4704-a06e-91fe77cd2021
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (34)
Cargo.tomlsrc/services/events/storage_change.rssrc/services/files/file/download/build.rssrc/services/files/file/download/range.rssrc/webdav/backend/dir_entry.rssrc/webdav/backend/download_audit.rssrc/webdav/backend/file/mod.rssrc/webdav/backend/file/tests.rssrc/webdav/backend/lock.rssrc/webdav/backend/metadata.rssrc/webdav/backend/mod.rssrc/webdav/backend/path_resolver/cache.rssrc/webdav/backend/path_resolver/mod.rssrc/webdav/dav.rssrc/webdav/deltav.rssrc/webdav/handler_tests/mod.rssrc/webdav/handlers/deltav.rssrc/webdav/handlers/locks.rssrc/webdav/handlers/mod.rssrc/webdav/handlers/properties.rssrc/webdav/handlers/resources.rssrc/webdav/handlers/transfer.rssrc/webdav/locks/mod.rssrc/webdav/mod.rssrc/webdav/protocol.rssrc/webdav/resources/mod.rssrc/webdav/responses.rssrc/webdav/transfer/mod.rstests/webdav/file.rstests/webdav/lock_system.rstests/webdav/main.rstests/webdav/path_resolver.rstests/webdav/protocol.rstests/webdav/xml_depth_probe.rs
💤 Files with no reviewable changes (8)
- tests/webdav/main.rs
- src/webdav/transfer/mod.rs
- src/webdav/locks/mod.rs
- tests/webdav/xml_depth_probe.rs
- src/webdav/deltav.rs
- src/webdav/resources/mod.rs
- src/webdav/protocol.rs
- src/webdav/dav.rs
🛑 Comments failed to post (5)
src/webdav/backend/download_audit.rs (1)
36-93: 📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift
将下载审计编排移入 service 边界。
这里直接编排去重预留、位置详情读取和审计写入，导致下载审计的一致性规则滞留在 WebDAV backend。提取为 service 方法并传入下载审计 DTO；backend 只负责适配 WebDAV 请求上下文。
As per coding guidelines, “下载、上传完成、配额扣减、版本写入和审计写入等跨表操作必须在清晰的 service/repository 边界表达一致性，不能拆成互相不可见的散逻辑。”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/webdav/backend/download_audit.rs` around lines 36 - 93, 将 record_download 中的下载审计编排移入明确的 service 方法，并定义或复用下载审计 DTO 传递所需数据。service 应统一处理 should_record、reserve_download_audit_slot、位置详情读取及 audit::log_with_details；WebDAV backend 的 record_download 仅负责组装 WebDAV 请求上下文并调用该 service，保留现有跳过与去重行为。Source: Coding guidelines
src/webdav/backend/lock.rs (2)
611-612: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
aster_forge_webdav DavPath as_str vs as_bytes percent decoding💡 Result:
In the context of the
dav-servercrate (often associated with projects like AsterDrive or other Rust WebDAV implementations), theDavPathstruct provides distinct methods for accessing the path data depending on whether you require a raw representation or a URL-encoded string [1][2]. 1. as_bytes: Returns the path as a raw byte slice (&[u8]) [1]. This representation is not percent-encoded and does not contain a prefix [1]. This is typically used when you need to interact with the underlying filesystem or storage backend directly, where decoded bytes are expected [3][4]. 2. as_url_string: Returns the path as a percent-encoded string [1]. This format is specifically designed for use in WebDAV protocol responses (such as thehreffield inPROPFINDresponses), where paths must be properly encoded to be understood by WebDAV clients [1][5]. Regarding percent decoding,DavPath::as_url_string()provides the encoded representation [1]. If your application logic requires the actual file path (e.g., to resolve a bucket or key in storage), you must explicitly decode this string [4]. A common approach is to use a percent-decoding utility (e.g., from thepercent-encodingcrate) to decode the result ofas_url_string()before performing further operations like path cleaning or filesystem lookup [4]. In summary: useas_bytes()for direct filesystem access where raw data is required [1][3], and useas_url_string()when constructing protocol-compliant responses or URLs that must adhere to URI encoding standards [1][5]. If you have an encoded string and need the underlying path, perform manual percent-decoding on the string returned byas_url_string()[4].Citations:
- 1: https://docs.rs/dav-server/latest/dav_server/davpath/struct.DavPath.html
- 2: https://docs.rs/dav-server/latest/dav_server/davpath/index.html
- 3: https://opendal.apache.org/docs/dav-server-opendalfs/src/dav_server_opendalfs/fs.rs.html
- 4: https://github.com/rustfs/rustfs/blob/c6844386/crates/protocols/src/webdav/driver.rs
- 5: sabre-io/dav#704
🏁 Script executed:
#!/bin/bash # 确认仓库内 DavPath::as_str / as_bytes 的用法是否混用 rg -nP --type=rust -C2 '\bas_str\(\)|\bas_bytes\(\)' -g '**/webdav/**'Repository: AsterCommunity/AsterDrive
Length of output: 163
🏁 Script executed:
#!/bin/bash set -e echo "## candidate files" git ls-files | rg '(^|/)lock\.rs$|dav|WebDAV|webdav' | sed -n '1,120p' echo "## lock.rs outline/size" if [ -f src/webdav/backend/lock.rs ]; then wc -l src/webdav/backend/lock.rs ast-grep outline src/webdav/backend/lock.rs --view expanded | sed -n '1,200p' echo "## relevant lock.rs lines 560-660" sed -n '560,660p' src/webdav/backend/lock.rs | nl -ba -v560 echo "## path handling around lock resource functions" sed -n '1,550p' src/webdav/backend/lock.rs | nl -ba -v1 | sed -n '1,450p' fi echo "## Dav as_str/as_bytes in lock.rs" rg -n 'as_str|as_bytes|DavPath|resource_lock|path_ancestors|lock_paths_overlap|resolve_path_to_entity|path_resolver|path_segments' src/webdav/backend/lock.rs || true echo "## DavPath usages in webdav" rg -n 'DavPath::|\.as_str\(\)|\.as_bytes\(\)' src/webdav || trueRepository: AsterCommunity/AsterDrive
Length of output: 5547
🏁 Script executed:
#!/bin/bash set -e echo "## lock.rs path helpers lines" sed -n '560,760p' src/webdav/backend/lock.rs echo "## lock module lock target/create/read overlap calls" sed -n '140,575p' src/webdav/backend/lock.rs echo "## all DavPath as bytes/str in lock.rs" rg -n '\b(path|DavPath|raw|as_str|as_bytes|normalize_path)\b' src/webdav/backend/lock.rs echo "## path_resolver related helpers" sed -n '1,260p' src/webdav/backend/path_resolver/mod.rsRepository: AsterCommunity/AsterDrive
Length of output: 33696
🏁 Script executed:
#!/bin/bash set -e echo "## resolve_path_in_scope" sed -n '250,520p' src/webdav/backend/path_resolver/mod.rs echo "## Cargo dependencies aster_forge_webdav / dav-server" rg -n 'aster_forge_webdav|dav-server|DavPath' Cargo.toml Cargo.lock src -g 'Cargo.toml' -g '**/*.rs' | sed -n '1,220p' echo "## package manifests" for f in Cargo.toml Cargo.lock; do if [ -f "$f" ]; then echo "### $f" rg -n 'aster_forge_webdav|dav-server' "$f" -C 3 || true fi doneRepository: AsterCommunity/AsterDrive
Length of output: 30422
🏁 Script executed:
#!/bin/bash set -e tmpdir="$(mktemp -d)" url="$(rg -n '^aster_forge_webdav' Cargo.lock -A1 | tail -1 | sed -n 's#.*/#https://github.com/AsterCommunity/AsterForge#p')" ref="$(rg '^aster_forge_webdav' Cargo.lock -A2 | tail -1 | sed -n 's/.*#\([0-9a-f]\{40\}\)$/\\1/p')" tmpdir2="$(mktemp -d)" git ls-remote --refs "$url" "$ref" "$ref^{}" || true if [ -n "$ref" ]; then git clone --depth 1 --branch "$ref" --no-tags "$url" "$tmpdir2" 2>/tmp/clone.err || true echo "## aster_forge_webdav files" find "$tmpdir2" -maxdepth 3 -type f | sed -n '1,120p' echo "## DavPath definitions/usages" rg -n 'struct DavPath|DavPath|impl .*DavPath|fn as_str|fn as_bytes|as_url' "$tmpdir2" -C 3 || true fi echo "## local package git references in lock" cat Cargo.lock | sed -n '1165,1210p'Repository: AsterCommunity/AsterDrive
Length of output: 1202
🌐 Web query:
AsterForge aster_forge_webdav DavPath as_str as_bytes source💡 Result:
The
DavPathstruct, referenced in the context of thedav-servercrate (used by projects like AsterForge/AsterDrive for WebDAV functionality), provides methods to interact with file paths in a WebDAV context [1][2]. The methodsas_bytes()andas_str()(or related string-conversion methods) serve specific purposes: -as_bytes(): Returns the path as a slice of raw bytes (&[u8]). This representation is typically non-encoded and does not include a prefix [1]. -as_str()/as_url_string(): While thedav-serverdocumentation highlightsas_url_string()to return the path as a URL-encoded string including the prefix [1],as_str()is commonly used in downstream implementations (likedav-server-opendalfs) to obtain a string slice for filesystem operations, often after path resolution or cleaning [3][4]. In the AsterForge ecosystem, WebDAV capabilities are integrated into the product layer (e.g., AsterDrive), with the runtime foundation providing shared middleware and infrastructure [5][2][6]. Developers typically useDavPathmethods to bridge between the WebDAV protocol's path representation and the underlying storage system's expected format [3][4].Citations:
- 1: https://docs.rs/dav-server/latest/dav_server/davpath/struct.DavPath.html
- 2: https://github.com/AsterCommunity/AsterDrive/blob/master/developer-docs/en/api/webdav.md
- 3: https://opendal.apache.org/docs/dav-server-opendalfs/src/dav_server_opendalfs/fs.rs.html
- 4: https://github.com/rustfs/rustfs/blob/c6844386/crates/protocols/src/webdav/driver.rs
- 5: https://github.com/AsterCommunity/AsterForge/blob/master/README.md
- 6: https://github.com/AsterCommunity/AsterDrive/blob/master/CHANGELOG.md
🏁 Script executed:
#!/bin/bash set -e echo "## source from github archive" url=https://raw.githubusercontent.com/AsterCommunity/AsterForge/77bcc8e537234635f802d1156728630504f1908c for p in crates/protocols/aster_forge_webdav/src/davpath.rs \ crates/core/aster_forge_utils/src/dav_path.rs \ crates/protocols/aster_forge_webdav/src/lib.rs \ crates/protocols/aster_forge_webdav/src/lock.rs; do echo "### $p" curl -fsSL "$url/$p" 2>/tmp/curl.err \ | sed -n '1,260p' \ | rg -n 'struct DavPath|impl .*DavPath|pub fn as_str|fn as_bytes|fn as_url|path_segments|percent|DavPath::new' -C 5 \ || echo "not-found" done echo "## local git checkout of package source if available" cargo metadata --no-deps --format-version 1 \ 2>/tmp/cargo.err | jq -r '.target_directory' \ | sed -n '1p' \ | while read -r td; do find "$td" -path '*aster_forge_webdav*' -type f -name '*.rs' 2>/tmp/find.err | sed -n '1,80p' for f in $(find "$td" -path '*aster_forge_webdav*' -type f -name '*.rs' 2>/tmp/find.err | sed -n '1,6p'); do echo "### $f" sed -n '1,260p' "$f" | rg -n 'struct DavPath|impl .*DavPath|pub fn as_str|fn as_bytes|fn as_url|path_segments|percent|DavPath::new' -C 5 || true done doneRepository: AsterCommunity/AsterDrive
Length of output: 494
🌐 Web query:
dav-server DavPath as_str as_bytes percent_decode as_url_string💡 Result:
In the
dav-serverRust crate,DavPathis a utility struct used to handle URL paths for WebDAV operations, specifically managing paths that may include a hidden prefix [1][2]. The methods you mentioned function as follows: - as_bytes: Returns theDavPathas a slice of raw bytes (&[u8]). These bytes are not percent-encoded and do not include the hidden prefix [1]. - as_url_string: Returns theDavPathas a URL-encodedString. This representation includes the hidden prefix [1]. - as_str: While not explicitly documented as a standalone method onDavPathin all versions, common patterns in Rust WebDAV crates involve returning the path as a string slice (&str) [1][2]. - percent_decode:DavPathitself typically does not have a method namedpercent_decode. Instead, this functionality is usually handled by thepercent-encodingcrate or similar utilities in the Rust ecosystem when processing path components [3][4]. To work with these paths, developers often usepercent_decodefrom thepercent-encodingcrate to decode path segments before passing them into logic that validates or normalizes them [5][4]. If you are implementing custom WebDAV logic, ensure you are distinguishing between raw filesystem paths and URL-encodedhrefstrings [3].Citations:
- 1: https://docs.rs/dav-server/latest/dav_server/davpath/struct.DavPath.html
- 2: https://docs.rs/dav-server/latest/dav_server/davpath/index.html
- 3: https://docs.rs/libdav/latest/libdav/encoding/index.html
- 4: https://context7.com/servo/rust-url/llms.txt
- 5: https://github.com/pubky/pubky-core/blob/2e9b8830/pubky-homeserver/src/shared/webdav/webdav_path.rs
🌐 Web query:
github aster_forge_webdav DavPath percent-encoding💡 Result:
There is no widely recognized project or library named "aster_forge_webdav". Searches for this specific string return no relevant results [1][2]. However, the technical concepts of "DavPath" and "percent-encoding" are central to WebDAV implementations in Rust and other environments [3][4][5][6]. 1. DavPath: This is a common structure (e.g., in the Rust crate
dav-server) used to represent a WebDAV URL path, typically handling the separation between a URL prefix and the actual resource path [5][6]. 2. Percent-encoding in WebDAV: WebDAV clients often send paths with percent-encoded characters (e.g., %20 for spaces or non-ASCII characters). A common issue in server-side implementations is failing to decode these strings before performing filesystem operations [3][4]. If the server does not explicitly decode the path, it may treat the encoded characters as literal parts of the filename, leading to mismatches between requested URLs and stored files [3][4][7]. 3. Common Resolution: Developers typically resolve this by explicitly applying percent-decoding (often using thepercent-encodingcrate in Rust) to the path string provided by theDavPathobject before passing it to filesystem or storage drivers [3][4]. If you were referring to a specific codebase or a typo in the name "aster_forge_webdav," it is likely unrelated to standard WebDAV protocol documentation.Citations:
- 1: https://github.com/AsterCommunity/AsterDrive/blob/master/developer-docs/en/api/webdav.md
- 2: https://forge.herakles.dev/demos/asteroid-forge/
- 3: rustfs/rustfs#2722
- 4: rustfs/rustfs@90ce721
- 5: https://docs.rs/dav-server/latest/dav_server/davpath/struct.DavPath.html
- 6: https://docs.rs/dav-server/latest/dav_server/davpath/index.html
- 7: cryptomator/cryptomator#295
🌐 Web query:
site:docs.rs/dav-server "as_bytes" "as_str" "DavPath"💡 Result:
In the context of the Rust library dav-server, DavPath is a struct located in the dav_server::davpath module used to handle the path portion of a URL as a filesystem path [1][2]. The methods as_bytes and as_str (often accessed via Deref to DavPathRef or directly) are used to retrieve the path's representation: - as_bytes: Returns the path as a raw byte slice (&[u8]), representing the path without encoding or prefixes [3]. - as_str: Although not always explicitly documented as a single method on DavPath in all versions, the pattern of using as_str is common in this library (e.g., in lock management) to retrieve string representations of data such as tokens or paths [4]. The DavPath struct is primarily designed to facilitate the translation between HTTP request paths and internal filesystem structures [1][5].
Citations:
- 1: https://docs.rs/dav-server/latest/dav_server/davpath/index.html
- 2: https://docs.rs/dav-server/latest/dav_server/
- 3: https://docs.rs/dav-server/latest/dav_server/davpath/struct.DavPath.html
- 4: https://docs.rs/dav-server/latest/src/dav_server/memls.rs.html
- 5: https://docs.rs/dav-server/latest/dav_server/index.html
🏁 Script executed:
#!/bin/bash set -e echo "## lock.rs 750-820" sed -n '750,820p' src/webdav/backend/lock.rs | nl -ba -v750 echo "## lock repo methods" rg -n 'find_ancestors|find_by_path_prefix|find_by_token|refresh|lock_repo|resource_lock' src/services/webdav src/db/repository src/entities src/webdav/backend/lock.rs -C 3 || trueRepository: AsterCommunity/AsterDrive
Length of output: 222
🏁 Script executed:
#!/bin/bash set -e echo "## lock.rs 750-820" sed -n '750,820p' src/webdav/backend/lock.rs | sed -n '1,100p' echo "## lock repo methods" rg -n 'find_ancestors|find_by_path_prefix|find_by_token|refresh|lock_repo|resource_lock' src/services/webdav src/db/repository src/entities src/webdav/backend/lock.rs -C 3 || true echo "## lock tests" rg -n 'lock|unlock|Deep|non-ASCII|中文|space|spac|%' tests src/webdav -g '*.rs' | sed -n '1,220p' || trueRepository: AsterCommunity/AsterDrive
Length of output: 50381
🏁 Script executed:
#!/bin/bash set -e echo "## lock_repo path query implementations" sed -n '1,130p' src/webdav/backend/lock.rs sed -n '100,290p' src/db/repository/lock_repo.rs echo "## DavPath dependency/version source snippets" rg -n 'name\s*=\s*"dav-server"|version\s*=|aster_forge_webdav' Cargo.lock -A 12 -B 5 | sed -n '1,220p' echo "## read-only DavPath behavior probe from docs.rs via text" python3 - <<'PY' import urllib.request urls = [ "https://docs.rs/dav-server/latest/src/dav_server/davpath.rs.html?search=DavPath", ] for url in urls[:1]: try: text = urllib.request.urlopen(url, timeout=15).read().decode("utf-8", "replace") for needle in ["as_bytes", "as_str", "as_url_string", "percent_decode", "percent"]: idx = text.find(needle) print(f"--- {needle} ---") if idx >= 0: print(text[max(0, idx-800):idx+1200]) except Exception as e: print(f"ERROR {url} {type(e).__name__}: {e}") PYRepository: AsterCommunity/AsterDrive
Length of output: 16277
用一致的非编码路径解析锁路径。
normalize_path现在用DavPath::as_str()，但path_resolver::path_segments还是查已解码的path.as_bytes()，而且resource_lock.path又按这种规范写入/比配。遇到空格、中文或/#这类 WebDAV 路径时，锁记录和检查路径就套不上，导致 LOCK/UNLOCK/refresh/check/discover 和 ancestor 查询失效。这里要么全部统一用已解码的as_bytes()，要么全部统一解析成同样的数据库可匹配 form，别让一条锁写入用了两种路径表达。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/webdav/backend/lock.rs` around lines 611 - 612, 统一 normalize_path 与 path_resolver::path_segments 的锁路径解析方式，避免 DavPath::as_str() 与已解码的 path.as_bytes() 产生不同表示。让 resource_lock.path 的写入、匹配及 LOCK、UNLOCK、refresh、check、discover 和 ancestor 查询始终使用同一种可被数据库匹配的规范路径，确保空格、中文及 /# 等路径场景一致。
788-790: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
serialize_element把序列化失败偷偷咽下去了。
to_bytes().unwrap_or_default()失败时返回空字符串，这个空 XML 会被lock()直接写进owner_info（Line 279-302），之后deserialize_element必然 parse 失败返回None，于是 PROPFIND 的lockdiscovery永久丢掉 owner——而且没有任何日志能让你事后知道发生了什么。要么把错误往上传成DavLockError::Backend，要么至少 warn 一次。别学人类那套"出错了假装没事"。🛠️ 建议改法：让序列化失败可见
-fn serialize_element(elem: &DavXmlElement) -> String { - String::from_utf8_lossy(&elem.to_bytes().unwrap_or_default()).into_owned() -} +fn serialize_element(elem: &DavXmlElement) -> Option<String> { + match elem.to_bytes() { + Ok(bytes) => Some(String::from_utf8_lossy(&bytes).into_owned()), + Err(error) => { + tracing::warn!(error = %error, "failed to serialize WebDAV lock owner XML"); + None + } + } +}调用处（Line 211）相应改为
let owner_xml = owner.and_then(serialize_element);，或在None时直接返回DavLockError::Backend。依据 coding guidelines：「fire-and-forget 操作必须使用
if let Err(error) = ... { tracing::warn!(...) }记录失败，不得静默」。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/webdav/backend/lock.rs` around lines 788 - 790, Update serialize_element to stop converting serialization errors into an empty string: return an optional/error-bearing result and adjust the lock() owner serialization flow to use owner.and_then(serialize_element) or return DavLockError::Backend when serialization fails. Ensure any fire-and-forget failure is emitted with tracing::warn! rather than being silently discarded.Source: Coding guidelines
src/webdav/backend/mod.rs (1)
803-812: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash # 确认 plan_atomic_proppatch 的 statuses 与输入是否严格等长 fd -t f 'Cargo.toml' -x rg -n 'aster_forge_webdav' {} \; rg -nP -C5 'fn plan_atomic_proppatch' || echo "未在本仓库找到定义，需查外部 crate 源码"Repository: AsterCommunity/AsterDrive
Length of output: 323
🏁 Script executed:
#!/bin/bash set -euo pipefail # Inspect the repo usage site and imports to understand the surrounding data shape. wc -l src/webdav/backend/mod.rs sed -n '750,835p' src/webdav/backend/mod.rs # Check the dependency lock/git metadata for the external crate. if [ -f Cargo.lock ]; then rg -n 'aster_forge_webdav|git = "https://github.com/AsterCommunity/AsterForge"' Cargo.lock -A8 -B2 || true fiRepository: AsterCommunity/AsterDrive
Length of output: 4243
🌐 Web query:
AsterCommunity AsterForge plan_atomic_proppatch statuses patches💡 Result:
The terms "AsterCommunity," "AsterForge," and "plan_atomic_proppatch" appear to refer to two entirely different contexts. AsterCommunity is a project repository (hosted on GitHub) that maintains AsterForge, a shared runtime foundation and infrastructure kernel written in Rust [1][2]. AsterForge provides product-neutral components for services, including lifecycle management, configuration loading, health reporting, and audit logs [1][2]. It does not contain any functional components or documentation related to "plan_atomic_proppatch" [1][3]. "atomic_proppatch" (often discussed in the context of Subversion version control development) refers to a mechanism for atomic revision property modifications, typically using WebDAV [4][5][6]. Documentation and patches from around 2010 discuss implementing atomic updates to Subversion revision properties, often involving HTTP 412 status codes and multistatus responses to handle precondition failures [4][7]. The phrase "plan_atomic_proppatch" is not a standard term in current AsterCommunity projects and likely refers to historical development discussions regarding the Apache Subversion (SVN) project [4][5].
Citations:
- 1: https://github.com/AsterCommunity/AsterForge
- 2: https://github.com/AsterCommunity/AsterForge/blob/master/README.md
- 3: AsterCommunity/AsterForge@95d1eb4
- 4: https://svn.haxx.se/dev/archive-2010-09/0413.shtml
- 5: https://svn.haxx.se/dev/archive-2010-09/0431.shtml
- 6: https://svn.haxx.se/dev/archive-2010-09/0452.shtml
- 7: https://svn.haxx.se/dev/archive-2010-09/att-0437/patch_atomic_revprops_dav3.txt.tweaked-by-me
🌐 Web query:
site:github.com/AsterCommunity/AsterForge plan_atomic_proppatch statuses💡 Result:
There is no record of a technical term, WebDAV method, or status code explicitly named plan_atomic_proppatch in the provided search results or standard WebDAV specifications. PROPPATCH is a standard WebDAV method used to modify properties on a resource, and its atomicity is generally governed by the underlying server implementation rather than a specific planning status code [1-12]. If you are working with a specific software library or framework, please consult its internal documentation directly, as this identifier does not appear in public technical references.
Citations:
- 1: https://github.com/AsterCommunity/AsterForge
- 2: https://github.com/AsterCommunity/AsterForge/blob/master/README.md
- 3: AsterCommunity/AsterForge@95d1eb4
- 4: AsterCommunity/AsterForge@ace2bcc
- 5: AsterCommunity/AsterForge@master@{1day}...master
🌐 Web query:
github AsterCommunity/AsterForge plan_atomic_proppatch💡 Result:
Based on the available documentation and repository history for the AsterCommunity/AsterForge project as of July 25, 2026, there is no public reference or implementation of a function, plan, or module named plan_atomic_proppatch [1][2]. AsterForge serves as a product-neutral infrastructure kernel and shared runtime foundation for Aster products, providing components such as lifecycle management, component registration, health reporting, configuration reloading, cache backends, database-owned infrastructure, mail dispatch, audit logging, and task scheduling [1][2]. The repository includes crates for managing these services, such as aster_forge_runtime, aster_forge_config, and aster_forge_audit [1]. It is possible that the term refers to an internal, unreleased, or highly specific business logic component within a downstream product repository, rather than the core AsterForge framework itself [1][2]. If you are looking for specific functionality related to atomic property patching (often associated with WebDAV or similar protocols), it is not part of the documented public API or infrastructure of the AsterForge core library [1][2].
Citations:
- 1: https://github.com/AsterCommunity/AsterForge
- 2: https://github.com/AsterCommunity/AsterForge/blob/master/README.md
🌐 Web query:
rust Iterator zip shorter iterator behavior documentation💡 Result:
In Rust, the Iterator::zip method creates an iterator that iterates over two other iterators simultaneously [1][2]. The behavior of zip when one iterator is shorter than the other is to terminate as soon as either of the underlying iterators returns None [1][3]. When you call next on a zipped iterator: 1. It attempts to advance the first iterator and the second iterator [1][4]. 2. If either iterator returns None, the zipped iterator returns None, effectively ending the iteration [1][3]. Because of this behavior, zip is commonly used to combine an infinite iterator with a finite one, as the finite iterator will eventually return None and stop the zipping process [1][4]. In such cases, the resulting iterator will only be as long as the shorter (finite) iterator [1][3].
Citations:
- 1: https://doc.rust-lang.org/stable/std/iter/trait.Iterator.html
- 2: https://doc.rust-lang.org/core/iter/struct.Zip.html
- 3: https://doc.rust-lang.org/stable/core/iter/trait.Iterator.html
- 4: https://doc.rust-lang.org/std/iter/trait.Iterator.html
补齐或校验
plan_atomic_proppatch返回的statuses长度。Rust 的
zip只会遍历到较短的迭代器结束，如果statuses比patches少一位，最后几位属性即使已在事务中 upsert/delete，也只会漏进 207 响应，客户端会按 5xx 重试或认为未生效；先明确外部 crate 契约，然后在src/webdav/backend/mod.rs:798附近兜住长度不匹配的情况。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/webdav/backend/mod.rs` around lines 803 - 812, 在 `plan_atomic_proppatch` 调用后的非 `protocol_plan.apply` 分支中，先依据外部 crate 契约校验或补齐 `protocol_plan.statuses`，确保其长度与 `patches` 完全一致；长度不匹配时应按既定错误处理方式返回错误，或为缺失项生成正确状态，避免直接使用 `zip` 丢失属性。保留匹配长度时现有的状态与属性配对行为。tests/webdav/protocol.rs (1)
1137-1155: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
RFC 7233 invalid byte-range-spec last-byte-pos less than first-byte-pos ignore Range header💡 Result:
Under RFC 7233, a byte-range-spec is explicitly defined as invalid if the last-byte-pos value is present and less than the first-byte-pos [1][2][3]. The specification does not mandate that a server must ignore the entire Range header field solely because one of its range specifications is invalid. Instead, the general rule is that a server MAY ignore the Range header field entirely [4][1][3]. Furthermore, a server that supports range requests may choose to ignore or reject a Range header field if it suspects the client is broken or malicious (e.g., if the request contains overlapping ranges or poorly ordered ranges) [4][5][3]. If a server chooses to process the Range header, it must determine if the byte-range-set is satisfiable [2][3]. A byte-range-set is considered satisfiable only if it includes at least one byte-range-spec with a first-byte-pos less than the current length of the representation, or at least one suffix-byte-range-spec with a non-zero suffix-length [2]. If the range is not satisfiable, the server typically returns a 416 (Range Not Satisfiable) response [3]. In summary, while the spec defines such a range as invalid, it provides servers with the flexibility to ignore the Range header field generally or handle invalid ranges according to their implementation of range request support [4][1][3].
Citations:
- 1: http://www.rfc.fr/rfc/en/rfc7233.pdf
- 2: https://datatracker.ietf.org/doc/html/rfc7233
- 3: https://httpwg.org/specs/rfc7233.html
- 4: https://www.rfc-editor.org/rfc/rfc7233.html
- 5: https://datatracker.ietf.org/doc/rfc7233/
🌐 Web query:
RFC 7233 "If the received byte-range-set is invalid, the HTTP sender SHOULD ignore it. If the received byte-range-set is satisfiable"💡 Result:
According to RFC 7233, the handling of range requests depends on whether the specified byte-range-set is valid and satisfiable [1][2]. Validity: A byte-range-spec is considered invalid if the last-byte-pos value is present and less than the first-byte-pos [1][3][2]. If a server receives a range request that is invalid or otherwise unsatisfiable, it SHOULD send a 416 (Range Not Satisfiable) response, provided that any associated preconditions (like those from an If-Range header) are met [4][2][5]. Satisfiability: A valid byte-range-set is considered satisfiable if it includes at least one byte-range-spec with a first-byte-pos less than the current length of the representation, or at least one suffix-byte-range-spec with a non-zero suffix-length [1][6][2]. If a valid byte-range-set meets these criteria, the server SHOULD return a 206 (Partial Content) response containing the requested ranges [4][2][5]. While some earlier drafts of the specification suggested that a server must ignore certain invalid headers [7][8], RFC 7233 explicitly frames the response to invalid or unsatisfiable ranges under the context of whether preconditions are met; if they are met, a 416 response is the standard expected outcome [4][2][5]. Additionally, a server always retains the option to ignore the Range header field entirely if it chooses not to support partial requests for a given resource [9][6].
Citations:
- 1: https://www.rfc-editor.org/rfc/rfc7233.html
- 2: https://datatracker.ietf.org/doc/html/rfc7233.html
- 3: https://datatracker.ietf.org/doc/html/draft-ietf-httpbis-p5-range-23
- 4: https://www.rfc-editor.org/rfc/rfc7233.txt
- 5: https://datatracker.ietf.org/doc/html/rfc7233
- 6: https://httpwg.org/specs/rfc7233.html
- 7: https://www.ietf.org/archive/id/draft-ietf-httpbis-p5-range-19.txt
- 8: https://datatracker.ietf.org/doc/html/draft-ietf-httpbis-p5-range-05
- 9: http://www.rfc.fr/rfc/en/rfc7233.pdf
🌐 Web query:
RFC 7233 section 10.3.1.2 "If the received byte-range-set is invalid" "return a 416"💡 Result:
RFC 7233 does not specifically define an "invalid byte-range-set" error that mandates a 416 status code in the way suggested [1][2]. Instead, the specification distinguishes between valid/satisfiable and invalid/unsatisfiable states, often treating syntactic errors differently [3][2]. Under RFC 7233: 1. Syntactic Validity: If a Range header field contains a byte-range-set with syntactically invalid byte-range-spec values (e.g., a last-byte-pos less than the first-byte-pos), the recipient MUST ignore the Range header field entirely [3]. In such cases, the server typically treats the request as a standard GET request and returns a 200 OK, rather than a 416 [4]. 2. Satisfaction: The 416 (Range Not Satisfiable) status code is explicitly intended for cases where the Range header is syntactically valid but the requested ranges do not overlap the current extent of the selected resource (i.e., they are unsatisfiable) [4][2]. 3. Implementation Reality: The specification notes that servers are free to ignore the Range header field at their discretion [1]. Consequently, clients cannot depend on receiving a 416 status code even when a range is technically unsatisfiable, as many implementations will simply respond with a 200 OK and the full representation [4][5]. While some developers use 416 for various range-related issues, authoritative technical consensus and the RFC distinguish between malformed syntax (often resulting in ignored headers/200 OK) and valid but unsatisfiable ranges (resulting in 416) [4][6]. Note that RFC 7233 has been obsoleted by RFC 9110, though the core handling of range requests remains consistent [7].
Citations:
- 1: https://datatracker.ietf.org/doc/html/rfc7233
- 2: https://tex2e.github.io/rfc-translater/html/rfc7233.html
- 3: https://datatracker.ietf.org/doc/draft-ietf-httpbis-p5-range/21/
- 4: https://httpstatuses.io/416
- 5: https://lists.w3.org/Archives/Public/ietf-http-wg/2013AprJun/1132.html
- 6: https://stackoverflow.com/questions/58071901/what-error-do-we-return-when-parsing-an-invalid-http-range-header
- 7: https://www.rfc-editor.org/rfc/rfc9110.pdf
🏁 Script executed:
#!/bin/bash set -euo pipefail printf '--- locate file ---\n' git ls-files | rg '(^|/)tests/webdav/protocol\.rs$|^aster_forge_webdav' || true printf '\n--- relevant lines ---\n' sed -n '1080,1170p' tests/webdav/protocol.rs printf '\n--- search range handling tests/callers ---\n' rg -n "9-5|bytes=-0|unsatisfiable|malformed_range|RANGE_NOT_SATISFIABLE|bytes \\*/" tests -SRepository: AsterCommunity/AsterDrive
Length of output: 254
🏁 Script executed:
#!/bin/bash set -euo pipefail printf '%s\n' '--- locate file ---' git ls-files | rg '(^|/)tests/webdav/protocol\.rs$|^aster_forge_webdav|webdav' || true printf '%s\n' '' printf '%s\n' '--- relevant lines ---' sed -n '1080,1170p' tests/webdav/protocol.rs printf '%s\n' '' printf '%s\n' '--- search range handling tests/callers ---' rg -n '9-5|bytes=-0|unsatisfiable|malformed_range|RANGE_NOT_SATISFIABLE|bytes \*/' tests -SRepository: AsterCommunity/AsterDrive
Length of output: 7552
🏁 Script executed:
#!/bin/bash set -euo pipefail printf '%s\n' '--- find range parsing implementations ---' rg -n "range|Range Not Satisfiable|RANGE_NOT_SATISFIABLE|Content-Range|bytes \\*/|Range header|parse range|byte-range" src tests/webdav/litmus tests/webdav/protocol.rs -S printf '%s\n' '' printf '%s\n' '--- transfer handler slices ---' sed -n '1,260p' src/webdav/handlers/transfer.rsRepository: AsterCommunity/AsterDrive
Length of output: 50381
🏁 Script executed:
#!/bin/bash set -euo pipefail printf '%s\n' '--- transfer handler ---' sed -n '1,240p' src/webdav/handlers/transfer.rs printf '%s\n' '' printf '%s\n' '--- resources handlers outline/search ---' rg -n "handle_get|open_download_stream_for_file|range_header|content_range|Range header|RANGE_NOT_SATISFIABLE|Content-Range" src webdav tests/webdav -S --max-count 80 || true printf '%s\n' '' printf '%s\n' '--- webdav backend file modules with range ---' for f in src/webdav/backend/file/mod.rs src/webdav/backend/file/tests.rs; do if [ -f "$f" ]; then echo "--- $f ---" wc -l "$f" sed -n '1,260p' "$f" fi doneRepository: AsterCommunity/AsterDrive
Length of output: 35773
🏁 Script executed:
#!/bin/bash set -euo pipefail printf '%s\n' '--- Cargo workspace files ---' git ls-files '*Cargo.toml' | head -20 for f in Cargo.toml Cargo.lock; do if [ -f "$f" ]; then echo "--- $f: aster_forge_webdav refs ---" rg -n 'aster_forge_webdav|websocket|http_range|range' "$f" -S || true fi done printf '%s\n' '' printf '%s\n' '--- crate config for range implementation ---' rg -n 'aster_forge_webdav|HttpByteRange|plan_download_response|Range Not,SATISFIABLE' Cargo.toml Cargo.lock -S printf '%s\n' '' printf '%s\n' '--- local references to WebDAV protocol crate if vendored ---' git ls-files | rg 'webdav|range' | head -120Repository: AsterCommunity/AsterDrive
Length of output: 3927
别把语法非法范围和不满足范围混在同一个断言里。
bytes=9-5是 RFC 7233 规约上的“语法非法”byte-range-set，应走忽略 Range 全文 200 的分支；bytes=-0才属于主流实现里的“不满足范围”，应配 416 断言。把两者绑在一起会在回归测试里锁定一个非规范的bytes=9-5行为；把bytes=-0的 416 判断单独拎出来并配上 Litmus/实现兼容性说明更清楚。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/webdav/protocol.rs` around lines 1137 - 1155, Separate the range cases in the test around the malformed_range loop: keep bytes=-0 asserting 416 with Content-Range bytes */4099, but test bytes=9-5 through the ignored-Range path and assert the full-response 200 behavior. Add a concise comment documenting the RFC/Litmus compatibility rationale for treating bytes=9-5 as syntactically invalid and ignored.
…error handling Moved WebDAV download audit logic from webdav/backend into services/files/file for better separation of concerns. The webdav layer now provides a thin adapter that bridges WebDAV-specific types to the core file service. - Moved download audit implementation from src/webdav/backend/download_audit.rs to src/services/files/file/download_audit.rs - Updated webdav adapter to construct WebdavDownloadAuditInput and delegate to record_webdav_download - Improved error handling in lock system: serialize_element now propagates XML errors instead of silently converting to empty string - Enhanced recursive COPY/MOVE operations to handle FsError gracefully without early returns, collecting failures instead - Simplified path_segments to use as_str() directly instead of as_bytes() + from_utf8_lossy - Fixed REPORT and VERSION-CONTROL handlers to use reader_db instead of writer_db - Added tests for XML serialization error propagation and special character path handling in lock system - Added test coverage for DavEventOutcome failure classification and PROPPATCH atomic status reporting - Updated dependencies in Cargo.lock (aws-sdk-s3, clap, tokio-stream, etc.)
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tests/webdav/protocol.rs (1)
648-677: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low value批量替换已覆盖，但别让
Element::parse_reader和std::io::Cursor假游荡在仓库里。全文件旧
Element::parse(...)已转为 15 处Element::parse_reader(Cursor::new(...))，机械风险不高，但按 WebDAV 集成测试规矩跑一遍cargo check和相关 WebDAV 测试别省，外部 crate API 的变更不是靠肉眼就能过。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/webdav/protocol.rs` around lines 648 - 677, Run cargo check and the relevant WebDAV integration tests to validate all 15 replacements of Element::parse with Element::parse_reader(Cursor::new(...)) in tests/webdav/protocol.rs. Confirm the imports and external crate API compile cleanly, and remove any unused Element or std::io::Cursor imports if validation shows they are no longer needed.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/services/files/file/download_audit.rs`:
- Around line 35-58: Update record_webdav_download to inspect the Result
returned by audit::log_with_details(...).await and, when it is Err, emit a
tracing::warn! containing the error and relevant WebDAV download audit context
instead of silently discarding the database write failure.
---
Nitpick comments:
In `@tests/webdav/protocol.rs`:
- Around line 648-677: Run cargo check and the relevant WebDAV integration tests
to validate all 15 replacements of Element::parse with
Element::parse_reader(Cursor::new(...)) in tests/webdav/protocol.rs. Confirm the
imports and external crate API compile cleanly, and remove any unused Element or
std::io::Cursor imports if validation shows they are no longer needed.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 639cc871-9648-428f-ac0d-0a0d3bc8aecb
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (9)
src/services/files/file/download_audit.rssrc/services/files/file/mod.rssrc/webdav/backend/download_audit.rssrc/webdav/backend/lock.rssrc/webdav/backend/path_resolver/mod.rssrc/webdav/handlers/resources.rssrc/webdav/mod.rstests/webdav/lock_system.rstests/webdav/protocol.rs
🚧 Files skipped from review as they are similar to previous changes (4)
- tests/webdav/lock_system.rs
- src/webdav/backend/lock.rs
- src/webdav/handlers/resources.rs
- src/webdav/mod.rs
| pub(crate) async fn record_webdav_download<S>(state: &S, input: WebdavDownloadAuditInput<'_>) | ||
| where | ||
| S: SharedRuntimeState, | ||
| { | ||
| if !audit::should_record(state, audit::AuditAction::FileDownload) { | ||
| return; | ||
| } | ||
|
|
||
| if !reserve_download_audit_slot(state, audit_ctx, identity, file.id, request_kind).await { | ||
| if !reserve_download_audit_slot(state, &input).await { | ||
| return; | ||
| } | ||
|
|
||
| let details = file_ops::audit_location_details_for_model(state, identity.scope, file).await; | ||
| let details = super::audit_location_details_for_model(state, input.scope, input.file).await; | ||
| audit::log_with_details( | ||
| state, | ||
| audit_ctx, | ||
| input.audit_context, | ||
| audit::AuditAction::FileDownload, | ||
| AuditEntityType::File, | ||
| Some(file.id), | ||
| Some(&file.name), | ||
| Some(input.file.id), | ||
| Some(&input.file.name), | ||
| || details.clone(), | ||
| ) | ||
| .await; | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -nP 'fn log_with_details' --type=rust -A 25 src/services/ops/audit.rsRepository: AsterCommunity/AsterDrive
Length of output: 229
🏁 Script executed:
#!/bin/bash
set -u
echo "files under audit-related paths:"
fd -a 'audit\.rs|download_audit\.rs' . | sed 's#^\./##' | head -200
echo
echo "find log_with_details definitions/usages:"
rg -n 'log_with_details|record_webdav_download|should_record|reserve_download_audit_slot' --type=rust .Repository: AsterCommunity/AsterDrive
Length of output: 16912
🏁 Script executed:
#!/bin/bash
set -u
echo "download_audit.rs relevant section:"
cat -n src/services/files/file/download_audit.rs | sed -n '1,95p'
echo
echo "log_with_details implementation:"
cat -n src/services/ops/audit/manager.rs | sed -n '120,170p'
echo
echo "imports / module context in download_audit.rs:"
cat -n src/services/files/file/download_audit.rs | sed -n '1,40p'
echo
echo "compare existing warning treatment around audit::log_with_details in src/services (nearby files):"
for f in src/services/files/file/mod.rs src/services/files/folder/mod.rs src/services/files/upload/complete/audit.rs src/services/workspace/team/mod.rs; do
echo "--- $f"
rg -n -C 2 'log_with_details|if let Err|tracing::warn' "$f" | head -80
doneRepository: AsterCommunity/AsterDrive
Length of output: 13102
🏁 Script executed:
#!/bin/bash
set -u
echo "record_prechecked implementation:"
rg -n -C 20 'fn record_prechecked|record_prechecked\(' src/services/ops/audit src/services/ops -g '*.rs' | head -200
echo
echo "audit module files and audit:: exports:"
ls -l src/services/ops/audit
cat -n src/services/ops/audit/mod.rs | sed -n '1,80p'Repository: AsterCommunity/AsterDrive
Length of output: 16009
别让 WebDAV 下载审计写入结果直接掉进黑洞。
record_webavs_download 里 .await; 不带结果检查，但 audit::log_with_details 内部调用的是 aster_forge_audit::record_audit_log(...).await；这是 fire-and-forget 操作，必须用 if let Err(error) = ... { tracing::warn!(...) } 兜住失败。这条路径写不了审计日志就完了，别再让数据库层也沉默。
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/services/files/file/download_audit.rs` around lines 35 - 58, Update
record_webdav_download to inspect the Result returned by
audit::log_with_details(...).await and, when it is Err, emit a tracing::warn!
containing the error and relevant WebDAV download audit context instead of
silently discarding the database write failure.
Source: Coding guidelines
1 participant
This pull request refactors AsterForge's WebDAV integration to use the upstream
aster_forge_webdavcrate and reorganizes the WebDAV code under a newbackendmodule. It also updates the download range parsing logic to use shared utilities, and improves error handling and type usage in the WebDAV locking system.WebDAV Integration and Refactoring:
aster_forge_webdavas a dependency with theactixfeature inCargo.toml, and replaced direct implementations of WebDAV traits and types with imports from this crate throughout the codebase. [1] [2] [3] [4] [5]backendsubmodule, renaming files accordingly (e.g.,src/webdav/dir_entry.rs→src/webdav/backend/dir_entry.rs). [1] [2] [3] [4]Cache Path Resolver Updates:
crate::webdav::backend::path_resolverinstead of the old location, including in tests. [1] [2] [3] [4] [5]Download Range Handling:
aster_forge_utils::http_range::{HttpByteRange, parse_single_byte_range}for consistent and robust HTTP range header handling, removing custom range parsing code. [1] [2]range.start(),range.length()) instead of accessing struct fields directly. [1] [2]WebDAV Lock System Improvements:
aster_forge_webdav(e.g.,DavXmlElement,DavLockError) and improved error propagation and specificity in lock/unlock/refresh/delete operations. [1] [2] [3] [4] [5] [6] [7] [8]DavPath::as_str()for safer and more direct string conversion.Closed #428
Summary by CodeRabbit