diff --git a/src/codecs/bmp/decoder.rs b/src/codecs/bmp/decoder.rs index 7cefbebae5..abc0f64a98 100644 --- a/src/codecs/bmp/decoder.rs +++ b/src/codecs/bmp/decoder.rs @@ -1235,6 +1235,18 @@ impl BmpDecoder { &mut self.reader } + /// The bit depth (`biBitCount`) declared by the embedded BMP header. + /// + /// This is the authoritative bit depth of the image, unlike the + /// `ICONDIRENTRY` bit-depth field, which may be `0` ("unspecified") for ICO + /// or carry the hotspot coordinate for CUR. The ICO decoder uses it to decide + /// whether a true-color image already has native alpha (and thus the `AND` + /// mask must be ignored). + #[cfg(feature = "ico")] + pub(crate) fn source_bit_count(&self) -> u16 { + self.bit_count + } + fn read_file_header(&mut self) -> ImageResult<()> { if self.no_file_header { return Ok(()); diff --git a/src/codecs/ico/decoder.rs b/src/codecs/ico/decoder.rs index 0d7d81426c..c43c96b6ac 100644 --- a/src/codecs/ico/decoder.rs +++ b/src/codecs/ico/decoder.rs @@ -383,6 +383,15 @@ impl ImageDecoder for IcoDecoder { decoder.read_image_data(buf)?; + // Whether the image already has a native alpha channel (and thus + // the AND mask must be ignored) is determined by the embedded BMP's + // real bit depth, NOT the ICONDIRENTRY bit-depth field. The latter + // is unreliable: it may be 0 ("unspecified", then guessed from the + // color count) for ICO, or hold the hotspot Y coordinate for CUR. A + // true 32bpp image whose entry bpp resolves to <32 would otherwise + // wrongly have the AND mask applied, zeroing its native alpha. + let source_bit_count = decoder.source_bit_count(); + let r = decoder.reader(); let image_end = r.stream_position()?; let data_end = self.reader_offset @@ -407,7 +416,7 @@ impl ImageDecoder for IcoDecoder { // 32bpp BMPs already have a native alpha channel, so the // AND mask is ignored. // For lower bit depths, read and apply the AND mask. - if self.selected_entry.bits_per_pixel < 32 { + if source_bit_count < 32 { let rgba = buf.as_chunks_mut::<4>().0; let rows = rgba.chunks_exact_mut(width as usize); @@ -443,11 +452,12 @@ impl ImageDecoder for IcoDecoder { } else if data_end == image_end { // accept images with no mask data Ok(DecodedImageAttributes::default()) - } else if self.spec_strictness == SpecCompliance::Lenient - && self.selected_entry.bits_per_pixel >= 32 + } else if self.spec_strictness == SpecCompliance::Lenient && source_bit_count >= 32 { // In lenient mode, we accept truncated mask data for 32bpp images // since they already have an alpha channel and we ignore the AND mask anyway. + // This mirrors the mask-ignore decision above and is likewise keyed on the + // embedded BMP's real bit depth, not the unreliable ICONDIRENTRY field. Ok(DecodedImageAttributes::default()) } else { Err(DecoderError::InvalidDataSize.into()) @@ -641,6 +651,42 @@ mod test { } } + // Regression test for the AND-mask-ignore decision being keyed on the + // embedded BMP's real bit depth rather than the unreliable ICONDIRENTRY + // bit-depth field. The fixture is a genuine 32bpp icon (native alpha = 128) + // with a conflicting AND mask. We rewrite its ICONDIRENTRY `wBitCount` field + // to "unspecified" (0) — still a perfectly valid 32bpp icon — which makes the + // directory entry resolve to a guessed *low* bit depth (from `bColorCount`). + // The native alpha must still be preserved, because the embedded BMP is truly + // 32bpp. Follow-up to #3056. + #[test] + fn bmp_32bpp_unspecified_entry_bpp_and_mask_ignored() { + let mut data = + std::fs::read("tests/images/ico/images/bmp-32bpp-conflicting-and-mask.ico").unwrap(); + + // The ICONDIRENTRY's `wBitCount` field lives at file bytes [12..14]. + // The fixture declares 32 (`20 00`); rewrite it to "unspecified" (`00 00`). + assert_eq!(&data[12..14], &[0x20, 0x00], "fixture layout changed"); + data[12] = 0x00; + data[13] = 0x00; + + let mut decoder = IcoDecoder::new(std::io::Cursor::new(&data)).unwrap(); + let layout = decoder.prepare_image().unwrap(); + let mut buf = vec![0u8; layout.total_bytes() as usize]; + decoder.read_image(&mut buf).unwrap(); + + // Every pixel should still have alpha=128 (the native alpha from the BMP). + // If the AND mask were incorrectly applied because the entry bpp resolved + // to <32, alpha would be 0. + for (i, pixel) in buf.chunks_exact(4).enumerate() { + assert_eq!( + pixel[3], 128, + "pixel {i}: expected alpha=128, got {}", + pixel[3] + ); + } + } + #[test] fn format_error_ico_strict_vs_lenient() { let data = std::fs::read("tests/images/ico/images/lenient-bpp.ico").unwrap();