From 0160f737e2bc4337f0b33a35650918da0e492e9f Mon Sep 17 00:00:00 2001 From: Nico Weber Date: Tue, 20 Feb 2024 10:22:44 -0500 Subject: [PATCH] LibGfx/ICC+icc: Be lenient about invalid profile creation datetimes Before, we used to reject profiles where the creation datetime was invalid per spec. But invalid dates happen in practice (most commonly, all fields set to 0). They don't affect profile conversion at all, so be lenient about this, in exchange for slightly more wordy code in the places that want to show the creation datetime. Fixes a crash rendering page 2 of https://fredrikbk.com/publications/copy-and-patch.pdf --- .../Libraries/LibGfx/ICC/BinaryWriter.cpp | 17 ++- Userland/Libraries/LibGfx/ICC/Profile.cpp | 117 ++++++++++++------ Userland/Libraries/LibGfx/ICC/Profile.h | 20 ++- .../LibGfx/ICC/WellKnownProfiles.cpp | 2 +- Userland/Utilities/icc.cpp | 11 +- 5 files changed, 114 insertions(+), 53 deletions(-) diff --git a/Userland/Libraries/LibGfx/ICC/BinaryWriter.cpp b/Userland/Libraries/LibGfx/ICC/BinaryWriter.cpp index d054ecb190..3912c3bc67 100644 --- a/Userland/Libraries/LibGfx/ICC/BinaryWriter.cpp +++ b/Userland/Libraries/LibGfx/ICC/BinaryWriter.cpp @@ -673,16 +673,13 @@ static ErrorOr encode_header(ByteBuffer& bytes, Profile const& profile) raw_header.data_color_space = profile.data_color_space(); raw_header.profile_connection_space = profile.connection_space(); - time_t profile_timestamp = profile.creation_timestamp(); - struct tm tm; - if (!gmtime_r(&profile_timestamp, &tm)) - return Error::from_errno(errno); - raw_header.profile_creation_time.year = tm.tm_year + 1900; - raw_header.profile_creation_time.month = tm.tm_mon + 1; - raw_header.profile_creation_time.day = tm.tm_mday; - raw_header.profile_creation_time.hours = tm.tm_hour; - raw_header.profile_creation_time.minutes = tm.tm_min; - raw_header.profile_creation_time.seconds = tm.tm_sec; + DateTime profile_timestamp = profile.creation_timestamp(); + raw_header.profile_creation_time.year = profile_timestamp.year; + raw_header.profile_creation_time.month = profile_timestamp.month; + raw_header.profile_creation_time.day = profile_timestamp.day; + raw_header.profile_creation_time.hours = profile_timestamp.hours; + raw_header.profile_creation_time.minutes = profile_timestamp.minutes; + raw_header.profile_creation_time.seconds = profile_timestamp.seconds; raw_header.profile_file_signature = ProfileFileSignature; raw_header.primary_platform = profile.primary_platform().value_or(PrimaryPlatform { 0 }); diff --git a/Userland/Libraries/LibGfx/ICC/Profile.cpp b/Userland/Libraries/LibGfx/ICC/Profile.cpp index f4d4945859..6b693480c8 100644 --- a/Userland/Libraries/LibGfx/ICC/Profile.cpp +++ b/Userland/Libraries/LibGfx/ICC/Profile.cpp @@ -21,45 +21,16 @@ namespace Gfx::ICC { namespace { -ErrorOr parse_date_time_number(DateTimeNumber const& date_time) +ErrorOr parse_date_time_number(DateTimeNumber const& date_time) { - // ICC V4, 4.2 dateTimeNumber - - // "Number of the month (1 to 12)" - if (date_time.month < 1 || date_time.month > 12) - return Error::from_string_literal("ICC::Profile: dateTimeNumber month out of bounds"); - - // "Number of the day of the month (1 to 31)" - if (date_time.day < 1 || date_time.day > 31) - return Error::from_string_literal("ICC::Profile: dateTimeNumber day out of bounds"); - - // "Number of hours (0 to 23)" - if (date_time.hours > 23) - return Error::from_string_literal("ICC::Profile: dateTimeNumber hours out of bounds"); - - // "Number of minutes (0 to 59)" - if (date_time.minutes > 59) - return Error::from_string_literal("ICC::Profile: dateTimeNumber minutes out of bounds"); - - // "Number of seconds (0 to 59)" - // ICC profiles apparently can't be created during leap seconds (seconds would be 60 there, but the spec doesn't allow that). - if (date_time.seconds > 59) - return Error::from_string_literal("ICC::Profile: dateTimeNumber seconds out of bounds"); - - struct tm tm = {}; - tm.tm_year = date_time.year - 1900; - tm.tm_mon = date_time.month - 1; - tm.tm_mday = date_time.day; - tm.tm_hour = date_time.hours; - tm.tm_min = date_time.minutes; - tm.tm_sec = date_time.seconds; - // timegm() doesn't read tm.tm_isdst, tm.tm_wday, and tm.tm_yday, no need to fill them in. - - time_t timestamp = timegm(&tm); - if (timestamp == -1) - return Error::from_string_literal("ICC::Profile: dateTimeNumber not representable as timestamp"); - - return timestamp; + return DateTime { + .year = date_time.year, + .month = date_time.month, + .day = date_time.day, + .hours = date_time.hours, + .minutes = date_time.minutes, + .seconds = date_time.seconds, + }; } ErrorOr parse_size(ICCHeader const& header, ReadonlyBytes icc_bytes) @@ -183,7 +154,7 @@ ErrorOr parse_connection_space(ICCHeader const& header) return space; } -ErrorOr parse_creation_date_time(ICCHeader const& header) +ErrorOr parse_creation_date_time(ICCHeader const& header) { // ICC v4, 7.2.8 Date and time field return parse_date_time_number(header.profile_creation_time); @@ -358,6 +329,74 @@ DeviceAttributes::DeviceAttributes(u64 bits) { } +static ErrorOr validate_date_time(DateTime const& date_time) +{ + // Returns if a DateTime is valid per ICC V4, 4.2 dateTimeNumber. + // In practice, some profiles contain invalid dates, but we should enforce this for data we write at least. + + // "Number of the month (1 to 12)" + if (date_time.month < 1 || date_time.month > 12) + return Error::from_string_literal("ICC::Profile: dateTimeNumber month out of bounds"); + + // "Number of the day of the month (1 to 31)" + if (date_time.day < 1 || date_time.day > 31) + return Error::from_string_literal("ICC::Profile: dateTimeNumber day out of bounds"); + + // "Number of hours (0 to 23)" + if (date_time.hours > 23) + return Error::from_string_literal("ICC::Profile: dateTimeNumber hours out of bounds"); + + // "Number of minutes (0 to 59)" + if (date_time.minutes > 59) + return Error::from_string_literal("ICC::Profile: dateTimeNumber minutes out of bounds"); + + // "Number of seconds (0 to 59)" + // ICC profiles apparently can't be created during leap seconds (seconds would be 60 there, but the spec doesn't allow that). + if (date_time.seconds > 59) + return Error::from_string_literal("ICC::Profile: dateTimeNumber seconds out of bounds"); + + return {}; +} + +ErrorOr DateTime::to_time_t() const +{ + TRY(validate_date_time(*this)); + + struct tm tm = {}; + tm.tm_year = year - 1900; + tm.tm_mon = month - 1; + tm.tm_mday = day; + tm.tm_hour = hours; + tm.tm_min = minutes; + tm.tm_sec = seconds; + // timegm() doesn't read tm.tm_isdst, tm.tm_wday, and tm.tm_yday, no need to fill them in. + + time_t timestamp = timegm(&tm); + if (timestamp == -1) + return Error::from_string_literal("ICC::Profile: dateTimeNumber not representable as timestamp"); + + return timestamp; +} + +ErrorOr DateTime::from_time_t(time_t timestamp) +{ + struct tm gmt_tm; + if (gmtime_r(×tamp, &gmt_tm) == NULL) + return Error::from_string_literal("ICC::Profile: timestamp not representable as DateTimeNumber"); + + // FIXME: Range-check, using something like `TRY(Checked(x).try_value())`? + DateTime result { + .year = static_cast(gmt_tm.tm_year + 1900), + .month = static_cast(gmt_tm.tm_mon + 1), + .day = static_cast(gmt_tm.tm_mday), + .hours = static_cast(gmt_tm.tm_hour), + .minutes = static_cast(gmt_tm.tm_min), + .seconds = static_cast(gmt_tm.tm_sec), + }; + TRY(validate_date_time(result)); + return result; +} + static ErrorOr read_header(ReadonlyBytes bytes) { if (bytes.size() < sizeof(ICCHeader)) diff --git a/Userland/Libraries/LibGfx/ICC/Profile.h b/Userland/Libraries/LibGfx/ICC/Profile.h index 9178f8d4d9..2c35680176 100644 --- a/Userland/Libraries/LibGfx/ICC/Profile.h +++ b/Userland/Libraries/LibGfx/ICC/Profile.h @@ -130,6 +130,22 @@ private: u64 m_bits = 0; }; +// Time is in UTC. +// Per spec, month is 1-12, day is 1-31, hours is 0-23, minutes 0-59, seconds 0-59 (i.e. no leap seconds). +// But in practice, some profiles have invalid dates, like 0-0-0 0:0:0. +// For valid profiles, the conversion to time_t will succeed. +struct DateTime { + u16 year = 1970; + u16 month = 1; // One-based. + u16 day = 1; // One-based. + u16 hours = 0; + u16 minutes = 0; + u16 seconds = 0; + + ErrorOr to_time_t() const; + static ErrorOr from_time_t(time_t); +}; + struct ProfileHeader { u32 on_disk_size { 0 }; Optional preferred_cmm_type; @@ -137,7 +153,7 @@ struct ProfileHeader { DeviceClass device_class {}; ColorSpace data_color_space {}; ColorSpace connection_space {}; - time_t creation_timestamp { 0 }; + DateTime creation_timestamp; Optional primary_platform {}; Flags flags; Optional device_manufacturer; @@ -219,7 +235,7 @@ public: ColorSpace connection_space() const { return m_header.connection_space; } u32 on_disk_size() const { return m_header.on_disk_size; } - time_t creation_timestamp() const { return m_header.creation_timestamp; } + DateTime creation_timestamp() const { return m_header.creation_timestamp; } Optional primary_platform() const { return m_header.primary_platform; } Flags flags() const { return m_header.flags; } Optional device_manufacturer() const { return m_header.device_manufacturer; } diff --git a/Userland/Libraries/LibGfx/ICC/WellKnownProfiles.cpp b/Userland/Libraries/LibGfx/ICC/WellKnownProfiles.cpp index dfaf6645e0..8022076052 100644 --- a/Userland/Libraries/LibGfx/ICC/WellKnownProfiles.cpp +++ b/Userland/Libraries/LibGfx/ICC/WellKnownProfiles.cpp @@ -18,7 +18,7 @@ static ProfileHeader rgb_header() header.device_class = DeviceClass::DisplayDevice; header.data_color_space = ColorSpace::RGB; header.connection_space = ColorSpace::PCSXYZ; - header.creation_timestamp = time(NULL); + header.creation_timestamp = MUST(DateTime::from_time_t(0)); header.rendering_intent = RenderingIntent::Perceptual; header.pcs_illuminant = XYZ { 0.9642, 1.0, 0.8249 }; return header; diff --git a/Userland/Utilities/icc.cpp b/Userland/Utilities/icc.cpp index 12433624ca..a684cb6e4f 100644 --- a/Userland/Utilities/icc.cpp +++ b/Userland/Utilities/icc.cpp @@ -324,7 +324,16 @@ ErrorOr serenity_main(Main::Arguments arguments) outln(" device class: {}", Gfx::ICC::device_class_name(profile->device_class())); outln(" data color space: {}", Gfx::ICC::data_color_space_name(profile->data_color_space())); outln(" connection space: {}", Gfx::ICC::profile_connection_space_name(profile->connection_space())); - outln("creation date and time: {}", Core::DateTime::from_timestamp(profile->creation_timestamp())); + + if (auto time = profile->creation_timestamp().to_time_t(); !time.is_error()) { + // Print in friendly localtime for valid profiles. + outln("creation date and time: {}", Core::DateTime::from_timestamp(time.release_value())); + } else { + outln("creation date and time: {:04}-{:02}-{:02} {:02}:{:02}:{:02} UTC (invalid)", + profile->creation_timestamp().year, profile->creation_timestamp().month, profile->creation_timestamp().day, + profile->creation_timestamp().hours, profile->creation_timestamp().minutes, profile->creation_timestamp().seconds); + } + out_optional(" primary platform", profile->primary_platform().map([](auto platform) { return primary_platform_name(platform); })); auto flags = profile->flags();