[YuangGao]

Which issue does this PR close?

Part of #8157.

Rationale for this change

The webdav service already supports delete but ignored the OpDelete argument, so If-Match never reached the DELETE request. RFC 9110 defines If-Match for state-changing methods including DELETE, and RFC 4918 recommends entity-tag conditions for WebDAV modifications.

What changes are included in this PR?

• Inject If-Match in webdav_delete.
• Declare delete_with_if_match.
• Return ConditionNotMatch instead of success when a conditional delete gets 404, matching s3 and azblob.
• Document that SabreDAV-based servers need it disabled via CapabilityOverrideLayer, and disable it in CI for nginx, ownCloud and Nextcloud.

Verified against the three servers CI covers. nginx-dav omits ETags from PROPFIND. Nextcloud and ownCloud derive ETags from one-second mtimes, so after an overwrite within the same second a stale ETag still matches and the delete succeeds (nextcloud/server#65078). With the capability on, Nextcloud passes the conditional delete tests except when that race hits test_delete_with_if_match_mismatch.

The delete path deliberately does not pass with_if_match to parse_error. SabreDAV answers a conditional delete of a missing target with 412, which that read-oriented carve-out would report as NotFound instead of ConditionNotMatch.

Are there any user-facing changes?

Yes — op.delete_with(path).if_match(etag) now works against WebDAV servers that expose a usable ETag. No breaking changes.

AI Usage Statement

AI-assisted implementation.

[Xuanwo]

I think having an enable config on a client is weird. It's not something we can control. I suggest we remove or deprecate all settings that follow this pattern.

[YuangGao]

sounds good, that makes sense, I will have a separate PR to deprecate enable_conditional_read

[YuangGao]

#8314

[YuangGao]

the red core / webdav found something real. nextcloud's etag comes from mtime at one-second resolution, same as owncloud. so two writes in the same second share an etag — 13 of 20 rapid overwrites came back unchanged, even though the bodies differed in content and length.

which means a stale etag can delete a revision you never saw. 6 of 10 tries against nextcloud:

PUT "version one"   -> etag E
PUT "version two"   -> same second, still E
DELETE If-Match: E  -> 204, gone

that's the race this capability was meant to remove, and none of the representative servers survive it — nginx-dav has no etag at all, owncloud and nextcloud are both one-second.

i left ci red on purpose instead of overriding nextcloud too, since that would hide the problem while we still advertise the capability.

@Xuanwo so should i drop the capability and note this on #8157, or do you see a deployment-side CapabilityOverrideLayer as enough?

[QazCetelic]

I've also been looking into implementing conditional write #8324 and noticed that some WebDAV implementations don't support If-Match. Some return weak ETags which only work with the If header. However, always using If doesn't work either because other services only support If-Match. So far, I've found that seemingly the best method is to use If-Match if it's a normal ETag and If when it's a weak ETag like this:

if args.if_not_exists() {
    req = req.header(header::IF_NONE_MATCH, "*");
}

let mut dav_if = Vec::new();

if let Some(v) = args.if_none_match() {
    req = req.header(header::IF_NONE_MATCH, v);
    if is_weak_etag(v) {
        // It's safe to send both headers for compatibility since it's a negative match
        dav_if.push(format!("Not [{}]", v.trim()));
    }
}

if let Some(v) = args.if_match() {
    if is_weak_etag(v) {
        // Do not also send If-Match: it can never match a weak validator and would make the combined request fail with 412.
        dav_if.push(format!("[{}]", v.trim()));
    } else {
        req = req.header(header::IF_MATCH, v);
    }
}

if !dav_if.is_empty() {
    req = req.header(HEADER_IF, format!("({})", dav_if.join(" ")));
}

This works on:

• lighttpd
• apache (with a single worker thread)
• nextcloud (aside from [Bug]: F-21 Overwriting a file within the same second keeps the old etag nextcloud/server#65078)

@YuangGao @Xuanwo I feel like the issue with Nextcloud should not be a reason to drop the capability. A faulty implementation of the WebDAV protocol is (if properly documented) not OpenDAL's concern imo.

[YuangGao]

@QazCetelic agreed, keeping the capability. weak ETags are safe for delete: If-Match uses strong comparison, so it fails with 412 instead of deleting. the If header fallback can be a follow-up.

@Xuanwo went with keeping delete_with_if_match and documenting the caveat: the webdav docs now say SabreDAV-based servers (Nextcloud, ownCloud) derive ETags from one-second mtimes and should disable it via CapabilityOverrideLayer, and CI overrides it for nextcloud too, linking nextcloud/server#65078. rebased onto main. the same call would apply to conditional write in #8324. let me know if you'd rather drop the capability instead.

[QazCetelic]

weak ETags are safe for delete: If-Match uses strong comparison, so it fails with 412 instead of deleting. the If header fallback can be a follow-up.

Can't it pick either If or If-Match based on the tag? Weak tags always start with W/ and are wrapped in ", this can be used to pick the header deterministically and avoids sending multiple HTTP requests for 1 action.

[YuangGao]

yeah, that's what I meant by fallback, picking the header from the W/ prefix, one request.

I'd leave it out of this PR though. weak ETags don't always change when the file changes, so a stale one could still match and delete newer content. none of our CI servers return weak ETags either, so I can't test it here. since you're already testing lighttpd and apache for #8324, maybe add it there and reuse it for delete later?