From 607972cf1eab0c2f19e10ac985a352117d9597d8 Mon Sep 17 00:00:00 2001 From: Nils Pukropp Date: Fri, 16 Oct 2026 10:32:05 +0200 Subject: [PATCH] fix(fuse): turn generated thumbnails the way their EXIF orientation says Thumbnails were decoded without the EXIF orientation, so a portrait photo stored as landscape pixels plus a tag got a sideways tile, and the drain uploaded that thumbnail to Drive for every client to show (B226). --- crates/pdfs-fuse/src/photos.rs | 74 +++++++++++++++++++++++++++++++--- docs/BUGS.md | 22 ++++++++++ 2 files changed, 91 insertions(+), 5 deletions(-) diff --git a/crates/pdfs-fuse/src/photos.rs b/crates/pdfs-fuse/src/photos.rs index 519cb2d..e3a2d27 100644 --- a/crates/pdfs-fuse/src/photos.rs +++ b/crates/pdfs-fuse/src/photos.rs @@ -226,11 +226,7 @@ fn scale_thumbnail_with_exiftool( exiftool: &OsStr, timeout: Duration, ) -> ScaleAttempt { - let direct = image::ImageReader::new(std::io::Cursor::new(bytes)) - .with_guessed_format() - .ok() - .and_then(|reader| reader.decode().ok()); - let image = match direct { + let image = match decode_upright(bytes) { Some(image) => image, None if is_raw_image_name(name) => { let (preview, orientation) = @@ -256,6 +252,27 @@ fn scale_thumbnail_with_exiftool( encode_thumb(&image) } +/// Decode `bytes` and turn the picture the way its EXIF orientation says. +/// +/// `ImageReader::decode` does not: a phone stores a portrait photo as landscape +/// pixels plus an orientation tag, so every such thumbnail came out on its +/// side, and the drain uploaded it to Drive that way for every client to show +/// (`docs/BUGS.md` B226). +fn decode_upright(bytes: &[u8]) -> Option { + use image::ImageDecoder as _; + let mut decoder = image::ImageReader::new(std::io::Cursor::new(bytes)) + .with_guessed_format() + .ok()? + .into_decoder() + .ok()?; + let orientation = decoder + .orientation() + .unwrap_or(image::metadata::Orientation::NoTransforms); + let mut image = image::DynamicImage::from_decoder(decoder).ok()?; + image.apply_orientation(orientation); + Some(image) +} + /// Shrink a decoded image to a [`THUMB_EDGE`] JPEG, keeping its aspect ratio. fn encode_thumb(image: &image::DynamicImage) -> ScaleAttempt { let (width, height) = (image.width(), image.height()); @@ -2149,6 +2166,53 @@ mod thumb_tests { assert_eq!(ratio_of(&thumb.bytes).map(|r| r < 1.0), Some(true)); } + /// `jpeg` with an EXIF APP1 segment that carries only `orientation`. + fn jpeg_with_orientation(width: u32, height: u32, orientation: u16) -> Vec { + let plain = jpeg(width, height); + let mut tiff = Vec::new(); + tiff.extend_from_slice(b"II*\0"); + tiff.extend_from_slice(&8u32.to_le_bytes()); + tiff.extend_from_slice(&1u16.to_le_bytes()); + tiff.extend_from_slice(&0x0112u16.to_le_bytes()); + tiff.extend_from_slice(&3u16.to_le_bytes()); + tiff.extend_from_slice(&1u32.to_le_bytes()); + tiff.extend_from_slice(&orientation.to_le_bytes()); + tiff.extend_from_slice(&[0, 0]); + tiff.extend_from_slice(&0u32.to_le_bytes()); + let mut app1 = b"Exif\0\0".to_vec(); + app1.extend_from_slice(&tiff); + let mut out = plain[..2].to_vec(); + out.extend_from_slice(&[0xFF, 0xE1]); + out.extend_from_slice(&u16::try_from(app1.len() + 2).unwrap().to_be_bytes()); + out.extend_from_slice(&app1); + out.extend_from_slice(&plain[2..]); + out + } + + /// A phone stores a portrait photo as landscape pixels and an EXIF + /// orientation; its thumbnail came out on its side (`docs/BUGS.md` B226). + #[test] + fn a_thumbnail_is_turned_the_way_its_exif_says() { + let ScaleAttempt::Made(thumb) = scale_thumbnail( + &jpeg_with_orientation(2000, 1000, 6), + "portrait.jpg", + &std::env::temp_dir(), + ) else { + panic!("a JPEG scales"); + }; + assert!(thumb.ratio < 1.0, "turned upright: {}", thumb.ratio); + assert_eq!(ratio_of(&thumb.bytes).map(|r| r < 1.0), Some(true)); + + let ScaleAttempt::Made(level) = scale_thumbnail( + &jpeg_with_orientation(2000, 1000, 1), + "landscape.jpg", + &std::env::temp_dir(), + ) else { + panic!("a JPEG scales"); + }; + assert!(level.ratio > 1.0); + } + #[test] fn undecodable_bytes_are_not_a_thumbnail() { // What a photo in a format this build has no decoder for looks like: the diff --git a/docs/BUGS.md b/docs/BUGS.md index 4543ddc..be2fa4f 100644 --- a/docs/BUGS.md +++ b/docs/BUGS.md @@ -100,6 +100,7 @@ a live account since. | Entry | Title | |---|---| +| [B226](#b226--thumbnails-of-portrait-phone-photos-lie-on-their-side) | Thumbnails of portrait phone photos lie on their side | | [B225](#b225--a-takeout-photo-without-a-sidecar-takes-a-siblings-name-and-date) | A Takeout photo without a sidecar takes a sibling's name and date | | [B223](#b223--a-damaged-config-file-is-replaced-with-defaults-on-the-next-settings-change) | A damaged config file is replaced with defaults on the next settings change | | [B224](#b224--fallocate-zero-ranges-are-undone-and-collapse-ranges-corrupt-the-file) | `fallocate` zero ranges are undone and collapse ranges corrupt the file | @@ -328,6 +329,27 @@ a live account since. Newest first. +### B226 — Thumbnails of portrait phone photos lie on their side + +**Status:** Fixed, not verified live +**Found:** 2026-10-16, architecture audit (`FINDINGS.md` 2.4.4). +**Where:** `decode_upright` and `scale_thumbnail_with_exiftool` in +`crates/pdfs-fuse/src/photos.rs` + +**Repro:** Upload a portrait photo taken with a phone that stores landscape pixels plus an EXIF +orientation tag, which most do. Its tile in the app, and in Proton's own apps, lies on its side. + +**Cause.** Generated thumbnails were decoded with `ImageReader::decode`, which ignores the EXIF +orientation. Only the RAW preview path applied it. The drain uploads the generated thumbnail with +the photo, so every client showed it sideways. + +**Fix.** `decode_upright` reads the orientation from the decoder and applies it before the image +is scaled. + +**Test:** `photos::tests::a_thumbnail_is_turned_the_way_its_exif_says`. + +--- + ### B225 — A Takeout photo without a sidecar takes a sibling's name and date **Status:** Fixed, not verified live