From c6a4588fb347d608a374cfe90d138985223b279d Mon Sep 17 00:00:00 2001 From: dualfroz Date: Fri, 4 Sep 2026 09:35:35 +0200 Subject: [PATCH] fix(validation): allow image height up to spec maximum of 400 Image height was validated against 0..=144, the same bound as width, but the RSS 2.0 specification allows heights up to 400 (width max is 144). Spec-valid images with a height between 145 and 400 were wrongly reported as invalid. Validate height against 0..=400 and fix the error message. --- PR.md | 88 +++++++++++++++++++++++++++++++++++++++++++++ src/validation.rs | 4 +-- tests/validation.rs | 42 ++++++++++++++++++++++ 3 files changed, 132 insertions(+), 2 deletions(-) create mode 100644 PR.md create mode 100644 tests/validation.rs diff --git a/PR.md b/PR.md new file mode 100644 index 0000000..d91176b --- /dev/null +++ b/PR.md @@ -0,0 +1,88 @@ +# Fix image height validation to allow the spec maximum of 400 + +## Problem + +`Channel`/`Image` validation rejects images whose `` is greater than +144, even though the RSS 2.0 specification allows heights up to 400. A +perfectly spec-compliant image is reported as invalid: + +```rust +use rss::validation::Validate; +use rss::ImageBuilder; + +let image = ImageBuilder::default() + .url("https://example.com/image.png".to_string()) + .link("https://example.com/".to_string()) + .title("title".to_string()) + .height("200".to_string()) // valid per the RSS spec + .build(); + +assert!(image.validate().is_err()); // <- wrongly errors today +``` + +## Root cause + +`src/validation.rs`, in `impl Validate for Image`, validated `height` against +the same range used for `width`: + +```rust +let height = height.parse::()?; +validate!( + (0..=144).contains(&height), + "Image height is not between 0 and 144" +); +``` + +The RSS 2.0 specification (, +`` sub-element) states: + +- width: "Maximum value for width is 144, default value is 88." +- height: "Maximum value for height is 400, default value is 31." + +So 144 is correct for `width`, but `height` must allow up to 400. The bound +(and the error message) were copied from the width check. + +## Fix + +Validate `height` against `0..=400` and correct the error message: + +```rust +let height = height.parse::()?; +validate!( + (0..=400).contains(&height), + "Image height is not between 0 and 400" +); +``` + +`width` is unchanged and keeps its specification maximum of 144. + +## Test + +Added `tests/validation.rs` (gated on the `validation` feature): + +- `image_height_allows_up_to_400`: heights 31, 200 and 400 validate OK. +- `image_height_above_400_is_invalid`: height 401 is rejected. +- `image_width_max_is_still_144`: width 144 is OK, 145 is rejected. + +## Gate + +- `cargo test --all-features` -> ok (all suites pass, exit 0) +- `cargo build --all-features` -> ok (exit 0) +- `cargo fmt --check` -> ok (exit 0) + +## Counterfactual + +Reverting the bound back to `0..=144` makes the new test fail: + +``` +thread 'image_height_allows_up_to_400' panicked at tests/validation.rs:28:5: +assertion failed: image_with_height("200").validate().is_ok() +``` + +## Flags for the account owner + +`cargo clippy --all-targets --all-features -- -D warnings` currently fails, but +this is pre-existing and unrelated to this change: on an unmodified checkout the +same lint (`clippy::get_first`, i.e. "accessing first element with `.get(0)`", +emitted by a newer Clippy than the tests were written against) already produces +these errors in `tests/read.rs`. The files touched here are clippy-clean. diff --git a/src/validation.rs b/src/validation.rs index 2f19ab0..2ccf7dc 100644 --- a/src/validation.rs +++ b/src/validation.rs @@ -222,8 +222,8 @@ impl Validate for Image { if let Some(height) = self.height() { let height = height.parse::()?; validate!( - (0..=144).contains(&height), - "Image height is not between 0 and 144" + (0..=400).contains(&height), + "Image height is not between 0 and 400" ); } diff --git a/tests/validation.rs b/tests/validation.rs new file mode 100644 index 0000000..b68e360 --- /dev/null +++ b/tests/validation.rs @@ -0,0 +1,42 @@ +#![cfg(feature = "validation")] + +use rss::validation::Validate; +use rss::ImageBuilder; + +fn image_with_height(height: &str) -> rss::Image { + ImageBuilder::default() + .url("https://example.com/image.png".to_string()) + .link("https://example.com/".to_string()) + .title("title".to_string()) + .height(height.to_string()) + .build() +} + +fn image_with_width(width: &str) -> rss::Image { + ImageBuilder::default() + .url("https://example.com/image.png".to_string()) + .link("https://example.com/".to_string()) + .title("title".to_string()) + .width(width.to_string()) + .build() +} + +#[test] +fn image_height_allows_up_to_400() { + // The RSS 2.0 specification sets the maximum image height to 400. + assert!(image_with_height("31").validate().is_ok()); + assert!(image_with_height("200").validate().is_ok()); + assert!(image_with_height("400").validate().is_ok()); +} + +#[test] +fn image_height_above_400_is_invalid() { + assert!(image_with_height("401").validate().is_err()); +} + +#[test] +fn image_width_max_is_still_144() { + // Width keeps its specification maximum of 144. + assert!(image_with_width("144").validate().is_ok()); + assert!(image_with_width("145").validate().is_err()); +}