Skip to content

feat: real WebDAV LOCK/UNLOCK with in-memory lock enforcement#704

Open
image72 wants to merge 3 commits into
sigoden:mainfrom
image72:feat/real-webdav-locking
Open

feat: real WebDAV LOCK/UNLOCK with in-memory lock enforcement#704
image72 wants to merge 3 commits into
sigoden:mainfrom
image72:feat/real-webdav-locking

Conversation

@image72

@image72 image72 commented May 13, 2026

Copy link
Copy Markdown

Summary

  • Replace fake LOCK/UNLOCK with real lock state tracking via LockManager
  • 23 new tests in tests/lock.rs covering all lock workflows
  • litmus compliance: 24/26 lock tests pass (was 16/26 with fake locks)

Changes

Real locking infrastructure

  • LockManager: thread-safe lock registry (RwLock<HashMap<String, Vec<LockInfo>>>)
  • LockInfo: stores token, scope (exclusive/shared), timeout, created_at
  • Supports exclusive AND shared locks with conflict detection
  • LockManager::refresh() for lock renewal via If: header

LOCK improvements

  • Parse lockinfo XML body for exclusive vs shared scope
  • Timeout header support: parse Second-N and Infinite, respond with granted timeout
  • Default timeout: 300 seconds (5 minutes)
  • cleanup_expired() called on acquire (lazy GC)
  • is_expired() filters expired entries in is_locked (read path)
  • LOCK on unmapped URLs and collections now allowed (RFC 4918 §7.4)

Lock enforcement on write operations

  • PUT, PATCH, DELETE, MOVE, MKCOL, PROPPATCH: rejected with 423 Locked if locked without token
  • COPY: destination lock enforcement
  • If: header lock token extraction for owner bypass
  • 412 Precondition Failed when If: token on unlocked resource

Lock lifecycle management

  • DELETE: remove_all() cleans locks on deleted resource
  • MOVE: remove_all() cleans locks on source after rename
  • UNLOCK: validates Lock-Token header, returns 409 on mismatch

Allow/DAV headers

  • Allow now includes PROPPATCH, MKCOL, LOCK, UNLOCK
  • DAV: 1, 2, 3 unchanged (already declared)

litmus test results

locks: 24/26 passed (92%)
✅ lock_excl, discover, refresh, unlock, copy
✅ lock_shared, double_sharedlock  
✅ notowner_lock, notowner_modify (PROPPATCH, DELETE)
✅ owner_modify, cond_put_corrupt_token
✅ fail_cond_put_unlocked, cond_put_with_not
⚠️  cond_put, complex_cond_put — etag in If: not yet evaluated

Files changed

  • src/server.rs: +350/-30
  • tests/lock.rs: new, 23 tests
  • tests/http.rs: update Allow header assertion
  • tests/webdav.rs: update lock/unlock tests
  • .cargo/config.toml: cross-compilation linker config

image72 added 3 commits May 13, 2026 17:12
- Add LockManager with thread-safe lock registry (RwLock<HashMap>)
- LOCK: parse lockinfo body, support exclusive/shared locks, conflict detection
- UNLOCK: validate Lock-Token header, release lock entries
- Lock enforcement on PUT, PATCH, DELETE, MOVE, MKCOL (423 Locked)
- If: header support for lock-owner bypass on write operations
- PROPPATCH: return 200 OK instead of 403 Forbidden
- Allow header: add PROPPATCH, MKCOL, LOCK, UNLOCK methods
- DAV header already declares Class 1,2,3
- Add aarch64-unknown-linux-musl cross-compilation linker config
…rt + 23 tests

- LockManager::remove_all: clean locks on resource delete/move
- handle_delete/handle_move: call remove_all after fs operations
- Timeout header parsing (Second-N, Infinite, default 300s)
- LockInfo.timeout_seconds with is_expired filtering
- cleanup_expired called on acquire (lazy GC on write path)
- LOCK response includes Timeout header + <D:timeout> XML
- tests/lock.rs: 23 tests covering lock/unlock/enforcement/timeout/delete/move
- LOCK on unmapped paths & collections (remove is_file restriction)
- LOCK refresh: parse If: token, call LockManager::refresh to update timeout
- If: header fix: use find(first) instead of rfind(last) for multi-condition headers
- PROPPATCH enforcement: block PROPPATCH on locked resource for non-owners
- COPY destination enforcement: check lock on copy destination path
- 412 Precondition Failed: when If: token on unlocked resource
- LockManager: add refresh(), get_lock(), has_locks() methods
@sigoden

sigoden commented May 15, 2026

Copy link
Copy Markdown
Owner

dufs is currently stateless, but your current PR make it stateful.

I tend to be against this PR, but I also want to hear more opinions.

@image72

image72 commented May 15, 2026

Copy link
Copy Markdown
Author

Thanks for the review — you're right to flag the statefulness concern. Let me address it directly.

1. This is minimal, non-persistent, self-contained state

LockManager {
    locks: RwLock<HashMap<String, Vec<LockInfo>>>,
}
  • No database, no persistence, no external dependencies (zero new crates — only std::sync::RwLock)
  • All state is lost on server restart, which is explicitly allowed by RFC 4918 §7.6
  • Garbage collection built in: cleanup_expired() on acquire, remove_all() on DELETE/MOVE
  • ~200 lines of core logic, fully encapsulated in LockManager
  • The same pattern dufs already uses: Arc<AtomicBool> for running

2. The current "fake lock" is worse than no lock

Dufs currently returns:

  • Lock-Token response header
  • lockdiscovery XML body
  • DAV: 1, 2, 3 header (declaring Class 2 locking support)

But doesn't actually track or enforce locks. This is the worst of both worlds — WebDAV clients (like macOS Finder) believe the resource is locked, but there's no actual protection. Either we should return 501 Not Implemented for LOCK/UNLOCK, or implement them correctly.

3. Cost vs benefit

Cost
Lines ~350 total (LockManager + handler logic)
Runtime Zero unless a client sends LOCK
Memory ~100-200 bytes per lock; 1000 concurrent locks ~200KB
Latency No impact on request path (lock-only ops)
Dependencies Zero new (std::sync::RwLock only)
Benefit
macOS Finder WebDAV Works (was broken without real locks)
litmus compliance 24/26 → 92% (was 16/26 → 61%)
Data safety Prevents concurrent write conflicts

4. If the project wants extra caution

Happy to add either:

  • A --disable-locks CLI flag
  • A webdav-locks cargo feature (default: on)

But I'd argue this isn't a feature toggle — it's a protocol compliance fix. Dufs already advertises DAV: 1, 2, 3 which means "Class 2" (locking support). The current implementation doesn't fulfill that contract.

The alternative would be to downgrade to DAV: 1 and return 501 Not Implemented for LOCK/UNLOCK. I'm happy to do that instead if you'd prefer to keep the server strictly stateless.

@image72

image72 commented May 15, 2026

Copy link
Copy Markdown
Author

Fair point on the statefulness concern. Instead of debating pros and cons here, maybe this is better decided by the actual users who deploy dufs.

Could we open a quick GitHub Discussion / poll with something like:

Do you need real WebDAV LOCK/UNLOCK support in dufs?

  • 👍 Yes, I need macOS Finder WebDAV or other clients that require locking
  • 👀 No strong opinion, but stateful design concerns me
  • 🚫 No, keep dufs strictly stateless — return 501 for LOCK/UNLOCK

That way the maintainer doesn't have to make the call alone — the community's usage patterns will decide.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants