filip/Stalwart
1
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

Return NOTFOUND error instead of MULTISTATUS on empty PROPFIND responses (fixes #1657)

Browse Source
This commit is contained in:
mdecimus
2025-06-24 16:33:26 +02:00
parent 01f7c8f56d
commit 9ca374151c
3 changed files with 116 additions and 108 deletions
Download Patch File Download Diff File Expand all files Collapse all files

105
crates/dav/src/common/propfind.rs
View File

@@ -326,7 +326,7 @@ impl PropFindRequestHandler for Server {
collection_children = collection_container.child_collection().unwrap();
sync_collection = SyncCollection::from(collection_container);
match get(
get(
self,
access_token,
collection_container,
@@ -340,22 +340,6 @@ impl PropFindRequestHandler for Server {
&mut is_sync_limited,
)
.await?
{
Some(paths) if paths.is_empty() && query.sync_type.is_none() => {
response.add_response(
Response::new_status([query.uri], StatusCode::NOT_FOUND)
.with_response_description("No resources found"),
);
return Ok(HttpResponse::new(StatusCode::MULTI_STATUS)
.with_xml_body(response.to_string()));
}
Some(paths) => paths,
None => {
return Ok(HttpResponse::new(StatusCode::MULTI_STATUS)
.with_xml_body(response.to_string()));
}
}
}
DavQueryResource::Multiget {
hrefs,
@@ -1137,14 +1121,13 @@ impl PropFindRequestHandler for Server {
.unwrap_or(self.core.groupware.max_results as u32)
)),
);
} else if response.response.0.is_empty() && query.sync_type.is_none() {
response.add_response(
Response::new_status([query.uri], StatusCode::NOT_FOUND)
.with_response_description("No resources found"),
);
}
Ok(HttpResponse::new(StatusCode::MULTI_STATUS).with_xml_body(response.to_string()))
if !response.response.0.is_empty() || !query.sync_type.is_none() {
Ok(HttpResponse::new(StatusCode::MULTI_STATUS).with_xml_body(response.to_string()))
} else {
Ok(HttpResponse::new(StatusCode::NOT_FOUND))
}
}
async fn dav_quota(
@@ -1187,7 +1170,7 @@ async fn get(
resource: UriResource<u32, Option<&str>>,
limit: usize,
is_sync_limited: &mut bool,
) -> crate::Result<Option<Vec<PropFindItem>>> {
) -> crate::Result<Vec<PropFindItem>> {
let account_id = resource.account_id;
let container_has_children = collection_children != collection_container;
let resources = data
@@ -1371,27 +1354,25 @@ async fn get(
}
Ok(if let Some(resource) = resource.resource {
Some(
resources
.subtree_with_depth(resource, query.depth)
.filter(|item| {
display_containers.as_ref().is_none_or(|containers| {
if container_has_children {
if item.is_container() {
containers.contains(item.document_id())
} else {
display_children
.as_ref()
.is_some_and(|children| children.contains(item.document_id()))
}
} else {
resources
.subtree_with_depth(resource, query.depth)
.filter(|item| {
display_containers.as_ref().is_none_or(|containers| {
if container_has_children {
if item.is_container() {
containers.contains(item.document_id())
} else {
display_children
.as_ref()
.is_some_and(|children| children.contains(item.document_id()))
}
}) && (!query.depth_no_root || item.path() != resource)
})
.map(|item| PropFindItem::new(resources.format_resource(item), account_id, item))
.collect::<Vec<_>>(),
)
} else {
containers.contains(item.document_id())
}
}) && (!query.depth_no_root || item.path() != resource)
})
.map(|item| PropFindItem::new(resources.format_resource(item), account_id, item))
.collect::<Vec<_>>()
} else {
if !query.depth_no_root && query.sync_type.is_none_or_initial() {
server
@@ -1406,31 +1387,27 @@ async fn get(
}
if query.depth != 0 {
Some(
resources
.tree_with_depth(query.depth - 1)
.filter(|item| {
display_containers.as_ref().is_none_or(|containers| {
if container_has_children {
if item.is_container() {
containers.contains(item.document_id())
} else {
display_children.as_ref().is_some_and(|children| {
children.contains(item.document_id())
})
}
} else {
resources
.tree_with_depth(query.depth - 1)
.filter(|item| {
display_containers.as_ref().is_none_or(|containers| {
if container_has_children {
if item.is_container() {
containers.contains(item.document_id())
} else {
display_children
.as_ref()
.is_some_and(|children| children.contains(item.document_id()))
}
})
} else {
containers.contains(item.document_id())
}
})
.map(|item| {
PropFindItem::new(resources.format_resource(item), account_id, item)
})
.collect::<Vec<_>>(),
)
})
.map(|item| PropFindItem::new(resources.format_resource(item), account_id, item))
.collect::<Vec<_>>()
} else {
None
Vec::new()
}
})
}

50
crates/dav/src/principal/propfind.rs
View File

@@ -288,30 +288,32 @@ impl PrincipalPropFind for Server {
)
};
for account_id in access_token.all_ids_by_collection(collection) {
let href = if account_id == access_token.primary_id() {
format!(
"{}/{}/",
resource_name.base_path(),
percent_encoding::utf8_percent_encode(
&access_token.name,
RFC_3986
),
)
} else {
let name = self
.store()
.get_principal_name(account_id)
.await
.caused_by(trc::location!())?
.unwrap_or_else(|| format!("_{account_id}"));
format!(
"{}/{}/",
resource_name.base_path(),
percent_encoding::utf8_percent_encode(&name, RFC_3986),
)
};
hrefs.push(Href(href));
hrefs.push(Href(format!(
"{}/{}/",
resource_name.base_path(),
percent_encoding::utf8_percent_encode(&name, RFC_3986),
)));
if account_id == access_token.primary_id() {
for account_id in access_token.all_ids_by_collection(collection) {
if account_id != access_token.primary_id() {
let other_name = self
.store()
.get_principal_name(account_id)
.await
.caused_by(trc::location!())?
.unwrap_or_else(|| format!("_{account_id}"));
hrefs.push(Href(format!(
"{}/{}/",
resource_name.base_path(),
percent_encoding::utf8_percent_encode(
&other_name,
RFC_3986
),
)));
}
}
}
fields.push(DavPropertyValue::new(property.clone(), hrefs));

69
tests/src/webdav/principals.rs
View File

@@ -16,6 +16,9 @@ pub async fn test(test: &WebDavTest) {
let principal_path = format!("D:href:{}/", DavResourceName::Principal.base_path());
let jane_principal_path = format!("D:href:{}/jane/", DavResourceName::Principal.base_path());
let path_support_card = format!("D:href:{}/support/", DavResourceName::Card.base_path());
let path_support_cal = format!("D:href:{}/support/", DavResourceName::Cal.base_path());
// Test 1: PROPFIND on /dav/pal should return all principals
let response = client
.propfind(
@@ -52,16 +55,29 @@ pub async fn test(test: &WebDavTest) {
.get(DavProperty::WebDav(WebDavProperty::Owner))
.with_values([path_pal.as_str()])
.with_status(StatusCode::OK);
props
.get(DavProperty::Principal(PrincipalProperty::CalendarHomeSet))
.with_values([path_cal.as_str()])
.with_status(StatusCode::OK);
props
.get(DavProperty::Principal(
PrincipalProperty::AddressbookHomeSet,
))
.with_values([path_card.as_str()])
.with_status(StatusCode::OK);
if *account == "jane" {
props
.get(DavProperty::Principal(PrincipalProperty::CalendarHomeSet))
.with_values([path_cal.as_str(), path_support_cal.as_str()])
.with_status(StatusCode::OK);
props
.get(DavProperty::Principal(
PrincipalProperty::AddressbookHomeSet,
))
.with_values([path_card.as_str(), path_support_card.as_str()])
.with_status(StatusCode::OK);
} else {
props
.get(DavProperty::Principal(PrincipalProperty::CalendarHomeSet))
.with_values([path_cal.as_str()])
.with_status(StatusCode::OK);
props
.get(DavProperty::Principal(
PrincipalProperty::AddressbookHomeSet,
))
.with_values([path_card.as_str()])
.with_status(StatusCode::OK);
}
props
.get(DavProperty::WebDav(WebDavProperty::PrincipalCollectionSet))
.with_values([principal_path.as_str()])
@@ -247,16 +263,29 @@ pub async fn test(test: &WebDavTest) {
.get(DavProperty::WebDav(WebDavProperty::Owner))
.with_values([path_pal.as_str()])
.with_status(StatusCode::OK);
props
.get(DavProperty::Principal(PrincipalProperty::CalendarHomeSet))
.with_values([path_cal.as_str()])
.with_status(StatusCode::OK);
props
.get(DavProperty::Principal(
PrincipalProperty::AddressbookHomeSet,
))
.with_values([path_card.as_str()])
.with_status(StatusCode::OK);
if *account == "jane" {
props
.get(DavProperty::Principal(PrincipalProperty::CalendarHomeSet))
.with_values([path_cal.as_str(), path_support_cal.as_str()])
.with_status(StatusCode::OK);
props
.get(DavProperty::Principal(
PrincipalProperty::AddressbookHomeSet,
))
.with_values([path_card.as_str(), path_support_card.as_str()])
.with_status(StatusCode::OK);
} else {
props
.get(DavProperty::Principal(PrincipalProperty::CalendarHomeSet))
.with_values([path_cal.as_str()])
.with_status(StatusCode::OK);
props
.get(DavProperty::Principal(
PrincipalProperty::AddressbookHomeSet,
))
.with_values([path_card.as_str()])
.with_status(StatusCode::OK);
}
props
.get(DavProperty::WebDav(WebDavProperty::SyncToken))
.with_status(StatusCode::OK)