Skip to content

Commit 07d49a2

Browse files
committed
inrondrop: patch some webdav identified depth & mkcol gaps
* Add generated tests Signed-off-by: Harshit Jain <reach@harsh1998.dev>
1 parent 843993f commit 07d49a2

10 files changed

Lines changed: 611 additions & 65 deletions

README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ IronDrop includes an RFC 4918-focused implementation. The WebDAV core engine is
3838
- Supported methods: `OPTIONS`, `PROPFIND`, `PROPPATCH`, `MKCOL`, `PUT`, `DELETE`, `COPY`, `MOVE`, `LOCK`, `UNLOCK`
3939
- WebDAV is feature-gated and disabled by default; enable explicitly with `--enable-webdav true`
4040
- Capability headers: `DAV: 1,2`, `Allow`, `MS-Author-Via`
41-
- `PROPFIND`: `allprop`, `propname`, named `prop`, per-property `propstat` grouping (`200`/`404`), and finite-depth refusal (`403` + `propfind-finite-depth`)
41+
- `PROPFIND`: `allprop`, `propname`, named `prop`, per-property `propstat` grouping (`200`/`404`), with `Depth: 0`, `1`, and recursive `infinity`
4242
- `PROPPATCH`: dead-property `set`/`remove` with `207 Multi-Status` results
4343
- Locking: exclusive write locks, lock refresh, `If` header token evaluation (including `Not` conditions), and token-gated write preconditions
4444
- Tree operations: lock-aware `DELETE` multistatus behavior (`207` with `423`/`424` where applicable)

doc/README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ Current WebDAV support targets RFC 4918 Class 1 + Class 2 core behavior without
2121
- `PROPFIND` semantics: `allprop`, `propname`, named `prop`, and `200`/`404` `propstat` grouping
2222
- `PROPPATCH` semantics: dead-property `set`/`remove` with `207` response model
2323
- Lock semantics: token-gated writes with `If`-header parsing, lock refresh, and lock-aware copy/move/delete preconditions
24-
- `PROPFIND` `Depth: infinity` on collections is refused with RFC-shaped `403` DAV precondition (`propfind-finite-depth`)
24+
- `PROPFIND` depth handling supports `Depth: 0`, `Depth: 1`, and recursive `Depth: infinity` on collections
2525

2626
Known scope limits:
2727

doc/WEBDAV_IMPLEMENTATION.md

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -68,8 +68,9 @@ IronDrop supports the common PROPFIND modes:
6868

6969
and depth handling:
7070

71-
- `Depth: 0` and `Depth: 1` for collections
72-
- `Depth: infinity` on collections refused with RFC-shaped finite-depth precondition response
71+
- `Depth: 0` returns only the target resource
72+
- `Depth: 1` returns the target plus immediate children
73+
- `Depth: infinity` recursively returns all descendants of a collection
7374

7475
```mermaid
7576
flowchart TD

src/webdav.rs

Lines changed: 110 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -116,16 +116,22 @@ fn handle_propfind(request: &Request, base_dir: &Path) -> Result<Response, AppEr
116116
return Err(AppError::NotFound);
117117
}
118118

119-
if depth == DavDepth::Infinity && target_path.is_dir() {
120-
return Ok(propfind_finite_depth_error_response());
121-
}
122-
123119
let mut resources = vec![target_path.clone()];
124-
if depth == DavDepth::One && target_path.is_dir() {
125-
for entry in std::fs::read_dir(&target_path)? {
126-
let entry = entry?;
127-
let entry_path = entry.path();
128-
resources.push(entry_path);
120+
if target_path.is_dir() {
121+
match depth {
122+
DavDepth::Zero => {}
123+
DavDepth::One => {
124+
let mut entries = Vec::new();
125+
for entry in std::fs::read_dir(&target_path)? {
126+
let entry = entry?;
127+
entries.push(entry.path());
128+
}
129+
entries.sort_by(|a, b| a.to_string_lossy().cmp(&b.to_string_lossy()));
130+
resources.extend(entries);
131+
}
132+
DavDepth::Infinity => {
133+
collect_recursive_resources(&target_path, &mut resources)?;
134+
}
129135
}
130136
}
131137

@@ -343,24 +349,24 @@ fn parse_prop_name_from_token(
343349
})
344350
}
345351

346-
fn propfind_finite_depth_error_response() -> Response {
347-
let mut headers = HashMap::new();
348-
headers.insert(
349-
"Content-Type".to_string(),
350-
"application/xml; charset=utf-8".to_string(),
351-
);
352-
Response {
353-
status_code: 403,
354-
status_text: "Forbidden".to_string(),
355-
headers,
356-
body: ResponseBody::Text(
357-
r#"<?xml version="1.0" encoding="utf-8"?>
358-
<D:error xmlns:D="DAV:">
359-
<D:propfind-finite-depth/>
360-
</D:error>"#
361-
.to_string(),
362-
),
352+
fn collect_recursive_resources(root: &Path, resources: &mut Vec<PathBuf>) -> Result<(), AppError> {
353+
let mut stack = vec![root.to_path_buf()];
354+
while let Some(current_dir) = stack.pop() {
355+
let mut children = Vec::new();
356+
for entry in std::fs::read_dir(&current_dir)? {
357+
let entry = entry?;
358+
children.push((entry.path(), entry.file_type()?));
359+
}
360+
children.sort_by(|(a, _), (b, _)| a.to_string_lossy().cmp(&b.to_string_lossy()));
361+
362+
for (entry_path, file_type) in children {
363+
resources.push(entry_path.clone());
364+
if file_type.is_dir() {
365+
stack.push(entry_path);
366+
}
367+
}
363368
}
369+
Ok(())
364370
}
365371

366372
fn handle_mkcol(request: &Request, base_dir: &Path) -> Result<Response, AppError> {
@@ -380,6 +386,11 @@ fn handle_mkcol(request: &Request, base_dir: &Path) -> Result<Response, AppError
380386
if target_path.exists() {
381387
return Ok(status_response(405, "Method Not Allowed"));
382388
}
389+
if !request_body_bytes(request)?.is_empty() {
390+
// We do not implement request-body extensions for MKCOL, so reject
391+
// non-empty bodies instead of silently ignoring them.
392+
return Ok(status_response(415, "Unsupported Media Type"));
393+
}
383394

384395
let Some(parent) = target_path.parent() else {
385396
return Ok(status_response(409, "Conflict"));
@@ -455,6 +466,7 @@ fn handle_delete(request: &Request, base_dir: &Path) -> Result<Response, AppErro
455466
if !target_path.exists() {
456467
return Err(AppError::NotFound);
457468
}
469+
validate_delete_depth_header(&request.headers, target_path.is_dir())?;
458470

459471
if target_path.is_dir() {
460472
let locked_descendants = locked_descendants_without_token(request, &target_path);
@@ -515,6 +527,9 @@ fn handle_copy_or_move(
515527
if !source.exists() {
516528
return Err(AppError::NotFound);
517529
}
530+
if is_move {
531+
validate_move_depth_header(&request.headers, source.is_dir())?;
532+
}
518533

519534
let destination_header = request
520535
.headers
@@ -628,11 +643,11 @@ fn handle_copy_or_move(
628643
fn parse_copy_depth_header(headers: &HashMap<String, String>) -> Result<CopyDepth, AppError> {
629644
let depth = headers
630645
.get("depth")
631-
.map(|value| value.trim())
632-
.unwrap_or("infinity");
633-
match depth {
646+
.map(|value| value.trim().to_ascii_lowercase())
647+
.unwrap_or_else(|| "infinity".to_string());
648+
match depth.as_str() {
634649
"0" => Ok(CopyDepth::Zero),
635-
"infinity" | "Infinity" | "INFINITY" => Ok(CopyDepth::Infinity),
650+
"infinity" => Ok(CopyDepth::Infinity),
636651
_ => Err(AppError::BadRequest),
637652
}
638653
}
@@ -1033,36 +1048,43 @@ fn handle_proppatch(request: &Request, base_dir: &Path) -> Result<Response, AppE
10331048
}
10341049

10351050
let key = lock_key(&target_path);
1036-
let mut ok_props: Vec<(PropName, Option<String>)> = Vec::new();
1051+
let mut ok_or_dependent_props: Vec<(PropName, Option<String>)> = Vec::new();
10371052
let mut missing_props: Vec<(PropName, Option<String>)> = Vec::new();
10381053
let mut forbidden_props: Vec<(PropName, Option<String>)> = Vec::new();
1054+
let mut has_failure = false;
10391055

10401056
let mut guard = dead_props_map()
10411057
.lock()
10421058
.map_err(|_| AppError::InternalServerError("dead properties map poisoned".to_string()))?;
1043-
let props = guard.entry(key).or_default();
1059+
let current_props = guard.get(&key).cloned().unwrap_or_default();
1060+
let mut working_props = current_props.clone();
10441061

10451062
for operation in operations {
10461063
for (name, value) in operation.props {
10471064
if is_protected_property(&name) {
10481065
forbidden_props.push((name, None));
1066+
has_failure = true;
10491067
continue;
10501068
}
10511069
match operation.action {
10521070
PropPatchAction::Set => {
1053-
props.insert(name.clone(), value.unwrap_or_default());
1054-
ok_props.push((name, None));
1071+
working_props.insert(name.clone(), value.unwrap_or_default());
1072+
ok_or_dependent_props.push((name, None));
10551073
}
10561074
PropPatchAction::Remove => {
1057-
if props.remove(&name).is_some() {
1058-
ok_props.push((name, None));
1075+
if working_props.remove(&name).is_some() {
1076+
ok_or_dependent_props.push((name, None));
10591077
} else {
10601078
missing_props.push((name, None));
1079+
has_failure = true;
10611080
}
10621081
}
10631082
}
10641083
}
10651084
}
1085+
if !has_failure {
1086+
guard.insert(key, working_props);
1087+
}
10661088
drop(guard);
10671089

10681090
let mut xml = String::from(
@@ -1075,8 +1097,16 @@ fn handle_proppatch(request: &Request, base_dir: &Path) -> Result<Response, AppE
10751097
xml.push_str(" <D:href>");
10761098
xml.push_str(&xml_escape(&href));
10771099
xml.push_str("</D:href>\n");
1078-
if !ok_props.is_empty() {
1079-
append_propstat(&mut xml, &ok_props, "HTTP/1.1 200 OK");
1100+
if !ok_or_dependent_props.is_empty() {
1101+
if has_failure {
1102+
append_propstat(
1103+
&mut xml,
1104+
&ok_or_dependent_props,
1105+
"HTTP/1.1 424 Failed Dependency",
1106+
);
1107+
} else {
1108+
append_propstat(&mut xml, &ok_or_dependent_props, "HTTP/1.1 200 OK");
1109+
}
10801110
}
10811111
if !missing_props.is_empty() {
10821112
append_propstat(&mut xml, &missing_props, "HTTP/1.1 404 Not Found");
@@ -1235,12 +1265,12 @@ fn parse_prop_elements(
12351265
fn parse_depth_header(headers: &HashMap<String, String>) -> Result<DavDepth, AppError> {
12361266
let depth = headers
12371267
.get("depth")
1238-
.map(|value| value.trim())
1239-
.unwrap_or("infinity");
1240-
match depth {
1268+
.map(|value| value.trim().to_ascii_lowercase())
1269+
.unwrap_or_else(|| "infinity".to_string());
1270+
match depth.as_str() {
12411271
"0" => Ok(DavDepth::Zero),
12421272
"1" => Ok(DavDepth::One),
1243-
"infinity" | "Infinity" | "INFINITY" => Ok(DavDepth::Infinity),
1273+
"infinity" => Ok(DavDepth::Infinity),
12441274
_ => Err(AppError::BadRequest),
12451275
}
12461276
}
@@ -1533,15 +1563,49 @@ fn parse_lock_depth(
15331563
) -> Result<bool, AppError> {
15341564
let depth = headers
15351565
.get("depth")
1536-
.map(|value| value.trim())
1537-
.unwrap_or(if is_collection { "infinity" } else { "0" });
1538-
match depth {
1566+
.map(|value| value.trim().to_ascii_lowercase())
1567+
.unwrap_or_else(|| {
1568+
if is_collection {
1569+
"infinity".to_string()
1570+
} else {
1571+
"0".to_string()
1572+
}
1573+
});
1574+
match depth.as_str() {
15391575
"0" => Ok(false),
1540-
"infinity" | "Infinity" | "INFINITY" if is_collection => Ok(true),
1576+
"infinity" if is_collection => Ok(true),
15411577
_ => Err(AppError::BadRequest),
15421578
}
15431579
}
15441580

1581+
fn validate_delete_depth_header(
1582+
headers: &HashMap<String, String>,
1583+
is_collection: bool,
1584+
) -> Result<(), AppError> {
1585+
if !is_collection {
1586+
return Ok(());
1587+
}
1588+
match headers.get("depth") {
1589+
None => Ok(()),
1590+
Some(value) if value.trim().eq_ignore_ascii_case("infinity") => Ok(()),
1591+
Some(_) => Err(AppError::BadRequest),
1592+
}
1593+
}
1594+
1595+
fn validate_move_depth_header(
1596+
headers: &HashMap<String, String>,
1597+
is_collection: bool,
1598+
) -> Result<(), AppError> {
1599+
if !is_collection {
1600+
return Ok(());
1601+
}
1602+
match headers.get("depth") {
1603+
None => Ok(()),
1604+
Some(value) if value.trim().eq_ignore_ascii_case("infinity") => Ok(()),
1605+
Some(_) => Err(AppError::BadRequest),
1606+
}
1607+
}
1608+
15451609
fn normalize_lock_token(value: &str) -> Option<String> {
15461610
let trimmed = value.trim();
15471611
if trimmed.is_empty() {

0 commit comments

Comments
 (0)