From 2105f89a68f04335c718ab91438b9bba70743cc3 Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Mon, 5 Oct 2026 09:03:31 +0200 Subject: [PATCH 1/2] fix: honor PDF color component ranges --- CHANGELOG.md | 3 + src/odr/internal/pdf/AGENTS.md | 5 +- src/odr/internal/pdf/pdf_color.cpp | 118 +++++++++++++++++++++------- src/odr/internal/pdf/pdf_color.hpp | 26 +++--- src/odr/internal/pdf/pdf_image.cpp | 6 +- test/src/internal/pdf/pdf_color.cpp | 58 ++++++++++++++ test/src/internal/pdf/pdf_image.cpp | 10 +++ 7 files changed, 181 insertions(+), 45 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index bdf4861d4..9e3f944a8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,9 @@ The release run heads these entries with the version and opens a fresh ## Unreleased +- PDF Lab and ICC colors, indexed palettes and image samples honor component + ranges. Invalid Lab white points and malformed range arrays are rejected. + - JBIG2 decoding checks integer arithmetic, clips off-page regions and bounds symbol allocation and loops. Unsupported intermediate regions are rejected instead of painted onto the page. diff --git a/src/odr/internal/pdf/AGENTS.md b/src/odr/internal/pdf/AGENTS.md index 9077c32a0..1bdeec7d9 100644 --- a/src/odr/internal/pdf/AGENTS.md +++ b/src/odr/internal/pdf/AGENTS.md @@ -181,7 +181,10 @@ above. So each run is raised by one font ascent: 0 to 255 and unresolved base spaces are rejected before image allocation. An ICC alternate that cannot stand in is dropped for the device space `N` names, and a short palette reads its missing bytes as 0. -- CMYK is naive (no ICC) and overprint is ignored. CIE, ICCBased, Indexed, +- Lab and ICC component ranges are shared by direct colors, Indexed palette + scaling and default image decoding (Table 90). ICC still uses its alternate; + profiles are not evaluated. CMYK uses the Adobe polynomial approximation, + and overprint is ignored. CIE, ICCBased, Indexed, Separation, DeviceN and Lab resolve to RGB at emission by sampling the tint `/Function` (types 0, 2, 3, 4). diff --git a/src/odr/internal/pdf/pdf_color.cpp b/src/odr/internal/pdf/pdf_color.cpp index 84aa0cde0..8b895451e 100644 --- a/src/odr/internal/pdf/pdf_color.cpp +++ b/src/odr/internal/pdf/pdf_color.cpp @@ -12,7 +12,9 @@ namespace odr::internal::pdf { namespace { -double clamp01(const double v) { return std::clamp(v, 0.0, 1.0); } +double clamp01(const double v) { + return std::isnan(v) ? 0.0 : std::clamp(v, 0.0, 1.0); +} double linear_to_srgb(const double c) { return util::color::linear_to_srgb(clamp01(c)); @@ -48,14 +50,33 @@ std::shared_ptr device_space(const ColorSpaceKind kind, return def; } -std::vector read_numbers(const Object &object) { - std::vector result; - if (object.is_array()) { - for (const Object &item : object.as_array()) { - result.push_back(item.as_real()); +bool read_numbers(const Object &object, const std::span values, + const ColorSpaceContext &context) { + const Object resolved = context.resolve(object); + if (!resolved.is_array() || resolved.as_array().size() != values.size()) { + return false; + } + for (std::size_t i = 0; i < values.size(); ++i) { + const Object item = context.resolve(resolved.as_array()[i]); + if (!item.is_real() || !std::isfinite(item.as_real())) { + return false; + } + values[i] = item.as_real(); + } + return true; +} + +bool read_ranges(const Object &object, const std::span values, + const ColorSpaceContext &context) { + if (!read_numbers(object, values, context)) { + return false; + } + for (std::size_t i = 0; i < values.size(); i += 2) { + if (values[i] > values[i + 1]) { + return false; } } - return result; + return true; } /// Resolve a colour-space name to a device/pattern space, or a resource space @@ -83,29 +104,49 @@ std::shared_ptr space_from_name(const std::string &name, } // namespace +std::array +ColorSpaceDef::component_range(const std::size_t index) const { + if (kind == ColorSpaceKind::lab && index < 3) { + return index == 0 ? std::array{0, 100} + : std::array{lab_range[2 * index - 2], + lab_range[2 * index - 1]}; + } + if (kind == ColorSpaceKind::icc_based && index < 4) { + return {icc_range[2 * index], icc_range[2 * index + 1]}; + } + return {0, 1}; +} + std::array ColorSpaceDef::to_rgb(const std::span c) const { const auto at = [&](const std::size_t i) { - return i < c.size() ? c[i] : 0.0; + const auto [minimum, maximum] = component_range(i); + const double value = i < c.size() ? c[i] : 0.0; + return kind == ColorSpaceKind::indexed + ? value + : std::clamp(std::isnan(value) ? 0.0 : value, minimum, maximum); }; switch (kind) { case ColorSpaceKind::device_gray: case ColorSpaceKind::cal_gray: { - const double g = clamp01(at(0)); + const double g = at(0); return {g, g, g}; } case ColorSpaceKind::device_rgb: case ColorSpaceKind::cal_rgb: - return {clamp01(at(0)), clamp01(at(1)), clamp01(at(2))}; + return {at(0), at(1), at(2)}; case ColorSpaceKind::device_cmyk: return cmyk_to_rgb(at(0), at(1), at(2), at(3)); case ColorSpaceKind::lab: return lab_to_rgb(at(0), at(1), at(2), white_point); case ColorSpaceKind::icc_based: - // No ICC engine: defer to the alternate, else pick a device space by the - // component count (ISO 32000-1 8.6.5.5). + if (components != 1 && components != 3 && components != 4) { + return {0, 0, 0}; + } if (alternate != nullptr) { - return alternate->to_rgb(c); + const std::array clipped{at(0), at(1), at(2), at(3)}; + return alternate->to_rgb( + std::span(clipped).first(static_cast(components))); } if (components == 1) { const double g = clamp01(at(0)); @@ -129,9 +170,13 @@ ColorSpaceDef::to_rgb(const std::span c) const { std::vector base_components(static_cast(n), 0.0); for (std::int32_t j = 0; j < n; ++j) { const std::size_t k = offset + static_cast(j); + const auto [minimum, maximum] = + base->component_range(static_cast(j)); + const double fraction = k < lookup.size() + ? static_cast(lookup[k]) / 255.0 + : 0.0; base_components[static_cast(j)] = - k < lookup.size() ? static_cast(lookup[k]) / 255.0 - : 0.0; + std::lerp(minimum, maximum, fraction); } return base->to_rgb(base_components); } @@ -174,6 +219,15 @@ std::vector ColorSpaceDef::initial_components() const { case ColorSpaceKind::device_n: // Initial tint is full colorant (ISO 32000-1 8.6.3). return std::vector(static_cast(components), 1.0); + case ColorSpaceKind::lab: + case ColorSpaceKind::icc_based: { + std::vector result(static_cast(components)); + for (std::size_t i = 0; i < result.size(); ++i) { + const auto [minimum, maximum] = component_range(i); + result[i] = std::clamp(0.0, minimum, maximum); + } + return result; + } case ColorSpaceKind::device_cmyk: // Initial DeviceCMYK colour is black, i.e. {0, 0, 0, 1} (ISO // 32000-1 8.6.3). @@ -260,6 +314,13 @@ pdf::parse_color_space(const Object &object, const ColorSpaceContext &context, return nullptr; } def->components = static_cast(count.as_integer()); + if (dict.has_value("Range") && + !read_ranges(dict.get("Range"), + std::span(def->icc_range) + .first(2 * static_cast(def->components)), + context)) { + return nullptr; + } if (dict.has_value("Alternate")) { def->alternate = parse_color_space(dict.get("Alternate"), context, depth + 1); @@ -282,19 +343,20 @@ pdf::parse_color_space(const Object &object, const ColorSpaceContext &context, auto def = std::make_shared(); def->kind = ColorSpaceKind::lab; def->components = 3; - if (array.size() >= 2) { - const Object params = context.resolve(array[1]); - if (params.is_dictionary()) { - const Dictionary &dict = params.as_dictionary(); - const std::vector wp = read_numbers(dict.get("WhitePoint")); - if (wp.size() == 3) { - def->white_point = {wp[0], wp[1], wp[2]}; - } - const std::vector range = read_numbers(dict.get("Range")); - if (range.size() == 4) { - def->lab_range = {range[0], range[1], range[2], range[3]}; - } - } + if (array.size() < 2) { + return nullptr; + } + const Object params = context.resolve(array[1]); + if (!params.is_dictionary()) { + return nullptr; + } + const Dictionary &dict = params.as_dictionary(); + if (!read_numbers(dict.get("WhitePoint"), def->white_point, context) || + def->white_point[0] <= 0 || def->white_point[1] != 1 || + def->white_point[2] <= 0 || + (dict.has_value("Range") && + !read_ranges(dict.get("Range"), def->lab_range, context))) { + return nullptr; } return def; } diff --git a/src/odr/internal/pdf/pdf_color.hpp b/src/odr/internal/pdf/pdf_color.hpp index 53436e6dd..64c9d2b17 100644 --- a/src/odr/internal/pdf/pdf_color.hpp +++ b/src/odr/internal/pdf/pdf_color.hpp @@ -3,6 +3,7 @@ #include #include +#include #include #include #include @@ -14,9 +15,7 @@ namespace odr::internal::pdf { class Object; -/// The colour-space families PDF content can select (ISO 32000-1 8.6). The -/// device families are also reachable through the dedicated operators -/// (`g`/`rg`/`k`); the rest arrive via `/ColorSpace` resources and `cs`/`scn`. +/// PDF colour-space families (ISO 32000-1 8.6). enum class ColorSpaceKind { device_gray, device_rgb, @@ -32,10 +31,8 @@ enum class ColorSpaceKind { unknown, }; -/// A resolved colour space: enough to convert a tuple of component values to -/// sRGB at emit time (ISO 32000-1 8.6.4 / 8.6.5 / 8.6.6). Non-device spaces are -/// approximated — ICC profiles by their alternate or component count, Cal* as -/// device, overprint ignored. +/// Resolved colour space for sRGB conversion (ISO 32000-1 8.6). +/// ICC uses an alternate; Cal* uses the corresponding device approximation. struct ColorSpaceDef { ColorSpaceKind kind{ColorSpaceKind::unknown}; /// Number of input components a colour in this space carries. @@ -44,6 +41,10 @@ struct ColorSpaceDef { // Lab (8.6.5.4): the white point and the a*/b* component ranges. std::array white_point{0.9505, 1.0, 1.089}; std::array lab_range{-100, 100, -100, 100}; + std::array icc_range{0, 1, 0, 1, 0, 1, 0, 1}; + + /// Valid component range, also used by Indexed palettes and image decoding. + [[nodiscard]] std::array component_range(std::size_t index) const; // Indexed (8.6.6.3): the base space, the packed palette and the max index. std::shared_ptr base; @@ -64,10 +65,8 @@ struct ColorSpaceDef { [[nodiscard]] std::vector initial_components() const; }; -/// How `parse_color_space` reaches indirect data: `resolve` dereferences, -/// `load_stream` decodes a stream's bytes (ICC profiles, an Indexed palette -/// given as a stream), and `named` looks up a colour space referenced by name -/// (a base space, the `/ColorSpace` resource table), forwarding its depth. +/// Resolve indirect objects, decode stream bytes and look up resource names. +/// Named lookups must forward the recursion depth. struct ColorSpaceContext { std::function resolve; std::function load_stream; @@ -80,9 +79,8 @@ struct ColorSpaceContext { /// transform. The naive `(1-c)(1-k)` reads pure cyan as `#00ffff`. std::array cmyk_to_rgb(double c, double m, double y, double k); -/// Build a colour space from its PDF object — a name (`/DeviceRGB`, …) or an -/// array (`[/ICCBased 5 0 R]`, `[/Separation …]`, …). Returns `nullptr` for an -/// unsupported, malformed or excessively nested definition. +/// Parse a colour-space name or array; return null for unsupported, malformed +/// or excessively nested definitions. std::shared_ptr parse_color_space(const Object &object, const ColorSpaceContext &context, std::uint32_t depth = 0); diff --git a/src/odr/internal/pdf/pdf_image.cpp b/src/odr/internal/pdf/pdf_image.cpp index 2b520e329..e1ec34389 100644 --- a/src/odr/internal/pdf/pdf_image.cpp +++ b/src/odr/internal/pdf/pdf_image.cpp @@ -180,14 +180,16 @@ std::string pdf::encode_image_png(const std::string &samples, if (decode.size() >= 2 * (k + 1)) { const double d_min = decode[2 * k]; const double d_max = decode[2 * k + 1]; - return d_min + sample * (d_max - d_min) / max_sample; + return std::lerp(d_min, d_max, static_cast(sample) / max_sample); } if (indexed) { // Default Indexed /Decode is [0, 2^bpc-1]: the sample is the palette // index, which `to_rgb` looks up directly (8.6.6.3). return sample; } - return static_cast(sample) / max_sample; + const auto [minimum, maximum] = color_space.component_range(k); + return std::lerp(minimum, maximum, + static_cast(sample) / max_sample); }; if (bits_per_component == 1 && components == 1 && !has_alpha) { diff --git a/test/src/internal/pdf/pdf_color.cpp b/test/src/internal/pdf/pdf_color.cpp index 8176b7a9f..a6d038398 100644 --- a/test/src/internal/pdf/pdf_color.cpp +++ b/test/src/internal/pdf/pdf_color.cpp @@ -262,3 +262,61 @@ TEST(PdfColor, a_mismatched_icc_alternate_is_ignored) { EXPECT_EQ(def->alternate, nullptr); EXPECT_EQ(to_rgb(*def, {1, 0, 0}), (std::array{1, 0, 0})); } + +TEST(PdfColor, lab_parameters_and_palette_ranges) { + Dictionary params; + params["WhitePoint"] = reals({0.9505, 1, 1.089}); + params["Range"] = reals({-128, 127, -128, 127}); + const Object lab(Array({Object(Name{"Lab"}), Object(params)})); + const Object indexed( + Array({Object(Name{"Indexed"}), lab, Object(Integer{0}), + Object(StandardString(std::string("\xff\x80\x80", 3)))})); + const auto def = parse_color_space(indexed, context()); + ASSERT_NE(def, nullptr); + const auto white = to_rgb(*def, {0}); + for (const double channel : white) { + EXPECT_NEAR(channel, 1.0, 0.001); + } + EXPECT_EQ(to_rgb(*def->base, {200, 300, -300}), + to_rgb(*def->base, {100, 127, -128})); + for (const Object &point : + {Object{}, reals({0, 1, 1}), reals({1, 2, 1}), reals({1, 1, -1})}) { + params["WhitePoint"] = point; + EXPECT_EQ( + parse_color_space(Object(Array({Object(Name{"Lab"}), Object(params)})), + context()), + nullptr); + } + params["WhitePoint"] = reals({1, 1, 1}); + params["Range"] = reals({10, -10, -20, 20}); + EXPECT_EQ( + parse_color_space(Object(Array({Object(Name{"Lab"}), Object(params)})), + context()), + nullptr); +} + +TEST(PdfColor, icc_component_ranges_clip_and_scale_palette_values) { + Dictionary profile; + profile["N"] = Object(Integer{3}); + profile["Range"] = reals({0.2, 0.8, 0.1, 0.9, 0.3, 0.7}); + const Object icc(Array({Object(Name{"ICCBased"}), Object(profile)})); + const auto def = parse_color_space(icc, context()); + ASSERT_NE(def, nullptr); + EXPECT_EQ(to_rgb(*def, {-1, 2, -1}), (std::array{0.2, 0.9, 0.3})); + const auto indexed = parse_color_space( + Object(Array({Object(Name{"Indexed"}), icc, Object(Integer{0}), + Object(StandardString(std::string("\0\xff\0", 3)))})), + context()); + ASSERT_NE(indexed, nullptr); + EXPECT_EQ(to_rgb(*indexed, {0}), (std::array{0.2, 0.9, 0.3})); + for (const Object &range : + {reals({0, 1}), reals({1, 0, 0, 1, 0, 1}), + reals({0, std::numeric_limits::infinity(), 0, 1, 0, 1}), + Object(Name{"Bad"})}) { + profile["Range"] = range; + EXPECT_EQ(parse_color_space( + Object(Array({Object(Name{"ICCBased"}), Object(profile)})), + context()), + nullptr); + } +} diff --git a/test/src/internal/pdf/pdf_image.cpp b/test/src/internal/pdf/pdf_image.cpp index 0bf5e48a1..bc32bac84 100644 --- a/test/src/internal/pdf/pdf_image.cpp +++ b/test/src/internal/pdf/pdf_image.cpp @@ -242,3 +242,13 @@ TEST(PdfImage, rejects_unrepresentable_output_sizes) { decode_mask_alpha("", 1, 1, 8, {}, false, 65536, 65536).empty()); } } + +TEST(PdfImage, lab_default_decode_uses_the_component_ranges) { + ColorSpaceDef lab; + lab.kind = ColorSpaceKind::lab; + lab.components = 3; + lab.lab_range = {-128, 127, -128, 127}; + const auto png = + decode_png(encode_image_png(bytes({255, 128, 128}), 1, 1, 8, lab, {})); + EXPECT_EQ(png.pixels, bytes({255, 255, 255})); +} From 32d02916ff0845650e419db2858dbae700f40c10 Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Tue, 6 Oct 2026 20:57:16 +0200 Subject: [PATCH 2/2] fix(pdf): keep a Lab or ICC space usable with invalid optional values An invalid ICC Range, an invalid Lab WhitePoint or Range, or a Lab space without parameters rejected the whole colour space. scn then read the three Lab components as RGB. Main used the defaults in all these cases, and they stay valid: the white point and the ranges are only taken when they pass the checks that protect the conversion. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01MxyTMutqSUJRGfxA8CyzMc --- src/odr/internal/pdf/AGENTS.md | 3 ++- src/odr/internal/pdf/pdf_color.cpp | 38 ++++++++++++++++------------- test/src/internal/pdf/pdf_color.cpp | 25 ++++++++++--------- 3 files changed, 36 insertions(+), 30 deletions(-) diff --git a/src/odr/internal/pdf/AGENTS.md b/src/odr/internal/pdf/AGENTS.md index 1bdeec7d9..9fe609f3e 100644 --- a/src/odr/internal/pdf/AGENTS.md +++ b/src/odr/internal/pdf/AGENTS.md @@ -182,7 +182,8 @@ above. So each run is raised by one font ascent: An ICC alternate that cannot stand in is dropped for the device space `N` names, and a short palette reads its missing bytes as 0. - Lab and ICC component ranges are shared by direct colors, Indexed palette - scaling and default image decoding (Table 90). ICC still uses its alternate; + scaling and default image decoding (Table 90). An invalid Lab `WhitePoint` + or `Range`, or an invalid ICC `Range`, keeps the default. ICC still uses its alternate; profiles are not evaluated. CMYK uses the Adobe polynomial approximation, and overprint is ignored. CIE, ICCBased, Indexed, Separation, DeviceN and Lab resolve to RGB at emission by sampling the tint diff --git a/src/odr/internal/pdf/pdf_color.cpp b/src/odr/internal/pdf/pdf_color.cpp index 8b895451e..ceb595748 100644 --- a/src/odr/internal/pdf/pdf_color.cpp +++ b/src/odr/internal/pdf/pdf_color.cpp @@ -314,12 +314,14 @@ pdf::parse_color_space(const Object &object, const ColorSpaceContext &context, return nullptr; } def->components = static_cast(count.as_integer()); - if (dict.has_value("Range") && - !read_ranges(dict.get("Range"), - std::span(def->icc_range) - .first(2 * static_cast(def->components)), - context)) { - return nullptr; + // an invalid optional Range keeps the default 0 to 1 per component + if (std::array range = def->icc_range; + dict.has_value("Range") && + read_ranges(dict.get("Range"), + std::span(range).first( + 2 * static_cast(def->components)), + context)) { + def->icc_range = range; } if (dict.has_value("Alternate")) { def->alternate = @@ -343,20 +345,22 @@ pdf::parse_color_space(const Object &object, const ColorSpaceContext &context, auto def = std::make_shared(); def->kind = ColorSpaceKind::lab; def->components = 3; - if (array.size() < 2) { - return nullptr; - } - const Object params = context.resolve(array[1]); + const Object params = + array.size() >= 2 ? context.resolve(array[1]) : Object{}; if (!params.is_dictionary()) { - return nullptr; + return def; } const Dictionary &dict = params.as_dictionary(); - if (!read_numbers(dict.get("WhitePoint"), def->white_point, context) || - def->white_point[0] <= 0 || def->white_point[1] != 1 || - def->white_point[2] <= 0 || - (dict.has_value("Range") && - !read_ranges(dict.get("Range"), def->lab_range, context))) { - return nullptr; + // an invalid WhitePoint or Range keeps the default, as a missing one does + if (std::array point{}; + read_numbers(dict.get("WhitePoint"), point, context) && point[0] > 0 && + point[1] == 1 && point[2] > 0) { + def->white_point = point; + } + if (std::array range{}; + dict.has_value("Range") && + read_ranges(dict.get("Range"), range, context)) { + def->lab_range = range; } return def; } diff --git a/test/src/internal/pdf/pdf_color.cpp b/test/src/internal/pdf/pdf_color.cpp index a6d038398..1ecaf29bb 100644 --- a/test/src/internal/pdf/pdf_color.cpp +++ b/test/src/internal/pdf/pdf_color.cpp @@ -282,17 +282,17 @@ TEST(PdfColor, lab_parameters_and_palette_ranges) { for (const Object &point : {Object{}, reals({0, 1, 1}), reals({1, 2, 1}), reals({1, 1, -1})}) { params["WhitePoint"] = point; - EXPECT_EQ( - parse_color_space(Object(Array({Object(Name{"Lab"}), Object(params)})), - context()), - nullptr); + const auto fallback = parse_color_space( + Object(Array({Object(Name{"Lab"}), Object(params)})), context()); + ASSERT_NE(fallback, nullptr); + EXPECT_EQ(fallback->white_point, (std::array{0.9505, 1, 1.089})); } params["WhitePoint"] = reals({1, 1, 1}); params["Range"] = reals({10, -10, -20, 20}); - EXPECT_EQ( - parse_color_space(Object(Array({Object(Name{"Lab"}), Object(params)})), - context()), - nullptr); + const auto fallback = parse_color_space( + Object(Array({Object(Name{"Lab"}), Object(params)})), context()); + ASSERT_NE(fallback, nullptr); + EXPECT_EQ(fallback->lab_range, (std::array{-100, 100, -100, 100})); } TEST(PdfColor, icc_component_ranges_clip_and_scale_palette_values) { @@ -314,9 +314,10 @@ TEST(PdfColor, icc_component_ranges_clip_and_scale_palette_values) { reals({0, std::numeric_limits::infinity(), 0, 1, 0, 1}), Object(Name{"Bad"})}) { profile["Range"] = range; - EXPECT_EQ(parse_color_space( - Object(Array({Object(Name{"ICCBased"}), Object(profile)})), - context()), - nullptr); + const auto fallback = parse_color_space( + Object(Array({Object(Name{"ICCBased"}), Object(profile)})), context()); + ASSERT_NE(fallback, nullptr); + EXPECT_EQ(fallback->icc_range, + (std::array{0, 1, 0, 1, 0, 1, 0, 1})); } }