feat(services/webdav): support conditional read headers by YuangGao · Pull Request #7637 · apache/opendal

YuangGao · 2026-05-29T05:27:53Z

Which issue does this PR close?

Part of #5486.

Rationale for this change

The webdav service already supports read but ignored the OpRead argument, so conditional headers never reached the GET request. Per RFC 7232 (HTTP/1.1 conditional requests, inherited by WebDAV servers), If-Match / If-None-Match / If-Modified-Since / If-Unmodified-Since are standard on GET.

What changes are included in this PR?

Inject the four conditional headers in webdav_get.
Declare read_with_if_match, read_with_if_none_match, read_with_if_modified_since, read_with_if_unmodified_since on the webdav service capability.

Stat-side conditional support (PROPFIND) is left for a follow-up because webdav_stat doesn't currently accept OpStat and PROPFIND conditional semantics are server-dependent.

Are there any user-facing changes?

Yes — op.reader_with(path).if_match(...) (and the three siblings) now work against webdav backends. No breaking changes;

AI Usage Statement

AI-assisted implementation.

YuangGao · 2026-06-04T00:38:38Z

@Xuanwo I believe this pr is ready to be reviewed, thanks!

erickguan

Can you also rebase?

erickguan · 2026-06-21T13:40:36Z

        cat << EOF >> $GITHUB_ENV
        OPENDAL_WEBDAV_ENDPOINT=http://127.0.0.1:8080
-        OPENDAL_TEST_CAPABILITY_OVERRIDES=write_with_user_metadata=false
+        OPENDAL_TEST_CAPABILITY_OVERRIDES=write_with_user_metadata=false,read_with_if_match=false,read_with_if_none_match=false,read_with_if_modified_since=false


Instead of setting OPENDAL_TEST_CAPABILITY_OVERRIDES, we could perhaps add a conditional read support toggle when initialize a backend. And when we read this toggle, we could also enable the toggle by an environment variable, e.g. OPENDAL_WEBDAV_CONDITIONAL_READ.

Inject If-Match, If-None-Match, If-Modified-Since and If-Unmodified-Since in webdav_get, declare the matching read_with_if_* capabilities, and map 412/304 responses to ConditionNotMatch. Add a disable_conditional_read backend option (default false) for servers that don't honor these headers, and turn it on for the nginx fixtures which don't return ETags in PROPFIND. Part of apache#5486.

erickguan · 2026-06-23T00:54:37Z

+    /// Enable this option to drop the four `read_with_if_*` capabilities
+    /// so callers fail fast instead of silently losing the condition.


Suggested change

/// Enable this option to drop the four `read_with_if_*` capabilities

/// so callers fail fast instead of silently losing the condition.

/// Set this option to disable conditional read, specifically, disabling capabilities:

/// - `read_with_if_match`

/// - `read_with_if_none_match`

/// - `read_with_if_modified_since`

/// - `read_with_if_unmodified_since`

/// so callers fail fast instead of silently losing the condition.

I don't understand the last sentence. Is OpenDAL sending data to a webdav server and webdav server will respond with an error? Can we send data to a webdav server without an error with this flag?
Backend has this code:

fn read(&self, ctx: &OperationContext, path: &str, args: OpRead) -> Result<Self::Reader> { let output: oio::StreamReader<WebdavReader> = { Ok(oio::StreamReader::new(WebdavReader::new(

fair point, the wording might be confusing. What I meant: the fail-fast is local — correctness_check returns Unsupported before we ever hit the server. Without the flag, nginx-dav just ignores If-Match and hands you a 200, which is the "silently losing" part

Does this follow your intend?

Suggested change

/// Enable this option to drop the four `read_with_if_*` capabilities

/// so callers fail fast instead of silently losing the condition.

/// Disable conditional read.

///

/// When enable conditional read, which is the default,

/// OpenDAL advertises and sends the RFC 7232 headers when provided arguments when reading data:

/// - `If-Match`

/// - `If-None-Match`

/// - `If-Modified-Since`

/// - `If-Unmodified-Since`

///

/// Some WebDAV-compatible servers (e.g., nginx-dav) don't return

/// ETags in PROPFIND or don't honor these headers on GET.

///

/// Set this option to `true` to disable conditional read, specifically, disabling capabilities:

/// - `read_with_if_match`

/// - `read_with_if_none_match`

/// - `read_with_if_modified_since`

/// - `read_with_if_unmodified_since`

///

/// Users would get `ErrorKind::Unsupported` when trying to read with conditions.

Also, I would go with enable_conditional_read with default true to avoid double negate in documentation. You ponder when reading documentation with double negate. It generally hints more intricate context.

erickguan

I would go with these:

erickguan · 2026-06-24T13:12:06Z

+    /// Enable this option to drop the four `read_with_if_*` capabilities
+    /// so callers fail fast instead of silently losing the condition.


Does this follow your intend?

Suggested change

/// Enable this option to drop the four `read_with_if_*` capabilities

/// so callers fail fast instead of silently losing the condition.

/// Disable conditional read.

///

/// When enable conditional read, which is the default,

/// OpenDAL advertises and sends the RFC 7232 headers when provided arguments when reading data:

/// - `If-Match`

/// - `If-None-Match`

/// - `If-Modified-Since`

/// - `If-Unmodified-Since`

///

/// Some WebDAV-compatible servers (e.g., nginx-dav) don't return

/// ETags in PROPFIND or don't honor these headers on GET.

///

/// Set this option to `true` to disable conditional read, specifically, disabling capabilities:

/// - `read_with_if_match`

/// - `read_with_if_none_match`

/// - `read_with_if_modified_since`

/// - `read_with_if_unmodified_since`

///

/// Users would get `ErrorKind::Unsupported` when trying to read with conditions.

Also, I would go with enable_conditional_read with default true to avoid double negate in documentation. You ponder when reading documentation with double negate. It generally hints more intricate context.

erickguan · 2026-06-24T13:16:38Z

+    /// Disable conditional read headers on GET requests.
+    ///
+    /// By default, OpenDAL advertises and sends the RFC 7232 headers
+    /// `If-Match`, `If-None-Match`, `If-Modified-Since` and
+    /// `If-Unmodified-Since` when callers ask for conditional reads.
+    ///
+    /// Some WebDAV-compatible servers (e.g., nginx-dav) don't return
+    /// ETags in PROPFIND or don't honor these conditions on GET.
+    /// Enable this option to drop the four `read_with_if_*` capabilities
+    /// so callers fail fast instead of silently losing the condition.
+    ///
+    /// Default: false


Suggested change

/// Disable conditional read headers on GET requests.

///

/// By default, OpenDAL advertises and sends the RFC 7232 headers

/// `If-Match`, `If-None-Match`, `If-Modified-Since` and

/// `If-Unmodified-Since` when callers ask for conditional reads.

///

/// Some WebDAV-compatible servers (e.g., nginx-dav) don't return

/// ETags in PROPFIND or don't honor these conditions on GET.

/// Enable this option to drop the four `read_with_if_*` capabilities

/// so callers fail fast instead of silently losing the condition.

///

/// Default: false

/// Disable conditional read.

///

/// When enable conditional read, which is the default,

/// OpenDAL advertises and sends the RFC 7232 headers when provided arguments when reading data:

/// - `If-Match`

/// - `If-None-Match`

/// - `If-Modified-Since`

/// - `If-Unmodified-Since`

///

/// When set to `true`, attempt using conditional read options causes a `ErrorKind::Unsupported` exception.

///

/// Some WebDAV-compatible servers (e.g., nginx-dav) don't return

/// ETags in PROPFIND or don't honor these headers on GET.

///

/// Default: false

I also recommend you to change this to positive tone.

erickguan · 2026-06-24T13:17:12Z

            StatusCode::NOT_FOUND => (ErrorKind::NotFound, false),
            // Some services (like owncloud) return 403 while file locked.
            StatusCode::FORBIDDEN => (ErrorKind::PermissionDenied, true),
+            StatusCode::PRECONDITION_FAILED | StatusCode::NOT_MODIFIED => {


Good!

Suggested change

StatusCode::PRECONDITION_FAILED | StatusCode::NOT_MODIFIED => {

// Some WebDAV-compatible servers (e.g., nginx-dav) don't return ETags in PROPFIND

StatusCode::PRECONDITION_FAILED | StatusCode::NOT_MODIFIED => {

YuangGao · 2026-06-25T00:41:30Z

@erickguan thanks for the review and suggestions! i have renamed it to enable_conditional_read (default true) and reworked the docs along your suggestion

skipped the core.rs wording though — 412/304 only shows up when the server actually honors the headers, so the nginx-dav line didn't quite fit. Put an RFC 7232 mapping note there instead.

erickguan

Thanks a lot for improving WebDAV! Minor formatting comments, so approved already.

erickguan · 2026-06-25T01:19:37Z

+                    Enable conditional read support.
+                    When enabled (the default), OpenDAL forwards the RFC
+                    7232 headers to the server when callers provide
+                    them: - `If-Match` - `If-None-Match` -


This looks like a broken format.

erickguan · 2026-06-25T01:20:01Z

+                    Enable conditional read support.
+                    When enabled (the default), OpenDAL forwards the RFC
+                    7232 headers to the server when callers provide
+                    them: - `If-Match` - `If-None-Match` -


This looks like a broken format

YuangGao marked this pull request as ready for review May 29, 2026 05:31

YuangGao requested a review from Xuanwo as a code owner May 29, 2026 05:31

dosubot Bot added size:S This PR changes 10-29 lines, ignoring generated files. releases-note/feat The PR implements a new feature or has a title that begins with "feat" labels May 29, 2026

YuangGao force-pushed the feat/webdav-conditional-read branch from acc918e to 4879d7d Compare May 29, 2026 05:40

dosubot Bot added size:M This PR changes 30-99 lines, ignoring generated files. and removed size:S This PR changes 10-29 lines, ignoring generated files. labels May 29, 2026

YuangGao force-pushed the feat/webdav-conditional-read branch from 4879d7d to ac45f57 Compare May 29, 2026 05:41

YuangGao requested a review from tisonkun as a code owner June 2, 2026 04:55

erickguan requested changes Jun 21, 2026

View reviewed changes

YuangGao force-pushed the feat/webdav-conditional-read branch from 37e27c9 to 36e7309 Compare June 23, 2026 00:29

YuangGao force-pushed the feat/webdav-conditional-read branch from 36e7309 to 9f9c828 Compare June 23, 2026 00:35

YuangGao requested review from Zheaoli and messense as code owners June 23, 2026 00:35

dosubot Bot added size:L This PR changes 100-499 lines, ignoring generated files. and removed size:M This PR changes 30-99 lines, ignoring generated files. labels Jun 23, 2026

erickguan reviewed Jun 23, 2026

View reviewed changes

erickguan reviewed Jun 24, 2026

View reviewed changes

rename to enable_conditional_read

69c90e6

chore: cargo fmt bindings/python

c4b28c9

erickguan approved these changes Jun 25, 2026

View reviewed changes

dosubot Bot added the lgtm This PR has been approved by a maintainer label Jun 25, 2026

inline header list in conditional-read doc

a1d17cc

erickguan merged commit 98f8edf into apache:main Jun 25, 2026
164 checks passed

		/// Enable this option to drop the four `read_with_if_*` capabilities
		/// so callers fail fast instead of silently losing the condition.

-    /// Enable this option to drop the four `read_with_if_*` capabilities
-    /// so callers fail fast instead of silently losing the condition.
+    /// Set this option to disable conditional read, specifically, disabling capabilities:
+    /// - `read_with_if_match`
+    /// - `read_with_if_none_match`
+    /// - `read_with_if_modified_since`
+    /// - `read_with_if_unmodified_since`
+    /// so callers fail fast instead of silently losing the condition.

-    /// Enable this option to drop the four `read_with_if_*` capabilities
-    /// so callers fail fast instead of silently losing the condition.
+    /// Disable conditional read.
+    ///
+    /// When enable conditional read, which is the default,
+    /// OpenDAL advertises and sends the RFC 7232 headers when provided arguments when reading data:
+    /// - `If-Match`
+    /// - `If-None-Match`
+    /// - `If-Modified-Since`
+    /// - `If-Unmodified-Since`
+    ///
+    /// Some WebDAV-compatible servers (e.g., nginx-dav) don't return
+    /// ETags in PROPFIND or don't honor these headers on GET.
+    ///
+    /// Set this option to `true` to disable conditional read, specifically, disabling capabilities:
+    /// - `read_with_if_match`
+    /// - `read_with_if_none_match`
+    /// - `read_with_if_modified_since`
+    /// - `read_with_if_unmodified_since`
+    ///
+    /// Users would get `ErrorKind::Unsupported` when trying to read with conditions.

	StatusCode::PRECONDITION_FAILED \| StatusCode::NOT_MODIFIED => {
	// Some WebDAV-compatible servers (e.g., nginx-dav) don't return ETags in PROPFIND
	StatusCode::PRECONDITION_FAILED \| StatusCode::NOT_MODIFIED => {

Uh oh!

Conversation

YuangGao commented May 29, 2026

Which issue does this PR close?

Rationale for this change

What changes are included in this PR?

Are there any user-facing changes?

AI Usage Statement

Uh oh!

YuangGao commented Jun 4, 2026

Uh oh!

erickguan left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

erickguan left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

YuangGao commented Jun 25, 2026

Uh oh!

erickguan left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants