max / makenotwork
3 files changed,
+99 insertions,
-5 deletions
| @@ -34,6 +34,15 @@ pub const DB_LOCK_TIMEOUT_SECS: u64 = 30; | |||
| 34 | 34 | /// clear pointer rather than handed a presigned URL that fails at S3. | |
| 35 | 35 | pub const S3_SINGLE_PUT_MAX_BYTES: u64 = 5 * 1024 * 1024 * 1024; | |
| 36 | 36 | ||
| 37 | + | /// Largest source a single server-side `CopyObject` can promote. S3 rejects a | |
| 38 | + | /// one-shot copy above 5 GiB; a larger source must go through ranged multipart | |
| 39 | + | /// `UploadPartCopy`. Numerically equal to [`S3_SINGLE_PUT_MAX_BYTES`] but a | |
| 40 | + | /// different limit — that one bounds a client PUT, this one bounds a server-side | |
| 41 | + | /// copy — so they are named separately and neither should be reused for the | |
| 42 | + | /// other. Without this branch a large upload succeeds and then fails at promote, | |
| 43 | + | /// the worst position to fail in. | |
| 44 | + | pub const S3_SINGLE_COPY_MAX_BYTES: u64 = 5 * 1024 * 1024 * 1024; | |
| 45 | + | ||
| 37 | 46 | /// Lifetime of an internal-API actor assertion, minted at `ssh-key-lookup` and | |
| 38 | 47 | /// forwarded by the CLI for the session. 24h comfortably exceeds any SSH session. | |
| 39 | 48 | pub const INTERNAL_ACTOR_TTL_SECS: i64 = 86_400; |
| @@ -312,6 +312,31 @@ pub async fn promote_staging_to_content( | |||
| 312 | 312 | // public backend with the private bucket as the copy source. | |
| 313 | 313 | let to_public = kind.content_served_from_public_bucket(); | |
| 314 | 314 | ||
| 315 | + | let staging = S3Key::from_stored(staging_key); | |
| 316 | + | ||
| 317 | + | // The source size picks the copy strategy: S3 rejects a single-part | |
| 318 | + | // `CopyObject` above 5 GiB, so a large source has to go through ranged | |
| 319 | + | // multipart `UploadPartCopy` or the promote fails *after* a successful | |
| 320 | + | // upload and scan — the worst position to fail in. The S3 object is the | |
| 321 | + | // authoritative size (same posture as blob confirm), so read it here rather | |
| 322 | + | // than threading a recorded size through all three call sites; one | |
| 323 | + | // HeadObject is negligible next to the object copy that follows. The source | |
| 324 | + | // always lives in `backend`'s (private, staging) bucket, including on the | |
| 325 | + | // cross-bucket public promote. | |
| 326 | + | let src_size = backend | |
| 327 | + | .object_size(staging_key) | |
| 328 | + | .await? | |
| 329 | + | .ok_or_else(|| { | |
| 330 | + | crate::error::AppError::Storage(format!( | |
| 331 | + | "cannot promote {staging_key}: staging object not found in storage" | |
| 332 | + | )) | |
| 333 | + | })? | |
| 334 | + | .max(0) as u64; | |
| 335 | + | let needs_multipart = src_size > crate::constants::S3_SINGLE_COPY_MAX_BYTES; | |
| 336 | + | // Only consulted on the multipart branch; a single CopyObject carries the | |
| 337 | + | // source's content type across on its own. | |
| 338 | + | let content_type = crate::storage::content_type_for(file_type, ext); | |
| 339 | + | ||
| 315 | 340 | // 1. Promote the object in storage (copy staging -> content). Idempotent. | |
| 316 | 341 | if to_public { | |
| 317 | 342 | let public = public_backend.ok_or_else(|| { | |
| @@ -319,13 +344,22 @@ pub async fn promote_staging_to_content( | |||
| 319 | 344 | "cannot promote CDN image {staging_key}: public bucket not configured" | |
| 320 | 345 | )) | |
| 321 | 346 | })?; | |
| 322 | - | public | |
| 323 | - | .copy_object_from(backend.bucket(), &S3Key::from_stored(staging_key), &content) | |
| 324 | - | .await?; | |
| 325 | - | } else { | |
| 347 | + | if needs_multipart { | |
| 348 | + | public | |
| 349 | + | .copy_object_multipart(backend.bucket(), &staging, &content, content_type, src_size, None) | |
| 350 | + | .await?; | |
| 351 | + | } else { | |
| 352 | + | public | |
| 353 | + | .copy_object_from(backend.bucket(), &staging, &content) | |
| 354 | + | .await?; | |
| 355 | + | } | |
| 356 | + | } else if needs_multipart { | |
| 357 | + | // Same-bucket multipart promote: the source bucket is this backend's own. | |
| 326 | 358 | backend | |
| 327 | - | .copy_object(&S3Key::from_stored(staging_key), &content) | |
| 359 | + | .copy_object_multipart(backend.bucket(), &staging, &content, content_type, src_size, None) | |
| 328 | 360 | .await?; | |
| 361 | + | } else { | |
| 362 | + | backend.copy_object(&staging, &content).await?; | |
| 329 | 363 | } | |
| 330 | 364 | ||
| 331 | 365 | // 2. Repoint the row + enqueue the staging delete atomically. |
| @@ -1055,6 +1055,23 @@ pub(crate) fn key_extension(key: &str) -> &str { | |||
| 1055 | 1055 | .unwrap_or("bin") | |
| 1056 | 1056 | } | |
| 1057 | 1057 | ||
| 1058 | + | /// The MIME type registered for `ext` under `file_type`, falling back to | |
| 1059 | + | /// `application/octet-stream` for an extension outside the allow-list. | |
| 1060 | + | /// | |
| 1061 | + | /// Needed by the multipart promote path: a single `CopyObject` carries the | |
| 1062 | + | /// source object's metadata across, but a multipart copy writes a *fresh* | |
| 1063 | + | /// destination object whose content type comes from `CreateMultipartUpload`, so | |
| 1064 | + | /// the promote has to name it explicitly or the served object would default to | |
| 1065 | + | /// the wrong type. | |
| 1066 | + | pub(crate) fn content_type_for(file_type: FileType, ext: &str) -> &'static str { | |
| 1067 | + | file_type | |
| 1068 | + | .allowed_types() | |
| 1069 | + | .iter() | |
| 1070 | + | .find(|(e, _)| e.eq_ignore_ascii_case(ext)) | |
| 1071 | + | .map(|(_, ct)| *ct) | |
| 1072 | + | .unwrap_or("application/octet-stream") | |
| 1073 | + | } | |
| 1074 | + | ||
| 1058 | 1075 | pub(crate) fn sanitize_filename(filename: &str) -> String { | |
| 1059 | 1076 | let sanitized: String = filename | |
| 1060 | 1077 | .chars() | |
| @@ -1449,6 +1466,40 @@ mod tests { | |||
| 1449 | 1466 | } | |
| 1450 | 1467 | ||
| 1451 | 1468 | #[test] | |
| 1469 | + | fn content_type_for_maps_allowed_extensions() { | |
| 1470 | + | // The multipart promote names the destination's type explicitly, so this | |
| 1471 | + | // must agree with the allow-list the upload was validated against. | |
| 1472 | + | assert_eq!(content_type_for(FileType::Video, "mp4"), "video/mp4"); | |
| 1473 | + | assert_eq!(content_type_for(FileType::Video, "webm"), "video/webm"); | |
| 1474 | + | assert_eq!(content_type_for(FileType::Cover, "png"), "image/png"); | |
| 1475 | + | // Extensions arrive from a key, which may carry any case. | |
| 1476 | + | assert_eq!(content_type_for(FileType::Video, "MP4"), "video/mp4"); | |
| 1477 | + | } | |
| 1478 | + | ||
| 1479 | + | #[test] | |
| 1480 | + | fn content_type_for_falls_back_for_unknown_extension() { | |
| 1481 | + | // `key_extension` yields "bin" when a key has no extension; an unknown | |
| 1482 | + | // extension must degrade to a generic type, never panic or mis-label. | |
| 1483 | + | assert_eq!( | |
| 1484 | + | content_type_for(FileType::Video, "bin"), | |
| 1485 | + | "application/octet-stream" | |
| 1486 | + | ); | |
| 1487 | + | assert_eq!( | |
| 1488 | + | content_type_for(FileType::Video, ""), | |
| 1489 | + | "application/octet-stream" | |
| 1490 | + | ); | |
| 1491 | + | } | |
| 1492 | + | ||
| 1493 | + | #[test] | |
| 1494 | + | fn single_copy_ceiling_matches_s3_and_is_distinct_from_the_put_ceiling() { | |
| 1495 | + | // Same number today, different limits: one bounds a client PUT, the | |
| 1496 | + | // other a server-side copy. Pinned so a future change to one doesn't | |
| 1497 | + | // silently move the other (they are deliberately separate constants). | |
| 1498 | + | assert_eq!(crate::constants::S3_SINGLE_COPY_MAX_BYTES, 5 * 1024 * 1024 * 1024); | |
| 1499 | + | assert_eq!(crate::constants::S3_SINGLE_PUT_MAX_BYTES, 5 * 1024 * 1024 * 1024); | |
| 1500 | + | } | |
| 1501 | + | ||
| 1502 | + | #[test] | |
| 1452 | 1503 | fn test_validate_extension() { | |
| 1453 | 1504 | assert!(S3Client::validate_extension(FileType::Audio, "episode.mp3").is_ok()); | |
| 1454 | 1505 | assert!(S3Client::validate_extension(FileType::Audio, "episode.MP3").is_ok()); |