fix(nc/webdav): honour If-Match / If-None-Match on PUT (RFC 7232)
Closes F5/F6. The NC PUT handler now evaluates conditional preconditions before body ingestion and returns 412 Precondition Failed when they fail: - If-None-Match: * on an existing target → 412 (create-if-absent) - If-None-Match with matching ETag → 412 (weak compare) - If-Match: * with no current representation → 412 - If-Match with no listed ETag strong-matching the current → 412 The lookup that drives the precondition reuses the same query the handler already needed for the 201-vs-204 distinction, so this adds no extra DB round-trip. Rejected requests skip body ingestion entirely so a 412 doesn't waste megabytes of bandwidth + disk I/O. Test F5/F6 flipped from 'pinned current 204' to assert 412, plus mirror cases F5b/F6b/F6c/F6d covering the legitimate-success paths so logical-operator regressions can't slip past silently.
This commit is contained in:
Edouard Vanbelle
@@ -543,6 +543,62 @@ fn parse_proppatch_favorite(body: &str) -> Option<u8> {
|
||||
|
||||
// ──────────────────── PUT ────────────────────
|
||||
|
||||
/// Strip the optional `W/` weak prefix and surrounding double-quotes
|
||||
/// from one ETag value in an `If-Match` / `If-None-Match` list. Returns
|
||||
/// `(is_weak, inner)`.
|
||||
fn parse_etag_value(raw: &str) -> (bool, &str) {
|
||||
let trimmed = raw.trim();
|
||||
if let Some(rest) = trimmed.strip_prefix("W/") {
|
||||
(true, rest.trim().trim_matches('"'))
|
||||
} else {
|
||||
(false, trimmed.trim_matches('"'))
|
||||
}
|
||||
}
|
||||
|
||||
/// RFC 7232 §3.2 — `If-None-Match` fails for PUT when:
|
||||
/// - the header value is `*` and a current representation exists, OR
|
||||
/// - any listed ETag matches the current representation (weak comparison
|
||||
/// — weak validators in the request are equivalent to strong for the
|
||||
/// match itself, only If-Match is required to be strong).
|
||||
fn if_none_match_precondition_fails(header: &str, current_etag: Option<&str>) -> bool {
|
||||
let v = header.trim();
|
||||
if v == "*" {
|
||||
return current_etag.is_some();
|
||||
}
|
||||
let Some(current) = current_etag else {
|
||||
return false;
|
||||
};
|
||||
v.split(',').any(|tag| {
|
||||
let (_, parsed) = parse_etag_value(tag);
|
||||
!parsed.is_empty() && parsed == current
|
||||
})
|
||||
}
|
||||
|
||||
/// RFC 7232 §3.1 — `If-Match` fails for PUT when:
|
||||
/// - the resource doesn't currently exist (no strong validator to match), OR
|
||||
/// - the header isn't `*` and no listed ETag strong-matches the current one
|
||||
/// (weak validators in the request never satisfy a strong-match).
|
||||
fn if_match_precondition_fails(header: &str, current_etag: Option<&str>) -> bool {
|
||||
let v = header.trim();
|
||||
let Some(current) = current_etag else {
|
||||
return true;
|
||||
};
|
||||
if v == "*" {
|
||||
return false;
|
||||
}
|
||||
!v.split(',').any(|tag| {
|
||||
let (is_weak, parsed) = parse_etag_value(tag);
|
||||
!is_weak && !parsed.is_empty() && parsed == current
|
||||
})
|
||||
}
|
||||
|
||||
fn precondition_failed_response() -> Response<Body> {
|
||||
Response::builder()
|
||||
.status(StatusCode::PRECONDITION_FAILED)
|
||||
.body(Body::empty())
|
||||
.unwrap()
|
||||
}
|
||||
|
||||
async fn handle_put(
|
||||
state: Arc<AppState>,
|
||||
req: Request<Body>,
|
||||
@@ -566,6 +622,31 @@ async fn handle_put(
|
||||
.and_then(|v| v.to_str().ok())
|
||||
.and_then(|v| v.parse::<i64>().ok());
|
||||
|
||||
// ── Conditional preconditions (RFC 7232 §3.1 / §3.2) ─────────────
|
||||
// Evaluated BEFORE body ingestion so a rejected PUT doesn't waste
|
||||
// bandwidth or disk I/O on a body the server is going to throw away.
|
||||
// The lookup is reused for the create-vs-update distinction below,
|
||||
// so this is also free of an extra DB hit.
|
||||
let existing = file_service.get_file_by_path(&internal_path).await.ok();
|
||||
let current_etag = existing.as_ref().map(|f| f.etag.as_str());
|
||||
|
||||
if let Some(value) = req
|
||||
.headers()
|
||||
.get(header::IF_NONE_MATCH)
|
||||
.and_then(|v| v.to_str().ok())
|
||||
&& if_none_match_precondition_fails(value, current_etag)
|
||||
{
|
||||
return Ok(precondition_failed_response());
|
||||
}
|
||||
if let Some(value) = req
|
||||
.headers()
|
||||
.get(header::IF_MATCH)
|
||||
.and_then(|v| v.to_str().ok())
|
||||
&& if_match_precondition_fails(value, current_etag)
|
||||
{
|
||||
return Ok(precondition_failed_response());
|
||||
}
|
||||
|
||||
// ── Direct PUT cap ───────────────────────────────────────────────
|
||||
// We use `direct_put_max_bytes` (default 1 GiB), not `max_upload_size`
|
||||
// (default 10 GB). Larger files must come through the chunked upload
|
||||
@@ -594,8 +675,9 @@ async fn handle_put(
|
||||
.await?;
|
||||
let content_type = ingested.content_type.clone();
|
||||
|
||||
// Distinguish create (201) vs update (204) for the response status.
|
||||
let existed = file_service.get_file_by_path(&internal_path).await.is_ok();
|
||||
// Distinguish create (201) vs update (204) for the response status,
|
||||
// using the lookup already done above for the precondition check.
|
||||
let existed = existing.is_some();
|
||||
|
||||
// Single streaming path — handles both update and create internally,
|
||||
// swapping the file row onto the already-ingested blob.
|
||||
|
||||
@@ -145,27 +145,83 @@ ACTUAL=$(nc_curl "$NC_FILES_BASE/f1-small.txt")
|
||||
pass "F4: GET after overwrite serves the new bytes (no stale-cache)"
|
||||
|
||||
# ─────────────────────────────────────────────────────────────
|
||||
# F5 / F6 — Conditional PUT (pinned: currently no-op)
|
||||
# F5 / F6 — Conditional PUT (RFC 7232 §3.1/§3.2, RFC 4918 §10)
|
||||
#
|
||||
# F5 covers `If-None-Match: *`: server MUST refuse the PUT with
|
||||
# 412 when the target representation already exists (used by
|
||||
# clients to do "create only if absent"). The mirror case — same
|
||||
# header on a NEW path — must succeed; covered by F5b.
|
||||
#
|
||||
# F6 covers `If-Match: "<etag>"`: server MUST refuse the PUT with
|
||||
# 412 when the supplied ETag doesn't strong-match the current
|
||||
# representation (used by clients to do "update only if
|
||||
# unchanged"). The mirror case — correct ETag → success — is
|
||||
# covered by F6b.
|
||||
# ─────────────────────────────────────────────────────────────
|
||||
echo " F5: PUT with If-None-Match: * on existing path (pinned current: 204, RFC-4918 would be 412)"
|
||||
echo " F5: PUT with If-None-Match: * on existing path → 412"
|
||||
HEADERS=$(nc_curl -D - -o /dev/null -X PUT \
|
||||
-H "If-None-Match: *" -H "Content-Type: text/plain" \
|
||||
--data-binary 'F5-payload' \
|
||||
"$NC_FILES_BASE/f1-small.txt")
|
||||
STATUS=$(awk 'NR==1{print $2}' <<< "$HEADERS" | tr -d '\r')
|
||||
[[ "$STATUS" == "204" || "$STATUS" == "201" ]] \
|
||||
|| fail "F5: unexpected status $STATUS (expected 204 — current ignore-conditional behaviour)"
|
||||
pass "F5: PUT honours no conditional headers today — pinned"
|
||||
[[ "$STATUS" == "412" ]] \
|
||||
|| fail "F5: expected 412 Precondition Failed for If-None-Match: * on existing path, got $STATUS"
|
||||
pass "F5: If-None-Match: * on existing path → 412"
|
||||
|
||||
echo " F6: PUT with If-Match: \"wrong-etag\" (pinned current: succeeds, RFC-4918 would be 412)"
|
||||
echo " F5b: PUT with If-None-Match: * on NEW path → 201/204"
|
||||
HEADERS=$(nc_curl -D - -o /dev/null -X PUT \
|
||||
-H "If-None-Match: *" -H "Content-Type: text/plain" \
|
||||
--data-binary 'F5b-payload' \
|
||||
"$NC_FILES_BASE/f5b-new.txt")
|
||||
STATUS=$(awk 'NR==1{print $2}' <<< "$HEADERS" | tr -d '\r')
|
||||
[[ "$STATUS" == "201" || "$STATUS" == "204" ]] \
|
||||
|| fail "F5b: expected 201/204 for If-None-Match: * on new path, got $STATUS"
|
||||
pass "F5b: If-None-Match: * on new path → $STATUS"
|
||||
|
||||
echo " F6: PUT with If-Match: \"wrong-etag\" → 412"
|
||||
HEADERS=$(nc_curl -D - -o /dev/null -X PUT \
|
||||
-H 'If-Match: "deadbeef-never-matches"' -H "Content-Type: text/plain" \
|
||||
--data-binary 'F6-payload' \
|
||||
"$NC_FILES_BASE/f1-small.txt")
|
||||
STATUS=$(awk 'NR==1{print $2}' <<< "$HEADERS" | tr -d '\r')
|
||||
[[ "$STATUS" == "204" || "$STATUS" == "201" ]] \
|
||||
|| fail "F6: unexpected status $STATUS (expected 204 — current ignore-conditional behaviour)"
|
||||
pass "F6: PUT honours no If-Match today — pinned"
|
||||
[[ "$STATUS" == "412" ]] \
|
||||
|| fail "F6: expected 412 Precondition Failed for non-matching If-Match, got $STATUS"
|
||||
pass "F6: If-Match with non-matching ETag → 412"
|
||||
|
||||
echo " F6b: PUT with correct If-Match → 204"
|
||||
# Fetch the current ETag of f1-small.txt via PROPFIND-ish HEAD,
|
||||
# then re-PUT with that exact value as If-Match. Must succeed.
|
||||
CURRENT_ETAG=$(nc_curl -D - -o /dev/null -X HEAD "$NC_FILES_BASE/f1-small.txt" \
|
||||
| awk 'BEGIN{IGNORECASE=1} /^etag:/ {print $2}' | tr -d '\r')
|
||||
[[ -n "$CURRENT_ETAG" ]] || fail "F6b: could not read current ETag via HEAD"
|
||||
HEADERS=$(nc_curl -D - -o /dev/null -X PUT \
|
||||
-H "If-Match: $CURRENT_ETAG" -H "Content-Type: text/plain" \
|
||||
--data-binary 'F6b-payload' \
|
||||
"$NC_FILES_BASE/f1-small.txt")
|
||||
STATUS=$(awk 'NR==1{print $2}' <<< "$HEADERS" | tr -d '\r')
|
||||
[[ "$STATUS" == "204" ]] \
|
||||
|| fail "F6b: expected 204 for If-Match with correct ETag, got $STATUS"
|
||||
pass "F6b: If-Match with current ETag → 204"
|
||||
|
||||
echo " F6c: PUT with If-Match: * on existing path → 204 (catch-all)"
|
||||
HEADERS=$(nc_curl -D - -o /dev/null -X PUT \
|
||||
-H 'If-Match: *' -H "Content-Type: text/plain" \
|
||||
--data-binary 'F6c-payload' \
|
||||
"$NC_FILES_BASE/f1-small.txt")
|
||||
STATUS=$(awk 'NR==1{print $2}' <<< "$HEADERS" | tr -d '\r')
|
||||
[[ "$STATUS" == "204" ]] \
|
||||
|| fail "F6c: expected 204 for If-Match: * on existing path, got $STATUS"
|
||||
pass "F6c: If-Match: * on existing path → 204"
|
||||
|
||||
echo " F6d: PUT with If-Match on NEW path → 412 (resource absent → cannot match)"
|
||||
HEADERS=$(nc_curl -D - -o /dev/null -X PUT \
|
||||
-H 'If-Match: "anything"' -H "Content-Type: text/plain" \
|
||||
--data-binary 'F6d-payload' \
|
||||
"$NC_FILES_BASE/f6d-new.txt")
|
||||
STATUS=$(awk 'NR==1{print $2}' <<< "$HEADERS" | tr -d '\r')
|
||||
[[ "$STATUS" == "412" ]] \
|
||||
|| fail "F6d: expected 412 for If-Match on absent path, got $STATUS"
|
||||
pass "F6d: If-Match on absent path → 412"
|
||||
|
||||
# ─────────────────────────────────────────────────────────────
|
||||
# F7 — PUT a "large" file → succeeds, GET returns exact bytes
|
||||
|
||||
Reference in New Issue
Block a user