fix(nextcloud): resume (PROPFIND) on upload chunk
PROPFIND on upload chunk is necessary for application to resume the upload
it is used at least by Nextcloud Android app
issue has been raised via #415
unable to test, so I am using a playbook in test/webdav to simulate a
PROPIND on remote.php/dav/uploads/{user}/{session}
This commit is contained in:
@@ -139,6 +139,92 @@ impl NextcloudChunkedUploadService {
|
||||
.map(|p| p.exists())
|
||||
.unwrap_or(false)
|
||||
}
|
||||
|
||||
/// Enumerate the chunks already stored in a session, plus the
|
||||
/// session directory's own mtime. Used by the PROPFIND handler
|
||||
/// to drive NextCloud's resume-upload flow — the Android client
|
||||
/// (and several mobile clients) issue PROPFIND on the session
|
||||
/// URL to discover which chunks are already uploaded, then only
|
||||
/// PUT the missing ones.
|
||||
///
|
||||
/// Returns `None` when the session directory doesn't exist
|
||||
/// (handler maps to 404). The `.file` and `.assembled` markers
|
||||
/// are filtered out — they're internal bookkeeping, not real
|
||||
/// chunks the client uploaded.
|
||||
pub async fn list_chunks(&self, user: &str, upload_id: &str) -> Result<Option<SessionListing>> {
|
||||
let session_dir = self.safe_session_dir(user, upload_id)?;
|
||||
if !session_dir.exists() {
|
||||
return Ok(None);
|
||||
}
|
||||
|
||||
let session_meta = fs::metadata(&session_dir)
|
||||
.await
|
||||
.map_err(|e| DomainError::internal_error("ChunkedUpload", e.to_string()))?;
|
||||
let session_mtime = session_meta
|
||||
.modified()
|
||||
.ok()
|
||||
.and_then(|t| t.duration_since(std::time::UNIX_EPOCH).ok())
|
||||
.map(|d| d.as_secs())
|
||||
.unwrap_or(0);
|
||||
|
||||
let mut chunks: Vec<ChunkInfo> = Vec::new();
|
||||
let mut dir = fs::read_dir(&session_dir)
|
||||
.await
|
||||
.map_err(|e| DomainError::internal_error("ChunkedUpload", e.to_string()))?;
|
||||
while let Some(entry) = dir
|
||||
.next_entry()
|
||||
.await
|
||||
.map_err(|e| DomainError::internal_error("ChunkedUpload", e.to_string()))?
|
||||
{
|
||||
let name = entry.file_name().to_string_lossy().to_string();
|
||||
// Filter internal markers — `.file` is the NC-protocol
|
||||
// assembly trigger target (it never reaches the disk
|
||||
// because MOVE redirects it), `.assembled` is our own
|
||||
// staging file from `assemble()`. Surfacing either to
|
||||
// the client would confuse its chunk-count check.
|
||||
if name == ".file" || name == ".assembled" {
|
||||
continue;
|
||||
}
|
||||
let meta = entry
|
||||
.metadata()
|
||||
.await
|
||||
.map_err(|e| DomainError::internal_error("ChunkedUpload", e.to_string()))?;
|
||||
let size = meta.len();
|
||||
let mtime = meta
|
||||
.modified()
|
||||
.ok()
|
||||
.and_then(|t| t.duration_since(std::time::UNIX_EPOCH).ok())
|
||||
.map(|d| d.as_secs())
|
||||
.unwrap_or(0);
|
||||
chunks.push(ChunkInfo { name, size, mtime });
|
||||
}
|
||||
// Sort by chunk name so PROPFIND output is deterministic
|
||||
// (clients don't strictly require this, but reproducible
|
||||
// listings make debugging from logs much easier).
|
||||
chunks.sort_by(|a, b| a.name.cmp(&b.name));
|
||||
|
||||
Ok(Some(SessionListing {
|
||||
session_mtime,
|
||||
chunks,
|
||||
}))
|
||||
}
|
||||
}
|
||||
|
||||
/// One chunk file inside an upload session.
|
||||
#[derive(Debug, Clone)]
|
||||
pub struct ChunkInfo {
|
||||
pub name: String,
|
||||
pub size: u64,
|
||||
pub mtime: u64,
|
||||
}
|
||||
|
||||
/// What `list_chunks` returns: the session's own mtime (for the
|
||||
/// collection's `<d:getlastmodified>`) plus the list of stored
|
||||
/// chunks.
|
||||
#[derive(Debug, Clone)]
|
||||
pub struct SessionListing {
|
||||
pub session_mtime: u64,
|
||||
pub chunks: Vec<ChunkInfo>,
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
|
||||
@@ -18,6 +18,7 @@ use crate::interfaces::middleware::auth::{AuthUser, CurrentUser};
|
||||
/// PUT /remote.php/dav/uploads/{user}/{upload_id}/{chunk} → store chunk
|
||||
/// MOVE /remote.php/dav/uploads/{user}/{upload_id}/.file → assemble
|
||||
/// DELETE /remote.php/dav/uploads/{user}/{upload_id} → abort
|
||||
/// PROPFIND /remote.php/dav/uploads/{user}/{upload_id} → list chunks (for resume)
|
||||
pub async fn handle_nc_uploads(
|
||||
state: Arc<AppState>,
|
||||
req: Request<Body>,
|
||||
@@ -31,6 +32,7 @@ pub async fn handle_nc_uploads(
|
||||
"PUT" => handle_put_chunk(state, req, &user, &upload_id, &rest).await,
|
||||
"MOVE" => handle_assemble(state, req, &user, &upload_id).await,
|
||||
"DELETE" => handle_abort(state, &user, &upload_id).await,
|
||||
"PROPFIND" => handle_propfind_session(state, &user, &upload_id).await,
|
||||
_ => Ok(Response::builder()
|
||||
.status(StatusCode::METHOD_NOT_ALLOWED)
|
||||
.body(Body::empty())
|
||||
@@ -38,6 +40,107 @@ pub async fn handle_nc_uploads(
|
||||
}
|
||||
}
|
||||
|
||||
/// PROPFIND on an upload session — used by the NextCloud Android
|
||||
/// client (and several mobile clients) to enumerate which chunks
|
||||
/// are already uploaded before resuming an interrupted transfer.
|
||||
/// Without this handler the client gets `405 METHOD_NOT_ALLOWED`
|
||||
/// and falls back to either failing the upload or starting from
|
||||
/// scratch — neither is acceptable on cellular / flaky links where
|
||||
/// resume is the whole point of chunked upload.
|
||||
///
|
||||
/// Response shape: 207 Multi-Status with one `<d:response>` for the
|
||||
/// session collection itself and one per chunk file. Properties
|
||||
/// returned are the minimum the NC client reads: `resourcetype`,
|
||||
/// `getcontentlength` (chunks only), and `getlastmodified` (so
|
||||
/// clients can detect stale partial uploads). Depth is ignored —
|
||||
/// we always return one level (the session + its direct chunks),
|
||||
/// which matches NC server behaviour.
|
||||
async fn handle_propfind_session(
|
||||
state: Arc<AppState>,
|
||||
user: &CurrentUser,
|
||||
upload_id: &str,
|
||||
) -> Result<Response<Body>, AppError> {
|
||||
let nc = state
|
||||
.nextcloud
|
||||
.as_ref()
|
||||
.ok_or_else(|| AppError::internal_error("Nextcloud services unavailable"))?;
|
||||
|
||||
let listing = nc
|
||||
.chunked_uploads
|
||||
.list_chunks(&user.username, upload_id)
|
||||
.await
|
||||
.map_err(|e| AppError::internal_error(format!("Failed to list chunks: {}", e)))?
|
||||
.ok_or_else(|| AppError::not_found("Upload session not found"))?;
|
||||
|
||||
let session_href = format!("/remote.php/dav/uploads/{}/{}/", user.username, upload_id);
|
||||
let session_last_modified =
|
||||
chrono::DateTime::<chrono::Utc>::from_timestamp(listing.session_mtime as i64, 0)
|
||||
.unwrap_or_else(chrono::Utc::now)
|
||||
.to_rfc2822();
|
||||
|
||||
let mut body = String::new();
|
||||
body.push_str(r#"<?xml version="1.0" encoding="utf-8"?>"#);
|
||||
body.push_str(r#"<d:multistatus xmlns:d="DAV:">"#);
|
||||
|
||||
// Session collection itself.
|
||||
body.push_str("<d:response>");
|
||||
body.push_str(&format!("<d:href>{}</d:href>", xml_escape(&session_href)));
|
||||
body.push_str("<d:propstat><d:prop>");
|
||||
body.push_str("<d:resourcetype><d:collection/></d:resourcetype>");
|
||||
body.push_str(&format!(
|
||||
"<d:getlastmodified>{}</d:getlastmodified>",
|
||||
xml_escape(&session_last_modified)
|
||||
));
|
||||
body.push_str("</d:prop><d:status>HTTP/1.1 200 OK</d:status></d:propstat>");
|
||||
body.push_str("</d:response>");
|
||||
|
||||
// One entry per chunk file.
|
||||
for chunk in &listing.chunks {
|
||||
let chunk_href = format!(
|
||||
"/remote.php/dav/uploads/{}/{}/{}",
|
||||
user.username, upload_id, chunk.name
|
||||
);
|
||||
let chunk_modified = chrono::DateTime::<chrono::Utc>::from_timestamp(chunk.mtime as i64, 0)
|
||||
.unwrap_or_else(chrono::Utc::now)
|
||||
.to_rfc2822();
|
||||
|
||||
body.push_str("<d:response>");
|
||||
body.push_str(&format!("<d:href>{}</d:href>", xml_escape(&chunk_href)));
|
||||
body.push_str("<d:propstat><d:prop>");
|
||||
body.push_str("<d:resourcetype/>");
|
||||
body.push_str(&format!(
|
||||
"<d:getcontentlength>{}</d:getcontentlength>",
|
||||
chunk.size
|
||||
));
|
||||
body.push_str(&format!(
|
||||
"<d:getlastmodified>{}</d:getlastmodified>",
|
||||
xml_escape(&chunk_modified)
|
||||
));
|
||||
body.push_str("</d:prop><d:status>HTTP/1.1 200 OK</d:status></d:propstat>");
|
||||
body.push_str("</d:response>");
|
||||
}
|
||||
|
||||
body.push_str("</d:multistatus>");
|
||||
|
||||
Ok(Response::builder()
|
||||
.status(StatusCode::MULTI_STATUS)
|
||||
.header(header::CONTENT_TYPE, "application/xml; charset=utf-8")
|
||||
.body(Body::from(body))
|
||||
.unwrap())
|
||||
}
|
||||
|
||||
/// Minimal XML escape — every value we inject above is either a
|
||||
/// well-formed RFC 2822 date, a number, or a path segment we
|
||||
/// control, but defense-in-depth keeps the response well-formed
|
||||
/// even if a chunk name ever contained an unexpected character.
|
||||
fn xml_escape(s: &str) -> String {
|
||||
s.replace('&', "&")
|
||||
.replace('<', "<")
|
||||
.replace('>', ">")
|
||||
.replace('"', """)
|
||||
.replace('\'', "'")
|
||||
}
|
||||
|
||||
/// MKCOL — create upload session directory.
|
||||
async fn handle_mkcol(
|
||||
state: Arc<AppState>,
|
||||
|
||||
+212
@@ -0,0 +1,212 @@
|
||||
#!/usr/bin/env bash
|
||||
# =============================================================
|
||||
# OxiCloud – NextCloud chunked-upload PROPFIND (resume support)
|
||||
# =============================================================
|
||||
# Verifies the PROPFIND handler on /remote.php/dav/uploads/{user}/{upload_id}.
|
||||
#
|
||||
# Why this matters: the NextCloud Android client (and several other
|
||||
# mobile NC clients) issue PROPFIND on the upload-session URL before
|
||||
# resuming an interrupted chunked upload. Without it the client gets
|
||||
# 405 METHOD_NOT_ALLOWED and either restarts the whole upload from
|
||||
# scratch or fails outright. This was a real bug reported against the
|
||||
# OxiCloud NC gateway by an admin running NC Android 3.31.1.
|
||||
#
|
||||
# Sequence:
|
||||
# 1. Login → JWT, then mint an app password (NC surface requires
|
||||
# Basic Auth with an app password, not the user's login password).
|
||||
# 2. MKCOL the session → 201.
|
||||
# 3. PROPFIND empty session → 207 Multi-Status with exactly ONE
|
||||
# <d:response> entry (the session collection itself).
|
||||
# 4. PUT two chunks (00000001, 00000002).
|
||||
# 5. PROPFIND again → 207 with THREE entries (collection + 2 chunks).
|
||||
# Asserts the chunk byte counts come back correctly so the
|
||||
# Android client's resume logic can compare against its expected
|
||||
# chunk sizes.
|
||||
# 6. Security checks:
|
||||
# a. PROPFIND with mismatched URL user → 403.
|
||||
# b. PROPFIND on a session that doesn't exist → 404.
|
||||
# c. PROPFIND with no auth → 401.
|
||||
# 7. DELETE the session → 204.
|
||||
#
|
||||
# Prerequisites:
|
||||
# - Server running at base_url
|
||||
# - OXICLOUD_ENABLE_AUTH=true (NC surface is auth-only)
|
||||
# - jq, curl, xmllint in PATH
|
||||
#
|
||||
# Run (from repo root):
|
||||
# bash tests/webdav/test_nextcloud_chunked_upload_propfind.sh
|
||||
# =============================================================
|
||||
|
||||
set -euo pipefail
|
||||
|
||||
SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
|
||||
cd "$SCRIPT_DIR"
|
||||
|
||||
source test.env
|
||||
source common.sh
|
||||
|
||||
# ── helpers ──────────────────────────────────────────────────────────────────
|
||||
|
||||
PASS=0
|
||||
FAIL=0
|
||||
|
||||
pass() { PASS=$(( PASS + 1 )); echo " PASS: $*"; }
|
||||
fail() { FAIL=$(( FAIL + 1 )); echo " FAIL: $*" >&2; exit 1; }
|
||||
|
||||
# All NC-surface curls go through this helper so the Basic-Auth
|
||||
# header (app password, not the user's login password) is applied
|
||||
# uniformly. The NC handler rejects login-password Basic Auth — only
|
||||
# app passwords pass the verify_basic_auth check.
|
||||
nc_curl() {
|
||||
curl -s -u "$username:$APP_PASS" "$@"
|
||||
}
|
||||
|
||||
# Mint an app password using the JWT we just got. The NC handlers
|
||||
# accept only app-password Basic Auth (rfc 7617), never the login
|
||||
# password, so this step is mandatory.
|
||||
#
|
||||
# Request DTO is `CreateAppPasswordRequestDto`: `label` is required,
|
||||
# `scopes` and `expires_in_days` are optional. Sending the wrong
|
||||
# field name makes axum's Json extractor return a plain-text error
|
||||
# body that subsequent jq parsing chokes on with "Invalid numeric
|
||||
# literal" — a confusing error mode worth pinning the field name
|
||||
# against.
|
||||
mint_app_password() {
|
||||
local response
|
||||
response=$(curl -s -X POST \
|
||||
-H "Authorization: Bearer $TOKEN" \
|
||||
-H "Content-Type: application/json" \
|
||||
-d '{"label":"nc-chunked-upload-propfind-test"}' \
|
||||
"$base_url/api/auth/app-passwords")
|
||||
APP_PASS=$(jq -r '.password // empty' <<< "$response" 2>/dev/null || echo "")
|
||||
[[ -n "$APP_PASS" ]] || fail "Could not mint app password (response: $response)"
|
||||
}
|
||||
|
||||
# Count <d:response> children in a multistatus body. We don't use a
|
||||
# full XML parser because the response shape is fixed by our handler
|
||||
# and a plain grep is robust enough for the assertions we need.
|
||||
count_responses() {
|
||||
grep -o '<d:response>' <<< "$1" | wc -l | tr -d ' '
|
||||
}
|
||||
|
||||
echo
|
||||
echo "=== NextCloud chunked-upload PROPFIND (resume support) ==="
|
||||
echo
|
||||
|
||||
# ── authenticate + mint app password ─────────────────────────────────────────
|
||||
|
||||
oxicloud_login
|
||||
mint_app_password
|
||||
|
||||
UPLOAD_ID="test-propfind-$(date +%s)-$$"
|
||||
NC_BASE="$base_url/remote.php/dav/uploads/$username/$UPLOAD_ID"
|
||||
|
||||
# Idempotent pre-test cleanup — drop any stale session from a previous run.
|
||||
nc_curl -o /dev/null -X DELETE "$NC_BASE" > /dev/null 2>&1 || true
|
||||
|
||||
# ── Step 1: MKCOL — create session ───────────────────────────────────────────
|
||||
|
||||
echo " step 1: MKCOL $NC_BASE"
|
||||
STATUS=$(nc_curl -o /dev/null -w "%{http_code}" -X MKCOL "$NC_BASE")
|
||||
[[ "$STATUS" == "201" ]] || fail "MKCOL expected 201, got $STATUS"
|
||||
pass "Session created (MKCOL → 201)"
|
||||
|
||||
# ── Step 2: PROPFIND on empty session — only the collection ──────────────────
|
||||
|
||||
echo " step 2: PROPFIND on empty session"
|
||||
BODY=$(nc_curl -X PROPFIND -H "Depth: 1" "$NC_BASE")
|
||||
[[ "$BODY" == *"<d:multistatus"* ]] \
|
||||
|| fail "PROPFIND empty session: not a multistatus body (got: $BODY)"
|
||||
N=$(count_responses "$BODY")
|
||||
[[ "$N" == "1" ]] \
|
||||
|| fail "PROPFIND empty session: expected 1 <d:response>, got $N (body: $BODY)"
|
||||
[[ "$BODY" == *"<d:collection/>"* ]] \
|
||||
|| fail "PROPFIND empty session: collection marker missing"
|
||||
pass "Empty-session PROPFIND returns 1 response (collection only)"
|
||||
|
||||
# ── Step 3: PUT two chunks ───────────────────────────────────────────────────
|
||||
|
||||
echo " step 3: PUT chunks 00000001 and 00000002"
|
||||
CHUNK1_SIZE=$(printf "first chunk bytes" | wc -c | tr -d ' ')
|
||||
CHUNK2_SIZE=$(printf "second chunk has different size" | wc -c | tr -d ' ')
|
||||
|
||||
STATUS=$(printf "first chunk bytes" | nc_curl -o /dev/null -w "%{http_code}" \
|
||||
-X PUT -H "Content-Type: application/octet-stream" --data-binary @- \
|
||||
"$NC_BASE/00000001")
|
||||
[[ "$STATUS" == "201" ]] || fail "PUT chunk 00000001 expected 201, got $STATUS"
|
||||
|
||||
STATUS=$(printf "second chunk has different size" | nc_curl -o /dev/null -w "%{http_code}" \
|
||||
-X PUT -H "Content-Type: application/octet-stream" --data-binary @- \
|
||||
"$NC_BASE/00000002")
|
||||
[[ "$STATUS" == "201" ]] || fail "PUT chunk 00000002 expected 201, got $STATUS"
|
||||
pass "Both chunks uploaded ($CHUNK1_SIZE B + $CHUNK2_SIZE B)"
|
||||
|
||||
# ── Step 4: PROPFIND populated session ───────────────────────────────────────
|
||||
|
||||
echo " step 4: PROPFIND populated session"
|
||||
BODY=$(nc_curl -X PROPFIND -H "Depth: 1" "$NC_BASE")
|
||||
N=$(count_responses "$BODY")
|
||||
[[ "$N" == "3" ]] \
|
||||
|| fail "PROPFIND populated session: expected 3 <d:response>, got $N (body: $BODY)"
|
||||
pass "Populated PROPFIND returns 3 responses (collection + 2 chunks)"
|
||||
|
||||
# Chunk hrefs appear in the body.
|
||||
[[ "$BODY" == *"/$UPLOAD_ID/00000001"* ]] \
|
||||
|| fail "PROPFIND populated session: chunk 00000001 href missing"
|
||||
[[ "$BODY" == *"/$UPLOAD_ID/00000002"* ]] \
|
||||
|| fail "PROPFIND populated session: chunk 00000002 href missing"
|
||||
pass "Both chunk hrefs present in multistatus body"
|
||||
|
||||
# Chunk content-lengths match what we uploaded — this is the field
|
||||
# the Android client reads to decide whether a stored chunk is
|
||||
# "complete" or "partial".
|
||||
[[ "$BODY" == *"<d:getcontentlength>$CHUNK1_SIZE</d:getcontentlength>"* ]] \
|
||||
|| fail "PROPFIND: chunk 00000001 size $CHUNK1_SIZE missing in body"
|
||||
[[ "$BODY" == *"<d:getcontentlength>$CHUNK2_SIZE</d:getcontentlength>"* ]] \
|
||||
|| fail "PROPFIND: chunk 00000002 size $CHUNK2_SIZE missing in body"
|
||||
pass "Chunk getcontentlength values match upload byte counts"
|
||||
|
||||
# ── Step 5a: Security — URL user doesn't match auth user → 403 ────────────────
|
||||
|
||||
echo " step 5a: PROPFIND with mismatched URL user → 403"
|
||||
STATUS=$(nc_curl -o /dev/null -w "%{http_code}" -X PROPFIND \
|
||||
"$base_url/remote.php/dav/uploads/someoneelse/$UPLOAD_ID")
|
||||
[[ "$STATUS" == "403" ]] \
|
||||
|| fail "Mismatched-user PROPFIND expected 403, got $STATUS"
|
||||
pass "Mismatched URL user rejected with 403"
|
||||
|
||||
# ── Step 5b: Security — nonexistent session → 404 ────────────────────────────
|
||||
|
||||
echo " step 5b: PROPFIND on nonexistent session → 404"
|
||||
STATUS=$(nc_curl -o /dev/null -w "%{http_code}" -X PROPFIND \
|
||||
"$base_url/remote.php/dav/uploads/$username/does-not-exist-$$-$(date +%s)")
|
||||
[[ "$STATUS" == "404" ]] \
|
||||
|| fail "Nonexistent-session PROPFIND expected 404, got $STATUS"
|
||||
pass "Nonexistent session returns 404"
|
||||
|
||||
# ── Step 5c: Security — no auth → 401 ────────────────────────────────────────
|
||||
|
||||
echo " step 5c: PROPFIND with no auth → 401"
|
||||
STATUS=$(curl -s -o /dev/null -w "%{http_code}" -X PROPFIND "$NC_BASE")
|
||||
[[ "$STATUS" == "401" ]] \
|
||||
|| fail "Unauthenticated PROPFIND expected 401, got $STATUS"
|
||||
pass "Unauthenticated request rejected with 401"
|
||||
|
||||
# ── Cleanup ──────────────────────────────────────────────────────────────────
|
||||
|
||||
echo " cleanup..."
|
||||
STATUS=$(nc_curl -o /dev/null -w "%{http_code}" -X DELETE "$NC_BASE")
|
||||
[[ "$STATUS" == "204" ]] || fail "DELETE session expected 204, got $STATUS"
|
||||
pass "Session deleted"
|
||||
|
||||
# Confirm cleanup: PROPFIND now returns 404.
|
||||
STATUS=$(nc_curl -o /dev/null -w "%{http_code}" -X PROPFIND "$NC_BASE")
|
||||
[[ "$STATUS" == "404" ]] \
|
||||
|| fail "Post-delete PROPFIND expected 404, got $STATUS"
|
||||
pass "Post-delete PROPFIND confirms session gone"
|
||||
|
||||
# ── summary ───────────────────────────────────────────────────────────────────
|
||||
|
||||
echo
|
||||
echo "Results: $PASS passed, $FAIL failed."
|
||||
[[ "$FAIL" -eq 0 ]] && echo "All tests passed." || exit 1
|
||||
Reference in New Issue
Block a user