security(upload): cap upload size to prevent memody/disk consumption
add OXICLOUD_CHUNK_MAX_BYTES which correspond to the max upload chunk allowed
(differs from OXICLOUD_MAX_UPLOAD_SIZE which is the max total size of a file)
hurl test validate the change
Streams the request body straight to the chunk file with peak heap of
~one HTTP frame, regardless of chunk size or the configured cap. The
`storage.chunk_max_bytes` config (env `OXICLOUD_CHUNK_MAX_BYTES`,
default 100 MB) bounds a single PUT — separate from `max_upload_size`
which governs whole-file uploads. Without this separation, a client
could submit a chunk up to the whole-file cap (10 GB default) and
monopolise server memory.
This commit is contained in:
@@ -420,29 +420,68 @@ pub async fn create_upload(
|
||||
params(
|
||||
("upload_id" = String, Path, description = "Upload session ID"),
|
||||
("chunk_index" = usize, Query, description = "Zero-based chunk index"),
|
||||
("checksum" = Option<String>, Query, description = "Optional MD5 checksum for integrity verification"),
|
||||
(
|
||||
"checksum" = Option<String>,
|
||||
Query,
|
||||
description = "Optional hex-encoded checksum for integrity verification. \
|
||||
Computed incrementally during the streaming write. \
|
||||
Algorithm is selected by `checksumalg` (default `md5`). \
|
||||
Also accepted via the legacy `Content-MD5` request header."
|
||||
),
|
||||
(
|
||||
"checksumalg" = Option<String>,
|
||||
Query,
|
||||
description = "Algorithm used by `checksum`. One of: `md5` (default, legacy), `sha256` / `sha-256`, `blake3`. \
|
||||
Unknown values return 400."
|
||||
),
|
||||
),
|
||||
request_body(content_type = "application/octet-stream", description = "Raw chunk bytes"),
|
||||
responses(
|
||||
(status = 200, description = "Chunk received", body = crate::application::ports::chunked_upload_ports::ChunkUploadResponseDto),
|
||||
(status = 400, description = "Invalid chunk or checksum mismatch"),
|
||||
(status = 400, description = "Invalid chunk, size mismatch, checksum mismatch, or unknown `checksumalg`"),
|
||||
(status = 404, description = "Upload session not found"),
|
||||
(status = 413, description = "Chunk exceeds `storage.chunk_max_bytes` cap"),
|
||||
),
|
||||
tag = "uploads",
|
||||
security(("bearerAuth" = []))
|
||||
)]
|
||||
pub async fn upload_chunk(
|
||||
state: State<Arc<AppState>>,
|
||||
State(state): State<Arc<AppState>>,
|
||||
auth_user: AuthUser,
|
||||
path: Path<String>,
|
||||
query: Query<ChunkUploadParams>,
|
||||
headers: HeaderMap,
|
||||
request: Request,
|
||||
) -> impl IntoResponse {
|
||||
let body = axum::body::to_bytes(request.into_body(), usize::MAX)
|
||||
// Cap the chunk body at `storage.chunk_max_bytes` (env
|
||||
// `OXICLOUD_CHUNK_MAX_BYTES`, default 100 MB). Previous code used
|
||||
// `usize::MAX` and `unwrap_or_default()` — two compounding bugs:
|
||||
// - No upper bound → an oversized chunk OOMs the server.
|
||||
// - Silent fallback to an empty body on transport error → the
|
||||
// inner size check would either reject (good case) or — if the
|
||||
// declared chunk_size was 0 (illegal but conceivable) — accept
|
||||
// an empty upload as success. Either way the client got no
|
||||
// actionable error.
|
||||
let max_chunk = state.core.config.storage.chunk_max_bytes;
|
||||
let body = match axum::body::to_bytes(request.into_body(), max_chunk).await {
|
||||
Ok(b) => b,
|
||||
Err(e) => {
|
||||
tracing::warn!(
|
||||
error = %e,
|
||||
upload_id = %path.0,
|
||||
max_chunk,
|
||||
"Chunked upload PATCH rejected — body read failed (size cap or transport error)"
|
||||
);
|
||||
return AppError::payload_too_large(format!(
|
||||
"Chunk read failed (cap {} bytes): {}",
|
||||
max_chunk, e
|
||||
))
|
||||
.into_response();
|
||||
}
|
||||
};
|
||||
ChunkedUploadHandler::upload_chunk_impl(State(state), auth_user, path, query, headers, body)
|
||||
.await
|
||||
.unwrap_or_default();
|
||||
ChunkedUploadHandler::upload_chunk_impl(state, auth_user, path, query, headers, body).await
|
||||
.into_response()
|
||||
}
|
||||
|
||||
#[utoipa::path(
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
use axum::{
|
||||
body::{self, Body},
|
||||
body::Body,
|
||||
http::{Request, StatusCode, header},
|
||||
response::Response,
|
||||
};
|
||||
@@ -10,6 +10,7 @@ use crate::common::di::AppState;
|
||||
use crate::common::mime_detect::{filename_from_path, refine_content_type_from_file};
|
||||
use crate::interfaces::errors::AppError;
|
||||
use crate::interfaces::middleware::auth::{AuthUser, CurrentUser};
|
||||
use crate::interfaces::upload_spool::stream_body_to_path;
|
||||
|
||||
/// Dispatch Nextcloud chunked upload WebDAV requests.
|
||||
///
|
||||
@@ -164,6 +165,14 @@ async fn handle_mkcol(
|
||||
}
|
||||
|
||||
/// PUT — store a chunk.
|
||||
///
|
||||
/// Streams the request body straight to the chunk file with peak heap of
|
||||
/// ~one HTTP frame, regardless of chunk size or the configured cap. The
|
||||
/// `storage.chunk_max_bytes` config (env `OXICLOUD_CHUNK_MAX_BYTES`,
|
||||
/// default 100 MB) bounds a single PUT — separate from `max_upload_size`
|
||||
/// which governs whole-file uploads. Without this separation, a client
|
||||
/// could submit a chunk up to the whole-file cap (10 GB default) and
|
||||
/// monopolise server memory.
|
||||
async fn handle_put_chunk(
|
||||
state: Arc<AppState>,
|
||||
req: Request<Body>,
|
||||
@@ -181,15 +190,13 @@ async fn handle_put_chunk(
|
||||
return Err(AppError::bad_request("Missing chunk name"));
|
||||
}
|
||||
|
||||
let max_upload = state.core.config.storage.max_upload_size;
|
||||
let body_bytes = body::to_bytes(req.into_body(), max_upload)
|
||||
.await
|
||||
.map_err(|e| AppError::bad_request(format!("Failed to read chunk body: {}", e)))?;
|
||||
let chunk_path = nc
|
||||
.chunked_uploads
|
||||
.safe_chunk_path(&user.username, upload_id, chunk_name)
|
||||
.map_err(|e| AppError::bad_request(format!("Invalid chunk path: {}", e)))?;
|
||||
|
||||
nc.chunked_uploads
|
||||
.store_chunk(&user.username, upload_id, chunk_name, &body_bytes)
|
||||
.await
|
||||
.map_err(|e| AppError::internal_error(format!("Failed to store chunk: {}", e)))?;
|
||||
let max_chunk = state.core.config.storage.chunk_max_bytes;
|
||||
stream_body_to_path(req.into_body(), &chunk_path, max_chunk).await?;
|
||||
|
||||
Ok(Response::builder()
|
||||
.status(StatusCode::CREATED)
|
||||
|
||||
@@ -6,7 +6,7 @@
|
||||
//! file (off tmpfs when [`StorageConfig::upload_temp_dir`] is configured) and
|
||||
//! BLAKE3-hashed on the fly so the dedup layer can short-circuit on a hit.
|
||||
|
||||
use std::path::PathBuf;
|
||||
use std::path::{Path, PathBuf};
|
||||
|
||||
use axum::body::Body;
|
||||
use http_body_util::BodyStream;
|
||||
@@ -84,3 +84,67 @@ pub async fn spool_body_to_temp(
|
||||
size: total_bytes as u64,
|
||||
})
|
||||
}
|
||||
|
||||
/// Stream an HTTP request body directly to a known destination file,
|
||||
/// enforcing `max_bytes` as a hard size limit.
|
||||
///
|
||||
/// Used by the chunked-upload PUT handlers — each chunk has a deterministic
|
||||
/// on-disk path (computed by `NextcloudChunkedUploadService::safe_chunk_path`
|
||||
/// or the equivalent REST helper), so there's no need for a spool/move
|
||||
/// dance. Peak heap is ~one HTTP frame regardless of chunk size or `max_bytes`.
|
||||
///
|
||||
/// **No hashing** — chunked uploads dedup at the assembled-file level, not
|
||||
/// the chunk level, so computing BLAKE3 here would be wasted work.
|
||||
///
|
||||
/// On size overflow the partial file is removed before the function returns,
|
||||
/// so a client retry against the same chunk name starts from a clean slate.
|
||||
/// On any other I/O error the partial file is also removed and the error
|
||||
/// surfaces — callers can assume the path is either fully written or absent.
|
||||
pub async fn stream_body_to_path(
|
||||
body: Body,
|
||||
path: &Path,
|
||||
max_bytes: usize,
|
||||
) -> Result<u64, AppError> {
|
||||
let mut file = tokio::fs::File::create(path)
|
||||
.await
|
||||
.map_err(|e| AppError::internal_error(format!("Failed to open chunk file: {e}")))?;
|
||||
|
||||
let mut total_bytes: usize = 0;
|
||||
let mut stream = BodyStream::new(body);
|
||||
|
||||
while let Some(frame_result) = stream.next().await {
|
||||
let frame = match frame_result {
|
||||
Ok(f) => f,
|
||||
Err(e) => {
|
||||
drop(file);
|
||||
let _ = tokio::fs::remove_file(path).await;
|
||||
return Err(AppError::bad_request(format!(
|
||||
"Failed to read request body: {e}"
|
||||
)));
|
||||
}
|
||||
};
|
||||
if let Some(chunk) = frame.data_ref() {
|
||||
total_bytes += chunk.len();
|
||||
if total_bytes > max_bytes {
|
||||
drop(file);
|
||||
let _ = tokio::fs::remove_file(path).await;
|
||||
return Err(AppError::payload_too_large(format!(
|
||||
"Chunk exceeds maximum size of {max_bytes} bytes"
|
||||
)));
|
||||
}
|
||||
if let Err(e) = file.write_all(chunk).await {
|
||||
drop(file);
|
||||
let _ = tokio::fs::remove_file(path).await;
|
||||
return Err(AppError::internal_error(format!(
|
||||
"Failed to write chunk: {e}"
|
||||
)));
|
||||
}
|
||||
}
|
||||
}
|
||||
file.flush()
|
||||
.await
|
||||
.map_err(|e| AppError::internal_error(format!("Failed to flush chunk file: {e}")))?;
|
||||
drop(file);
|
||||
|
||||
Ok(total_bytes as u64)
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user