fix(media-proxy): serve originals and speed up slow transforms (#3393)

This commit is contained in:
Hampus authored and GitHub committed 2026-10-11 14:41:01 +02:00
1 parent 44bc29b101
commit aa3b058ce6
29 files changed
+632 -83

No files matched your search

+58 -19
View File
@@ -6,13 +6,20 @@ use crate::{
metrics::cache::CoalescerMetrics,
};
use parking_lot::Mutex;
use std::{collections::HashMap, future::Future, sync::Arc, time::Instant};
use std::{
collections::HashMap,
future::Future,
sync::Arc,
time::{Duration, Instant},
};
use thiserror::Error;
use tokio::sync::{Notify, OwnedSemaphorePermit, Semaphore, TryAcquireError};
#[cfg(test)]
mod tests;
const TIMED_OUT_RETRY_AFTER: Duration = Duration::from_secs(60);
#[derive(Clone, Copy, Debug, Error, Eq, PartialEq)]
pub enum CoalescerError {
#[error("native transform queue is full")]
@@ -23,6 +30,8 @@ pub enum CoalescerError {
BufferBudgetExhausted,
#[error("native transform allocation failed")]
AllocationFailed,
#[error("media too large to process")]
TooLargeToProcess,
#[error("request timed out")]
RequestTimeout,
#[error("coalesced work failed")]
@@ -43,6 +52,12 @@ struct Slot {
notify: Notify,
}
#[derive(Debug, Default)]
struct Keys {
in_flight: HashMap<String, Arc<Slot>>,
timed_out: HashMap<String, Instant>,
}
#[derive(Clone, Copy, Debug, Eq, PartialEq)]
enum CoordinatorState {
Active,
@@ -58,8 +73,12 @@ struct CoordinatorGuard<'a> {
}
impl CoordinatorGuard<'_> {
fn publish<C>(&mut self, result: Result<BudgetedBytes, CoalescerError>, complete: C)
where
fn publish<C>(
&mut self,
result: Result<BudgetedBytes, CoalescerError>,
timed_out: bool,
complete: C,
) where
C: FnOnce(&BudgetedBytes),
{
if let Ok(bytes) = &result {
@@ -70,7 +89,7 @@ impl CoordinatorGuard<'_> {
assert!(state.is_none());
*state = Some(result);
}
self.coalescer.release(self.key);
self.coalescer.release(self.key, timed_out);
self.state = CoordinatorState::Published;
self.slot.notify.notify_waiters();
}
@@ -87,14 +106,14 @@ impl Drop for CoordinatorGuard<'_> {
*state = Some(Err(CoalescerError::WorkCancelled));
}
}
self.coalescer.release(self.key);
self.coalescer.release(self.key, false);
self.slot.notify.notify_waiters();
}
}
#[derive(Debug)]
pub struct ByteCoalescer {
in_flight: Mutex<HashMap<String, Arc<Slot>>>,
keys: Mutex<Keys>,
budget: ByteBudget,
coordinator_capacity: Arc<Semaphore>,
waiter_capacity: Arc<Semaphore>,
@@ -114,7 +133,7 @@ impl ByteCoalescer {
assert!(max_in_flight > 0);
assert!(max_waiters > 0);
Self {
in_flight: Mutex::new(HashMap::new()),
keys: Mutex::new(Keys::default()),
budget,
coordinator_capacity: Arc::new(Semaphore::new(max_in_flight)),
waiter_capacity: Arc::new(Semaphore::new(max_waiters)),
@@ -134,7 +153,7 @@ impl ByteCoalescer {
let idle = self.idle.notified();
tokio::pin!(idle);
idle.as_mut().enable();
if self.in_flight.lock().is_empty() {
if self.keys.lock().in_flight.is_empty() {
return;
}
idle.await;
@@ -166,11 +185,15 @@ impl ByteCoalescer {
_permit: permit,
state: CoordinatorState::Active,
};
let result = work().await.map_err(coalesced_work_error).and_then(|data| {
let outcome = work().await;
let timed_out = outcome.as_ref().is_err_and(|error| {
error.downcast_ref::<MediaError>() == Some(&MediaError::RequestTimeout)
});
let result = outcome.map_err(coalesced_work_error).and_then(|data| {
data.try_into_budgeted(&self.budget)
.ok_or(CoalescerError::BufferBudgetExhausted)
});
guard.publish(result.clone(), complete);
guard.publish(result.clone(), timed_out, complete);
return result;
}
@@ -201,8 +224,8 @@ impl ByteCoalescer {
}
fn claim(&self, key: &str) -> Result<SlotClaim, CoalescerError> {
let mut in_flight = self.in_flight.lock();
if let Some(existing) = in_flight.get(key) {
let mut keys = self.keys.lock();
if let Some(existing) = keys.in_flight.get(key) {
let permit = self
.waiter_capacity
.clone()
@@ -216,6 +239,13 @@ impl ByteCoalescer {
})?;
return Ok((existing.clone(), None, Some(permit)));
}
if keys
.timed_out
.get(key)
.is_some_and(|until| Instant::now() < *until)
{
return Err(CoalescerError::RequestTimeout);
}
let permit = self
.coordinator_capacity
.clone()
@@ -228,16 +258,22 @@ impl ByteCoalescer {
state: Mutex::new(None),
notify: Notify::new(),
});
in_flight.insert(key.to_owned(), slot.clone());
keys.in_flight.insert(key.to_owned(), slot.clone());
Ok((slot, Some(permit), None))
}
fn release(&self, key: &str) {
fn release(&self, key: &str, timed_out: bool) {
let idle = {
let mut in_flight = self.in_flight.lock();
let removed = in_flight.remove(key);
let mut keys = self.keys.lock();
let removed = keys.in_flight.remove(key);
assert!(removed.is_some());
in_flight.is_empty()
if timed_out {
let now = Instant::now();
keys.timed_out.retain(|_, until| *until > now);
keys.timed_out
.insert(key.to_owned(), now + TIMED_OUT_RETRY_AFTER);
}
keys.in_flight.is_empty()
};
if idle {
self.idle.notify_waiters();
@@ -249,8 +285,11 @@ fn coalesced_work_error(error: anyhow::Error) -> CoalescerError {
if let Some(known) = error.downcast_ref::<CoalescerError>().copied() {
return known;
}
if error.downcast_ref::<MediaError>() == Some(&MediaError::AllocationFailed) {
return CoalescerError::AllocationFailed;
match error.downcast_ref::<MediaError>() {
Some(MediaError::AllocationFailed) => return CoalescerError::AllocationFailed,
Some(MediaError::TooLargeToProcess) => return CoalescerError::TooLargeToProcess,
Some(MediaError::RequestTimeout) => return CoalescerError::RequestTimeout,
_ => {}
}
tracing::error!(error = ?error, "coalesced work failed");
CoalescerError::WorkFailed
+72
View File
@@ -49,6 +49,78 @@ async fn failure_propagates() {
assert_eq!(CoalescerError::WorkFailed, err);
}
#[tokio::test]
async fn work_that_ran_out_its_deadline_answers_retries_without_running_again() {
let coalescer = coalescer();
let runs = AtomicU32::new(0);
for _ in 0..2 {
let err = coalescer
.run_once_until(
"slow-gif",
None,
|| async {
runs.fetch_add(1, Ordering::SeqCst);
Err::<MediaBytes, _>(MediaError::RequestTimeout.into())
},
|_| {},
)
.await
.unwrap_err();
assert_eq!(CoalescerError::RequestTimeout, err);
}
assert_eq!(1, runs.load(Ordering::SeqCst));
coalescer
.run_once_until(
"another-gif",
None,
|| async { Ok(MediaBytes::from(b"fresh".to_vec())) },
|_| {},
)
.await
.expect("other keys still run");
}
#[tokio::test]
async fn shed_admissions_and_other_failures_are_not_remembered() {
let coalescer = coalescer();
let runs = AtomicU32::new(0);
for error in [
CoalescerError::RequestTimeout,
CoalescerError::RequestTimeout,
CoalescerError::AllocationFailed,
] {
assert_eq!(
error,
coalescer
.run_once_until(
"shed",
None,
|| async {
runs.fetch_add(1, Ordering::SeqCst);
Err::<MediaBytes, _>(error.into())
},
|_| {},
)
.await
.unwrap_err()
);
}
for _ in 0..2 {
let _ = coalescer
.run_once_until(
"broken",
None,
|| async {
runs.fetch_add(1, Ordering::SeqCst);
Err::<MediaBytes, _>(MediaError::MediaDecodeFailed.into())
},
|_| {},
)
.await;
}
assert_eq!(5, runs.load(Ordering::SeqCst));
}
#[tokio::test]
async fn waiter_can_time_out_behind_a_slow_leader() {
let coalescer = coalescer();
+1
View File
@@ -16,6 +16,7 @@ pub const MAX_ANIMATED_FRAMES_DEFAULT: u32 = 20_000;
pub const MAX_ANIMATED_TOTAL_PIXELS_DEFAULT: usize = 4 * MAX_MEDIA_IMAGE_PIXELS_DEFAULT;
pub const MAX_JXL_DECODE_PIXELS: usize = 4096 * 4096;
pub const MAX_STATIC_OUTPUT_PIXELS: usize = 6144 * 4096;
pub const MAX_GIF_RESIZE_OUTPUT_PIXELS: usize = 16 << 20;
pub const SVG_RASTER_MAX_DIMENSION: u32 = 4096;
pub const LARGE_SOURCE_LOG_PIXELS: usize = 4096 * 4096;
const _: () = assert!(MAX_ANIMATED_FRAMES_DEFAULT >= 20_000);
@@ -7,6 +7,7 @@ use super::{
MediaError, native_animated_frame_limit, native_buffer_to_media_bytes, native_status_error,
};
use crate::{
constants,
image_transform::ImageOptions,
media_limits::MediaLimits,
metrics::transform::TransformMetrics,
@@ -176,6 +177,15 @@ pub(super) struct GifResizeDims {
pub(super) height: c_int,
}
impl GifResizeDims {
pub(super) fn fits_work_budget(self, frames: c_int) -> bool {
(self.width as usize)
.saturating_mul(self.height as usize)
.saturating_mul(frames.max(1) as usize)
<= constants::MAX_GIF_RESIZE_OUTPUT_PIXELS
}
}
pub(super) fn gif_resize_dims(
sniffed: mime::SniffInfo,
options: &ImageOptions,
@@ -156,6 +156,9 @@ pub(super) fn encode_vips_image(
{
return encode_animated_apng(image, ph, animation, media_limits, animation_loop_count);
}
let width = unsafe { native::fluxer_vips_image_get_width(image.as_ptr()) };
let height = unsafe { native::fluxer_vips_image_get_height(image.as_ptr()) };
let output_pixels = u64::try_from(width).unwrap_or(0) * u64::try_from(height).unwrap_or(0);
if format == OutputFormat::GIF
&& let Some(frame_height) = page_height
&& frame_height > 0
@@ -164,7 +167,13 @@ pub(super) fn encode_vips_image(
image,
resolve_animation_loop_count(image, animation_loop_count),
)?;
let suffix = output_suffix(format, quality, Some(frame_height), effort_override)?;
let suffix = output_suffix(
format,
quality,
Some(frame_height),
effort_override,
output_pixels,
)?;
return write_vips_image_to_vec(
image,
&suffix,
@@ -172,7 +181,7 @@ pub(super) fn encode_vips_image(
animation.deadline_unix_ms,
);
}
let suffix = output_suffix(format, quality, page_height, effort_override)?;
let suffix = output_suffix(format, quality, page_height, effort_override, output_pixels)?;
write_vips_image_to_vec(
image,
&suffix,
@@ -67,7 +67,7 @@ pub(crate) fn validate_jxl_decode_pixels(
.saturating_mul(dims.height as usize)
.saturating_mul(dims.pages.max(1) as usize);
if pixels > constants::MAX_JXL_DECODE_PIXELS {
return Err(MediaError::InvalidImageDimensions);
return Err(MediaError::TooLargeToProcess);
}
Ok(())
}
@@ -96,7 +96,7 @@ pub(crate) fn validate_static_output_pixels(
dims: ImageDimensions,
) -> Result<(), MediaError> {
if static_output_pixels(options, dims) > constants::MAX_STATIC_OUTPUT_PIXELS {
return Err(MediaError::InvalidImageDimensions);
return Err(MediaError::TooLargeToProcess);
}
Ok(())
}
@@ -154,6 +154,8 @@ pub enum MediaError {
MediaTransformFailed,
#[error("invalid image dimensions")]
InvalidImageDimensions,
#[error("media too large to process")]
TooLargeToProcess,
#[error("unsupported media type")]
UnsupportedMediaType,
#[error("stream too long")]
@@ -1,6 +1,6 @@
// SPDX-License-Identifier: AGPL-3.0-or-later
use super::super::animated_transform::gif_resize_dims;
use super::super::animated_transform::{GifResizeDims, gif_resize_dims};
use super::super::{
AnimationLimits, AnimationMode, ImageOptions, ImageQuality, MediaError, ResizeMode,
};
@@ -156,6 +156,27 @@ fn animated_gif_with_trailing_bytes_after_the_trailer_passes_through_byte_identi
assert_eq!(gif, passed_through.bytes);
}
#[test]
fn animated_gif_with_trailing_bytes_after_the_trailer_resizes() {
for tail in [&[0u8; 4096][..], b"trailing garbage"] {
let mut gif = animated_gif_frames(3, 20);
gif.extend_from_slice(tail);
let resized = transform_image(
&gif,
&ImageOptions {
width: Some(8),
format: OutputFormat::GIF,
animation: animated_mode(),
..Default::default()
},
)
.expect("a gif with trailing bytes after the trailer still resizes");
assert_animated_gif(&resized.bytes, 3);
assert_eq!(vec![20u16; 3], gif_frame_delays_cs(&resized.bytes));
}
}
#[test]
fn animated_gif_over_the_frame_cap_passes_through_byte_identically() {
let gif = animated_gif_frames(5, 20);
@@ -398,3 +419,42 @@ fn animated_gif_two_bounds_fit_inside_without_distortion() {
assert_eq!(240, dims.width);
assert_eq!(180, dims.height);
}
#[test]
fn the_gif_resize_work_budget_counts_output_pixels_across_every_frame() {
let thumbnail = GifResizeDims {
width: 480,
height: 270,
};
assert!(thumbnail.fits_work_budget(129));
assert!(!thumbnail.fits_work_budget(130));
assert!(!thumbnail.fits_work_budget(739));
let still = GifResizeDims {
width: 4096,
height: 4096,
};
assert!(still.fits_work_budget(1));
assert!(still.fits_work_budget(0));
assert!(!still.fits_work_budget(2));
}
#[test]
fn an_animated_gif_resize_over_the_work_budget_passes_through_byte_identically() {
let mut gif = animated_gif_frames(2, 20);
gif[6..8].copy_from_slice(&4096u16.to_le_bytes());
gif[8..10].copy_from_slice(&4096u16.to_le_bytes());
let passed_through = transform_image(
&gif,
&ImageOptions {
width: Some(4095),
format: OutputFormat::GIF,
quality: ImageQuality::Lossless,
animation: animated_mode(),
..Default::default()
},
)
.expect("a gif too costly to resize still passes through");
assert_eq!("image/gif", passed_through.content_type);
assert_eq!(gif, passed_through.bytes);
}
@@ -25,12 +25,45 @@ fn webp_suffix(requested: EncodeEffort) -> String {
ResolvedImageQuality::High,
None,
Some(requested),
0,
)
.expect("webp output suffix")
.into_string()
.expect("utf8 output suffix")
}
#[test]
fn png_compression_drops_for_outputs_above_a_megapixel() {
let png_suffix = |output_pixels| {
output_suffix(
OutputFormat::PNG,
ResolvedImageQuality::High,
None,
None,
output_pixels,
)
.expect("png output suffix")
.into_string()
.expect("utf8 output suffix")
};
assert_eq!(
".png[strip,compression=9,filter=all]",
png_suffix(160 * 160)
);
assert_eq!(
".png[strip,compression=9,filter=all]",
png_suffix(1024 * 1024)
);
assert_eq!(
".png[strip,compression=6,filter=all]",
png_suffix(1024 * 1024 + 1)
);
assert_eq!(
".png[strip,compression=6,filter=all]",
png_suffix(3024 * 4032)
);
}
#[test]
fn animated_webp_default_effort_matches_fast_tier() {
assert_eq!(2, ResolvedImageQuality::Low.default_effort(true));
@@ -69,6 +69,12 @@ pub(super) fn png_ihdr_dimensions(ihdr: &[u8]) -> Option<(u32, u32)> {
))
}
pub(super) fn png_zlib_level(bytes: &[u8]) -> Option<u8> {
let chunks = parse_png_chunks(bytes).ok()?;
let idat = chunks.iter().find(|chunk| chunk.kind == *b"IDAT")?;
Some(idat.data.get(1)? >> 6)
}
pub(super) fn test_media_limits() -> MediaLimits {
MediaLimits::default_from_config()
}
@@ -267,7 +267,7 @@ fn metadata_refuses_jxl_above_the_decode_pixel_cap() {
.await
.unwrap_err()
});
assert_eq!(MediaError::InvalidImageDimensions, err);
assert_eq!(MediaError::TooLargeToProcess, err);
}
#[test]
@@ -4,12 +4,13 @@ use super::super::image_probe::load_image;
use super::super::native_runtime::ensure_vips_init;
use super::super::transform::source_supports_pages;
use super::super::{ImageOptions, MediaError, NativeCost, image_transform_cost};
use super::fixtures::{animated_mode, metadata_value, transform_image};
use super::fixtures::{animated_mode, metadata_value, png_zlib_level, transform_image};
use crate::{
mime, native,
output_format::OutputFormat,
test_fixtures::{
JXL_64_LOSSY_ALPHA, JXL_4100_LOSSY_ALPHA, png_dimensions, synthetic_bmp, synthetic_png,
JPEG_ROTATED_MISMATCHED_SCAN, JXL_64_LOSSY_ALPHA, JXL_4100_LOSSY_ALPHA, png_dimensions,
synthetic_bmp, synthetic_png,
},
};
@@ -118,7 +119,7 @@ fn refuses_jxl_above_the_decode_pixel_cap_before_decoding() {
},
)
.unwrap_err();
assert_eq!(MediaError::InvalidImageDimensions, err);
assert_eq!(MediaError::TooLargeToProcess, err);
}
}
@@ -147,7 +148,7 @@ fn refuses_full_size_svg_raster_above_the_static_output_cap() {
},
)
.unwrap_err();
assert_eq!(MediaError::InvalidImageDimensions, err);
assert_eq!(MediaError::TooLargeToProcess, err);
let err = transform_image(
svg,
&ImageOptions {
@@ -157,7 +158,7 @@ fn refuses_full_size_svg_raster_above_the_static_output_cap() {
},
)
.unwrap_err();
assert_eq!(MediaError::InvalidImageDimensions, err);
assert_eq!(MediaError::TooLargeToProcess, err);
}
#[test]
@@ -213,3 +214,29 @@ fn transform_cost_charges_svg_at_its_output_size() {
};
assert!(cost < 1 << 20, "cost {cost}");
}
#[test]
fn transforms_a_rotated_jpeg_with_mismatched_scan_headers_at_full_size() {
let out = transform_image(
JPEG_ROTATED_MISMATCHED_SCAN,
&ImageOptions {
format: OutputFormat::PNG,
..Default::default()
},
)
.unwrap();
assert_eq!(Some((768, 1024)), png_dimensions(&out.bytes));
}
#[test]
fn png_outputs_above_a_megapixel_use_default_zlib_compression() {
let png_options = ImageOptions {
format: OutputFormat::PNG,
..Default::default()
};
let small = transform_image(&synthetic_bmp(160, 160), &png_options).unwrap();
assert_eq!(Some(3), png_zlib_level(&small.bytes));
let large = transform_image(&synthetic_bmp(1100, 1000), &png_options).unwrap();
assert_eq!(Some((1100, 1000)), png_dimensions(&large.bytes));
assert_eq!(Some(2), png_zlib_level(&large.bytes));
}
@@ -4,12 +4,13 @@ use super::super::{
MediaError, VideoThumbnailOptions, extract_video_thumbnail,
extract_video_thumbnail_with_options,
};
use super::fixtures::{decode_rgba, metadata_value, test_media_limits};
use super::fixtures::{decode_rgba, metadata_value, png_zlib_level, test_media_limits};
use crate::{
image_quality::ResolvedImageQuality,
output_format::OutputFormat,
test_fixtures::{
ffmpeg_gen_media, ffmpeg_gen_mp4, ffmpeg_gen_rotated_mp4, ffmpeg_mirror_mp4, png_dimensions,
ffmpeg_gen_media, ffmpeg_gen_mp4, ffmpeg_gen_rotated_mp4, ffmpeg_mirror_mp4,
fixture_h264_mp4, png_dimensions,
},
};
@@ -35,6 +36,14 @@ fn color_tagged_video(primaries: &str, transfer: &str, matrix: &str) -> Option<V
)
}
#[test]
fn a_png_video_thumbnail_uses_default_zlib_compression() {
let thumb =
extract_video_thumbnail(&fixture_h264_mp4(), OutputFormat::PNG, &test_media_limits())
.expect("h264 video should thumbnail");
assert_eq!(Some(2), png_zlib_level(&thumb.bytes));
}
#[test]
fn video_thumbnail_accepts_smpte170m_sd_primaries() {
let Some(video) = color_tagged_video("smpte170m", "smpte170m", "smpte170m") else {
@@ -161,10 +161,23 @@ pub fn transform_image(
&& !options.wants_cover_crop()
&& sniffed.mime == "image/gif"
{
let bytes = if let Some(dims) = gif_resize_dims(sniffed, options) {
resize_animated_gif_with_ffmpeg(input, dims, options, media_limits)?
} else {
input.to_vec()
let bytes = match gif_resize_dims(sniffed, options) {
Some(dims) => {
let frames = probe_animated(media_limits, input)?.map_or(1, |probe| probe.pages);
if dims.fits_work_budget(frames) {
resize_animated_gif_with_ffmpeg(input, dims, options, media_limits)?
} else {
tracing::info!(
input_bytes = input.len(),
width = dims.width,
height = dims.height,
frames,
"animated gif resize over the work budget, serving the source"
);
input.to_vec()
}
}
None => input.to_vec(),
};
return Ok(ProcessedMedia {
bytes,
@@ -382,13 +395,9 @@ pub fn transform_image(
}
let loader_options = if animated && source_supports_pages(sniffed.mime) {
if sniffed.mime == "image/jpeg" {
"n=-1,access=sequential"
} else {
"n=-1,access=sequential,fail=true"
}
"n=-1,access=sequential,fail=true"
} else if sniffed.mime == "image/jpeg" {
"access=sequential"
""
} else {
"access=sequential,fail=true"
};
@@ -13,12 +13,14 @@ use std::ffi::CString;
const AUTO_LOSSLESS_PALETTE_ANIMATION_MAX_BYTES: usize = 4 * 1024 * 1024;
const AUTO_LOSSLESS_PALETTE_ANIMATION_MAX_PIXELS: usize = 16 * 1024 * 1024;
const VIPS_WEBP_MAX_EFFORT: u8 = 6;
const PNG_MAX_COMPRESSION_MAX_PIXELS: u64 = 1024 * 1024;
pub(super) fn output_suffix(
format: OutputFormat,
quality: ResolvedImageQuality,
page_height: Option<c_int>,
effort_override: Option<EncodeEffort>,
output_pixels: u64,
) -> Result<CString, MediaError> {
let q = quality.encoder_quality();
let animated = page_height.is_some();
@@ -43,12 +45,19 @@ pub(super) fn output_suffix(
),
}
}
OutputFormat::PNG | OutputFormat::APNG => match page_height {
Some(ph) if format == OutputFormat::APNG => {
format!(".png[strip,compression=9,filter=all,page-height={ph}]")
OutputFormat::PNG | OutputFormat::APNG => {
let compression = if output_pixels > PNG_MAX_COMPRESSION_MAX_PIXELS {
6
} else {
9
};
match page_height {
Some(ph) if format == OutputFormat::APNG => {
format!(".png[strip,compression={compression},filter=all,page-height={ph}]")
}
_ => format!(".png[strip,compression={compression},filter=all]"),
}
_ => ".png[strip,compression=9,filter=all]".to_owned(),
},
}
OutputFormat::GIF => match page_height {
Some(ph) => {
format!(".gif[strip,dither=1.0,effort=7,interframe_maxerror=8.0,page-height={ph}]")
@@ -54,7 +54,7 @@ pub fn extract_video_thumbnail_with_options(
} = options;
let runtime = vips_runtime()?;
let (max_width, max_height) = native_thumbnail_bounds(width, height, media_limits)?;
let suffix = output_suffix(format, quality, None, None)?;
let suffix = output_suffix(format, quality, None, None, u64::MAX)?;
let mut out_ptr: *mut c_void = ptr::null_mut();
let mut out_size: size_t = 0;
let mut out_capacity: size_t = 0;
@@ -193,7 +193,7 @@ int fluxer_read_gif_frame_delays(const uint8_t *bytes, size_t len,
block_count++;
uint8_t introducer = bytes[offset++];
if (introducer == 0x3b) {
if (offset != len || has_pending_delay || frame_count != expected_frames) {
if (has_pending_delay || frame_count != expected_frames) {
return FLUXER_NATIVE_STATUS_CODEC_FAILURE;
}
if (out_loop_count != NULL) *out_loop_count = loop_count;
@@ -148,7 +148,7 @@ int fluxer_validate_complete_gif(const uint8_t *bytes, size_t len,
context.block_count++;
uint8_t introducer = context.bytes[context.offset++];
if (introducer == 0x3b) {
if (context.image_count == 0 || context.offset != context.len ||
if (context.image_count == 0 ||
context.image_count > (size_t)INT_MAX) {
return FLUXER_NATIVE_STATUS_CODEC_FAILURE;
}
@@ -184,6 +184,13 @@ pub(in crate::server) fn content_type_is_trustworthy(content_type: &str) -> bool
)
}
pub(in crate::server) fn is_browser_displayable_image(mime: &str) -> bool {
matches!(
mime,
"image/jpeg" | "image/png" | "image/gif" | "image/webp"
)
}
pub(in crate::server) fn is_svg_content_type(content_type: &str) -> bool {
mime::normalize(Some(content_type))
.is_some_and(|value| value.eq_ignore_ascii_case("image/svg+xml"))
@@ -112,8 +112,10 @@ impl NativeTaskExecutor {
.native_transforms
.wait_until_admitted(&admission, deadline)
.await;
let admitted = Instant::now();
let waited = admitted.duration_since(wait_started);
self.native_metrics
.observe_wait(metrics::duration_millis(wait_started.elapsed()));
.observe_wait(metrics::duration_millis(waited));
let permit = permit?;
let memory = self.memory.clone();
let capacity = self.memory_budget_bytes;
@@ -127,7 +129,7 @@ impl NativeTaskExecutor {
bytes,
capacity, "native work refused, larger than the memory budget"
);
return Err(MediaError::InvalidImageDimensions.into());
return Err(MediaError::TooLargeToProcess.into());
}
NativeCost::Bytes(bytes) => bytes,
NativeCost::UpToBudget(bytes) => bytes.min(capacity),
@@ -139,6 +141,15 @@ impl NativeTaskExecutor {
work()
})
.await
.map_err(|error| {
let ran_short = deadline
.is_some_and(|deadline| deadline.saturating_duration_since(admitted) < waited);
if ran_short && error.downcast_ref() == Some(&MediaError::RequestTimeout) {
TimedSemaphoreError::RequestTimeout.into()
} else {
error
}
})
}
pub(in crate::server) fn begin_shutdown(&self) {
@@ -200,7 +211,7 @@ impl NativeTaskExecutor {
let completion = match deadline {
Some(deadline) => match tokio::time::timeout_at(deadline.into(), receiver).await {
Ok(completion) => completion,
Err(_) => return Err(TimedSemaphoreError::RequestTimeout.into()),
Err(_) => return Err(MediaError::RequestTimeout.into()),
},
None => receiver.await,
};
@@ -311,8 +322,8 @@ mod tests {
.await
.expect_err("the task outlives its deadline");
assert_eq!(
Some(&TimedSemaphoreError::RequestTimeout),
error.downcast_ref::<TimedSemaphoreError>()
Some(&MediaError::RequestTimeout),
error.downcast_ref::<MediaError>()
);
let rendered = metrics.render();
assert_eq!(
@@ -347,6 +358,49 @@ mod tests {
);
}
#[tokio::test]
async fn a_task_admitted_late_times_out_as_an_admission_timeout() {
let metrics = metrics::Metrics::new();
let executor = Arc::new(executor(&metrics, 1, 1));
let (release_holder, mut holder_released) = mpsc::channel::<()>(1);
let (release_queued, mut queued_released) = mpsc::channel::<()>(1);
let holder = Arc::clone(&executor);
let held = tokio::spawn(async move {
holder
.run_native(
None,
|| Ok(NativeCost::Bytes(0)),
move || {
holder_released.blocking_recv();
Ok(())
},
)
.await
});
tokio::time::sleep(Duration::from_millis(20)).await;
let deadline = Instant::now() + Duration::from_millis(300);
let queued = executor.run_native(
Some(deadline),
|| Ok(NativeCost::Bytes(0)),
move || {
queued_released.blocking_recv();
Ok(())
},
);
let release = async {
tokio::time::sleep(Duration::from_millis(240)).await;
drop(release_holder);
};
let (error, ()) = tokio::join!(queued, release);
let error = error.expect_err("the queued task outlives its deadline");
assert_eq!(
Some(&TimedSemaphoreError::RequestTimeout),
error.downcast_ref::<TimedSemaphoreError>()
);
drop(release_queued);
held.await.expect("held task").expect("held work");
}
#[tokio::test]
async fn a_task_that_completes_before_its_deadline_is_never_detached() {
let metrics = metrics::Metrics::new();
@@ -438,7 +492,7 @@ mod tests {
.await
.expect_err("the job exceeds the whole budget");
assert_eq!(
Some(&MediaError::InvalidImageDimensions),
Some(&MediaError::TooLargeToProcess),
error.downcast_ref::<MediaError>()
);
}
+13 -3
View File
@@ -17,7 +17,8 @@ use crate::{
attachment_signature,
format_policy::{
OriginalImageRequest, default_transform_quality,
effective_animated_image_output_format, extension_from_mime, is_svg_content_type,
effective_animated_image_output_format, extension_from_mime,
is_browser_displayable_image, is_svg_content_type,
same_format_loaded_image_request_can_use_original_with_sniff,
},
media_operations::spool_stored_media,
@@ -196,6 +197,7 @@ pub(in crate::server) async fn serve_asset_image(
object,
range_header,
source_format,
sniffed_content_type: sniffed.mime,
detail,
});
}
@@ -219,11 +221,19 @@ struct AssetTransformFailure<'a> {
object: storage::Object,
range_header: Option<&'a str>,
source_format: AssetExtension,
sniffed_content_type: &'static str,
detail: String,
}
fn asset_transform_failure_response(failure: AssetTransformFailure<'_>) -> Response {
let src_ct = failure.object.content_type.as_str();
let stored_ct = failure.object.content_type.as_str();
let src_ct = if !stored_ct.starts_with("image/")
&& is_browser_displayable_image(failure.sniffed_content_type)
{
failure.sniffed_content_type
} else {
stored_ct
};
let src_is_displayable = src_ct.starts_with("image/")
&& src_ct != "image/avif"
&& src_ct != "image/heic"
@@ -234,7 +244,7 @@ fn asset_transform_failure_response(failure: AssetTransformFailure<'_>) -> Respo
return media_response(MediaResponse {
method: failure.method,
data: failure.object.data.into(),
content_type: &failure.object.content_type,
content_type: src_ct,
range_header: failure.range_header,
disposition: None,
});
+55 -1
View File
@@ -2,7 +2,9 @@
use super::*;
use crate::{
config::Config, output_format::OutputFormat, server::asset_path::parse_standard_asset_path,
config::Config,
output_format::OutputFormat,
server::asset_path::{parse_simple_asset_path, parse_standard_asset_path},
storage::Object,
};
use axum::{body::to_bytes, http::header};
@@ -48,6 +50,7 @@ fn asset_transform_failure(error: CoalescerError) -> Response {
object: asset_source_object(),
range_header: None,
source_format: AssetExtension::Png,
sniffed_content_type: "image/png",
detail: "asset key=avatars/1/abc".to_owned(),
})
}
@@ -239,6 +242,7 @@ fn a_transform_failure_on_a_source_the_browser_cannot_show_stays_an_error() {
},
range_header: None,
source_format: AssetExtension::Svg,
sniffed_content_type: "image/svg+xml",
detail: "asset key=avatars/1/abc".to_owned(),
})
};
@@ -252,6 +256,56 @@ fn a_transform_failure_on_a_source_the_browser_cannot_show_stays_an_error() {
);
}
#[test]
fn an_octet_stream_asset_sniffed_as_an_image_browsers_cannot_show_stays_an_error() {
let response = asset_transform_failure_response(AssetTransformFailure {
method: Method::GET,
error: CoalescerError::TooLargeToProcess,
object: Object {
data: Bytes::from_static(&[0xff, 0x0a, 0x00, 0x00]),
content_type: "application/octet-stream".to_owned(),
},
range_header: None,
source_format: AssetExtension::Jxl,
sniffed_content_type: "image/jxl",
detail: "asset key=avatars/1/abc".to_owned(),
});
assert_eq!(StatusCode::INTERNAL_SERVER_ERROR, response.status());
}
#[tokio::test]
async fn an_untransformable_asset_stored_as_octet_stream_serves_the_original_as_its_sniffed_type() {
let fake = crate::storage::tests::fake_s3().await;
let tmp = tempfile::tempdir().unwrap();
let mut cfg = fake.config(tmp.path());
cfg.media.transform_timeout_ms = 60_000;
let app = test_app_state(cfg);
let asset =
parse_simple_asset_path("/emojis/1554531234351165442.webp", AssetKind::Emoji).unwrap();
let mut corrupt_gif = crate::test_fixtures::minimal_gif();
corrupt_gif.truncate(corrupt_gif.len() - 6);
corrupt_gif.extend_from_slice(&[0x02, 0x02, 0xff, 0xff, 0x00, 0x3b]);
fake.put_object(
&format!("{}/{}", app.cfg.storage.bucket_cdn, asset.storage_key),
crate::storage::tests::FakeObject {
body: corrupt_gif.clone(),
content_type: Some("application/octet-stream".to_owned()),
..crate::storage::tests::FakeObject::default()
},
);
let response =
serve_asset_image(&app, Method::GET, asset, &HashMap::new(), &HeaderMap::new()).await;
assert_eq!(StatusCode::OK, response.status());
assert_eq!(
Some("image/gif"),
response
.headers()
.get(header::CONTENT_TYPE)
.and_then(|value| value.to_str().ok())
);
assert_eq!(&corrupt_gif[..], &response_body(response).await[..]);
}
#[tokio::test]
async fn a_stored_passthrough_stream_declares_the_length_it_serves() {
let tmp = tempfile::tempdir().unwrap();
@@ -52,7 +52,10 @@ where
.cache()
.get_or_run(cache_key, format, deadline, work)
.await;
if matches!(result, Err(CoalescerError::WorkFailed)) {
if matches!(
result,
Err(CoalescerError::WorkFailed | CoalescerError::TooLargeToProcess)
) {
runtime.metrics().record_transform_failure();
}
result
@@ -63,7 +66,7 @@ pub(in crate::server) fn coalescer_failure_response(
timeout_code: &'static str,
) -> Option<Response> {
let reason = match error {
CoalescerError::WorkFailed => return None,
CoalescerError::WorkFailed | CoalescerError::TooLargeToProcess => return None,
CoalescerError::RequestTimeout => {
return Some(text_with_reason(
StatusCode::GATEWAY_TIMEOUT,
@@ -123,19 +123,17 @@ pub(in crate::server) async fn run_video_transform(
}
pub(in crate::server) fn coalesced_work_result<T>(result: anyhow::Result<T>) -> anyhow::Result<T> {
match result {
Ok(value) => Ok(value),
Err(error) if transform_error_is_timeout(&error) => {
Err(anyhow::Error::new(CoalescerError::RequestTimeout))
let Err(error) = result else {
return result;
};
let retyped = match error.downcast_ref::<TimedSemaphoreError>() {
Some(TimedSemaphoreError::QueueFull | TimedSemaphoreError::RequestTimeout) => {
CoalescerError::RequestTimeout
}
Err(error)
if error.downcast_ref::<TimedSemaphoreError>()
== Some(&TimedSemaphoreError::Closed) =>
{
Err(anyhow::Error::new(CoalescerError::Unavailable))
}
Err(error) => Err(error),
}
Some(TimedSemaphoreError::Closed) => CoalescerError::Unavailable,
None => return Err(error),
};
Err(anyhow::Error::new(retyped))
}
pub(in crate::server) fn transform_error_is_timeout(error: &anyhow::Error) -> bool {
@@ -187,6 +185,12 @@ mod tests {
Some(&MediaError::MediaDecodeFailed),
failed.downcast_ref::<MediaError>()
);
let ran_out = coalesced_work_result::<()>(Err(MediaError::RequestTimeout.into()))
.expect_err("work that ran out its deadline passes through");
assert_eq!(
Some(&MediaError::RequestTimeout),
ran_out.downcast_ref::<MediaError>()
);
}
#[test]
+27 -4
View File
@@ -16,7 +16,7 @@ use crate::{
image_quality::ImageQuality,
image_transform::{AnimationLimits, AnimationMode, ImageOptions, ResizeMode},
media_limits::MediaLimits,
media_process::MediaBytes,
media_process::{MediaBytes, MediaError},
metrics::{self, transform::TransformMetrics},
mime,
output_format::OutputFormat,
@@ -24,7 +24,8 @@ use crate::{
format_policy::{
OriginalImageRequest, content_type_is_trustworthy, default_transform_quality,
effective_animated_image_output_format, extension_from_mime,
external_default_output_extension, image_extension_from_filename, is_svg_content_type,
external_default_output_extension, image_extension_from_filename,
is_browser_displayable_image, is_svg_content_type,
same_format_loaded_image_request_can_use_original_with_sniff, source_image_format,
transform_response_content_type, transform_static_quality_default,
},
@@ -573,20 +574,42 @@ async fn serve_image_transform(
requested_download: transform.requested_download,
});
}
let can_serve_original = transform.width.is_none()
&& transform.height.is_none()
&& is_browser_displayable_image(sniffed.mime);
let mut too_large_original = None;
let leader_original = &mut too_large_original;
let transformed = match cached_transform(CachedTransformRequest {
runtime,
cache_key,
format: options.format,
deadline: deadline_instant(options.deadline_ms),
work: move || async move {
coalesced_work_result(run_transform(runtime, data, options).await)
.map(|media| MediaBytes::from(media.bytes))
let original = can_serve_original.then(|| data.clone());
let result = coalesced_work_result(run_transform(runtime, data, options).await);
if result
.as_ref()
.is_err_and(|error| error.downcast_ref() == Some(&MediaError::TooLargeToProcess))
{
*leader_original = original;
}
result.map(|media| MediaBytes::from(media.bytes))
},
})
.await
{
Ok(bytes) => bytes.as_bytes().clone(),
Err(error) => {
if let Some(original) = too_large_original {
return original_bytes_response(OriginalBytes {
method,
data: original,
content_type: sniffed.mime,
filename,
range_header,
requested_download: transform.requested_download,
});
}
if let Some(response) = coalescer_failure_response(error, "coalescer_timeout_image") {
return response;
}
@@ -4,8 +4,9 @@ use super::*;
use crate::{
constants::AssetExtension,
server::format_policy::{default_transform_quality, transform_static_quality_default},
test_fixtures::{fixture_h264_mp4, fixture_jpeg, synthetic_png},
test_fixtures::{JXL_4100_LOSSY_ALPHA, fixture_h264_mp4, fixture_jpeg, synthetic_png},
};
use axum::body::to_bytes;
const TRANSFORM_WIDTH: u32 = 32;
@@ -200,6 +201,112 @@ async fn a_redundant_format_parameter_shares_the_implicit_transform_cache_entry(
}
}
async fn attachment_transform(data: &[u8], filename: &str, params: &[(&str, &str)]) -> Response {
let params = params
.iter()
.map(|(name, value)| ((*name).to_owned(), (*value).to_owned()))
.collect::<HashMap<_, _>>();
serve_bytes_or_transform(
&test_runtime(),
ServeBytesRequest {
method: Method::GET,
data: Bytes::copy_from_slice(data),
content_type: "application/octet-stream".to_owned(),
cache_identity: filename,
filename,
route: TransformRoute::Attachment,
params: &params,
headers: &HeaderMap::new(),
},
)
.await
}
#[tokio::test]
async fn a_full_size_transform_too_large_to_process_serves_the_original() {
let png = synthetic_png(6145, 4097);
let response = attachment_transform(&png, "big.png", &[("format", "webp")]).await;
assert_eq!(StatusCode::OK, response.status());
assert_eq!(
Some("image/png"),
response
.headers()
.get(header::CONTENT_TYPE)
.and_then(|value| value.to_str().ok())
);
let body = to_bytes(response.into_body(), usize::MAX)
.await
.expect("body");
assert_eq!(png, body);
}
#[tokio::test]
async fn a_waiter_releases_its_source_while_the_leader_runs() {
const KEY: &str = "attachment:art.png|asset_kind=not-applicable|w=none|h=none|fmt=webp|q=lossless|anim=false|effort=none|resize=fit";
let runtime = test_runtime();
let params = HashMap::from([("format".to_owned(), "webp".to_owned())]);
let headers = HeaderMap::new();
let source = Bytes::from(synthetic_png(48, 48));
let probe = source.clone();
let (release, released) = tokio::sync::oneshot::channel::<()>();
let leader =
runtime
.cache()
.get_or_run(KEY.to_owned(), OutputFormat::WebP, None, || async move {
let _ = released.await;
Ok(MediaBytes::from(vec![0u8; 4]))
});
let waiter = async {
let response = serve_bytes_or_transform(
&runtime,
ServeBytesRequest {
method: Method::GET,
data: source,
content_type: "application/octet-stream".to_owned(),
cache_identity: "art.png",
filename: "art.png",
route: TransformRoute::Attachment,
params: &params,
headers: &headers,
},
);
tokio::pin!(response);
assert!(
tokio::time::timeout(std::time::Duration::from_millis(100), response.as_mut())
.await
.is_err(),
"the request waits on the running leader"
);
assert!(
probe.is_unique(),
"a waiter must not pin its source while the leader runs"
);
let _ = release.send(());
response.await
};
let (leader, response) = tokio::join!(leader, waiter);
leader.expect("leader publishes");
assert_eq!(StatusCode::OK, response.status());
}
#[tokio::test]
async fn a_resized_transform_too_large_to_process_still_fails() {
let response = attachment_transform(
&synthetic_png(6145, 4097),
"big.png",
&[("format", "webp"), ("width", "6145")],
)
.await;
assert_eq!(StatusCode::BAD_REQUEST, response.status());
}
#[tokio::test]
async fn a_full_size_transform_too_large_to_process_never_serves_an_undisplayable_original() {
let response =
attachment_transform(JXL_4100_LOSSY_ALPHA, "big.jxl", &[("format", "webp")]).await;
assert_eq!(StatusCode::BAD_REQUEST, response.status());
}
#[test]
fn the_budget_leaves_room_for_the_cache_and_headroom() {
assert_eq!(
@@ -32,6 +32,7 @@ pub fn apng_header(frames: u32) -> Vec<u8> {
pub const JXL_4100_LOSSY_ALPHA: &[u8] = include_bytes!("jxl_4100_lossy_alpha.jxl");
pub const JXL_64_LOSSY_ALPHA: &[u8] = include_bytes!("jxl_64_lossy_alpha.jxl");
pub const JPEG_ROTATED_MISMATCHED_SCAN: &[u8] = include_bytes!("jpeg_rotated_mismatched_scan.jpg");
pub fn synthetic_png(width: u32, height: u32) -> Vec<u8> {
ensure_vips_init().unwrap();
Binary file not shown.

After

Width:  |  Height:  |  Size: 5.2 KiB

+4 -4
View File
@@ -10,10 +10,10 @@ pub use adversarial::{
};
pub use ffmpeg_cli::{ffmpeg_gen_media, ffmpeg_gen_mp4, ffmpeg_gen_rotated_mp4, ffmpeg_mirror_mp4};
pub use images::{
JXL_64_LOSSY_ALPHA, JXL_4100_LOSSY_ALPHA, animated_gif_fixture, animated_gif_frames,
apng_header, first_webp_anim_frame_size, gif_frame_delays_cs, gif_loop_count, minimal_gif,
png_dimensions, synthetic_bmp, synthetic_png, webp_animation_loop_count, webp_canvas_size,
webp_chunk_payloads, webp_with_metadata_chunk,
JPEG_ROTATED_MISMATCHED_SCAN, JXL_64_LOSSY_ALPHA, JXL_4100_LOSSY_ALPHA, animated_gif_fixture,
animated_gif_frames, apng_header, first_webp_anim_frame_size, gif_frame_delays_cs,
gif_loop_count, minimal_gif, png_dimensions, synthetic_bmp, synthetic_png,
webp_animation_loop_count, webp_canvas_size, webp_chunk_payloads, webp_with_metadata_chunk,
};
pub use media::{
fixture_audio_mp3_with_png_cover_art, fixture_audio_mp4_with_attached_picture,