From d368658d7e79fd44cbee941318fed6799c6adf4b Mon Sep 17 00:00:00 2001 From: axunonb Date: Thu, 6 Mar 2025 09:47:01 +0100 Subject: [PATCH 1/2] Update `PRODID` and `VERSION` property handling Issue: `Calendar` has setters for `ProductId` and `Version` which are overridden with fixed values when serializing. - When creating a new `Calendar` instance, `ProductId` and `Version` contain default values - When Deserializing an iCalendar, `ProductId` and `Version` will be taken from the input - `ProductId` and `Version` can be overridden by user code. An attempt to set as an empty string will throw. - Update the default `PRODID` property `LibraryMetadata.ProdId` to include the ical.net assembly version. Example: "PRODID:-//github.com/ical-org/ical.net//NONSGML ical.net 5.4.3//EN" - Modified `CalendarSerializer.SerializeToString` so that the `ProdId` or `Version` set by users do not get overridden - Add an xmldoc description about the purpose of `ProdId` and `Version`, and about the risks when modified - Add unit tests Resolves #531 --- Ical.Net.Tests/DeserializationTests.cs | 24 ++++++++++ Ical.Net.Tests/SerializationTests.cs | 38 +++++++++++++--- Ical.Net/Calendar.cs | 46 ++++++++++++++++++-- Ical.Net/Constants.cs | 34 +++++++++++++-- Ical.Net/Serialization/CalendarSerializer.cs | 16 +------ 5 files changed, 128 insertions(+), 30 deletions(-) diff --git a/Ical.Net.Tests/DeserializationTests.cs b/Ical.Net.Tests/DeserializationTests.cs index ce4f885e..8a326f1f 100644 --- a/Ical.Net.Tests/DeserializationTests.cs +++ b/Ical.Net.Tests/DeserializationTests.cs @@ -588,4 +588,28 @@ public void KeepApartDtEndAndDuration_Tests(bool useDtEnd) Assert.That(calendar.Events.Single().Duration != null, Is.EqualTo(!useDtEnd)); }); } + + [Test] + public void CalendarWithMissingProdIdOrVersion_ShouldLeavePropertiesInvalid() + { + var ics = """ + BEGIN:VCALENDAR + BEGIN:VEVENT + DTSTART:20070406T230000Z + DTEND:20070407T010000Z + END:VEVENT + END:VCALENDAR + """; + + var calendar = Calendar.Load(ics); + var deserialized = new CalendarSerializer(calendar).SerializeToString(); + + Assert.Multiple(() => + { + Assert.That(calendar.ProductId, Is.EqualTo(null)); + Assert.That(calendar.Version, Is.EqualTo(null)); + // The serialized calendar should not contain the PRODID or VERSION properties, which are required + Assert.That(deserialized, Does.Not.Contain("PRODID:").And.Not.Contains("VERSION:")); + }); + } } diff --git a/Ical.Net.Tests/SerializationTests.cs b/Ical.Net.Tests/SerializationTests.cs index 9b4c0975..537a2a25 100644 --- a/Ical.Net.Tests/SerializationTests.cs +++ b/Ical.Net.Tests/SerializationTests.cs @@ -525,20 +525,44 @@ public void TestRRuleUntilSerialization() Assert.That(!until.EndsWith("Z"), Is.True); } - [Test(Description = "PRODID and VERSION should use ical.net values instead of preserving deserialized values")] - public void LibraryMetadataTests() + [Test] + public void ProductId_and_Version_CanBeChanged() { var c = new Calendar { ProductId = "FOO", - Version = "BAR" + Version = "BAR", }; + var serialized = new CalendarSerializer().SerializeToString(c); - var expectedProdid = $"PRODID:{LibraryMetadata.ProdId}"; - Assert.That(serialized.Contains(expectedProdid, StringComparison.Ordinal), Is.True); + + Assert.Multiple(() => + { + Assert.That(serialized, Does.Contain($"PRODID:{c.ProductId}")); + Assert.That(serialized, Does.Contain($"VERSION:{c.Version}")); + }); + } + + [Test] + public void ProductId_and_Version_CannotBeSetAsEmpty() + { + var c = new Calendar(); + Assert.Multiple(() => + { + Assert.That(() => c.ProductId = string.Empty, Throws.ArgumentException); + Assert.That(() => c.Version = string.Empty, Throws.ArgumentException); + }); + } - var expectedVersion = $"VERSION:{LibraryMetadata.Version}"; - Assert.That(serialized.Contains(expectedVersion, StringComparison.Ordinal), Is.True); + [Test] + public void ProductId_and_Version_HaveDefaultValues() + { + var c = new Calendar(); + Assert.Multiple(() => + { + Assert.That(c.ProductId, Is.EqualTo(LibraryMetadata.ProdId)); + Assert.That(c.Version, Is.EqualTo(LibraryMetadata.Version)); + }); } [Test] diff --git a/Ical.Net/Calendar.cs b/Ical.Net/Calendar.cs index 77222ece..28d19c00 100644 --- a/Ical.Net/Calendar.cs +++ b/Ical.Net/Calendar.cs @@ -60,12 +60,16 @@ public static IList Load(string ical) /// public Calendar() { - Name = Components.Calendar; + // Note: ProductId and Version Property values will be empty before _deserialization_ + ProductId = LibraryMetadata.ProdId; + Version = LibraryMetadata.Version; + Initialize(); } private void Initialize() { + Name = Components.Calendar; _mUniqueComponents = new UniqueComponentListProxy(Children); _mEvents = new UniqueComponentListProxy(Children); _mTodos = new UniqueComponentListProxy(Children); @@ -143,20 +147,54 @@ public override int GetHashCode() public virtual ICalendarObjectList TimeZones => _mTimeZones; /// - /// A collection of components in the iCalendar. + /// A collection of components in the iCalendar. /// public virtual IUniqueComponentList Todos => _mTodos; + /// + /// Gets or sets the version of the iCalendar definition. The default is + /// as per RFC 5545 Section 3.7.4 and must be specified. + /// + /// It specifies the identifier corresponding to the highest version number of the iCalendar specification + /// that is required in order to interpret the iCalendar object. + /// + /// Do not change unless you are sure about the consequences. + /// + /// The default value does not apply to deserialized objects. + /// public virtual string Version { get => Properties.Get("VERSION"); - set => Properties.Set("VERSION", value); + set + { + if (string.IsNullOrEmpty(value)) + { + throw new ArgumentException("Version to set must not be null or empty"); + } + Properties.Set("VERSION", value); + } } + /// + /// Gets or sets the product ID of the iCalendar, which typically contains the name of the software + /// that created the iCalendar. The default is . + /// + /// Be careful when setting a custom value, as it is free-form text that must conform to the iCalendar specification + /// (RFC 5545 Section 3.7.3). The product ID must be specified. + /// + /// The default value does not apply to deserialized objects. + /// public virtual string ProductId { get => Properties.Get("PRODID"); - set => Properties.Set("PRODID", value); + set + { + if (string.IsNullOrEmpty(value)) + { + throw new ArgumentException("Product ID to set must not be null or empty"); + } + Properties.Set("PRODID", value); + } } public virtual string Scale diff --git a/Ical.Net/Constants.cs b/Ical.Net/Constants.cs index 10db3327..a8b78cf1 100644 --- a/Ical.Net/Constants.cs +++ b/Ical.Net/Constants.cs @@ -3,7 +3,9 @@ // Licensed under the MIT license. // +#nullable enable using System; +using System.Diagnostics; namespace Ical.Net; @@ -124,7 +126,7 @@ public class SerializationConstants } /// -/// Status codes available to an item +/// Status codes available to an item /// public static class EventStatus { @@ -137,7 +139,7 @@ public static class EventStatus } /// -/// Status codes available to a item. +/// Status codes available to a item. /// public static class TodoStatus { @@ -152,7 +154,7 @@ public static class TodoStatus } /// -/// Status codes available to a entry. +/// Status codes available to a entry. /// public static class JournalStatus { @@ -235,8 +237,32 @@ public static class TransparencyType public static class LibraryMetadata { + private static readonly string _assemblyVersion = GetAssemblyVersion(); + + /// + /// The VERSION property for iCalendar objects generated by this library (RFC 5545 Section 3.7.4), + /// unless overridden by user code. + /// public const string Version = "2.0"; - public static readonly string ProdId = "-//github.com/ical-org/ical.net//NONSGML ical.net 4.0//EN"; + + /// + /// The default PRODID property for iCalendar objects generated by this library (RFC 5545 Section 3.7.3), + /// unless overridden by user code. + /// + /// The text between the double slashes represents the organization or software that created the iCalendar object. + /// + /// + public static string ProdId => $"-//github.com/ical-org/ical.net//NONSGML ical.net {_assemblyVersion}//EN"; + + private static string GetAssemblyVersion() + { + var assembly = typeof(LibraryMetadata).Assembly; + var fileVersionInfo = FileVersionInfo.GetVersionInfo(assembly.Location); + // Prefer the file version, but fall back to the assembly version if it's not available. + return fileVersionInfo.FileVersion + ?? assembly.GetName().Version?.ToString() // will only change for major versions + ?? "1.0.0.0"; + } } public static class CalendarScales diff --git a/Ical.Net/Serialization/CalendarSerializer.cs b/Ical.Net/Serialization/CalendarSerializer.cs index 53d10557..ff3194d3 100644 --- a/Ical.Net/Serialization/CalendarSerializer.cs +++ b/Ical.Net/Serialization/CalendarSerializer.cs @@ -27,20 +27,6 @@ public CalendarSerializer(SerializationContext ctx) : base(ctx) { } protected override IComparer PropertySorter => new CalendarPropertySorter(); - public override string SerializeToString(object obj) - { - if (obj is Calendar) - { - // If we're serializing a calendar, we should indicate that we're using ical.net to do the work - var calendar = (Calendar) obj; - calendar.Version = LibraryMetadata.Version; - calendar.ProductId = LibraryMetadata.ProdId; - - return base.SerializeToString(calendar); - } - - return base.SerializeToString(obj); - } public override object Deserialize(TextReader tr) => null; @@ -70,4 +56,4 @@ public int Compare(ICalendarProperty x, ICalendarProperty y) : string.Compare(x.Name, y.Name, StringComparison.OrdinalIgnoreCase); } } -} \ No newline at end of file +} From 49b81e3b10eb0479e49df9ec4df70657deb0588f Mon Sep 17 00:00:00 2001 From: axunonb Date: Sun, 9 Mar 2025 16:03:38 +0100 Subject: [PATCH 2/2] Remove null or empty check from PRODID and VERSION see https://github.com/ical-org/ical.net/pull/748#discussion_r1986091395 --- Ical.Net.Tests/SerializationTests.cs | 11 ----------- Ical.Net/Calendar.cs | 18 ++---------------- 2 files changed, 2 insertions(+), 27 deletions(-) diff --git a/Ical.Net.Tests/SerializationTests.cs b/Ical.Net.Tests/SerializationTests.cs index 537a2a25..df5ea751 100644 --- a/Ical.Net.Tests/SerializationTests.cs +++ b/Ical.Net.Tests/SerializationTests.cs @@ -543,17 +543,6 @@ public void ProductId_and_Version_CanBeChanged() }); } - [Test] - public void ProductId_and_Version_CannotBeSetAsEmpty() - { - var c = new Calendar(); - Assert.Multiple(() => - { - Assert.That(() => c.ProductId = string.Empty, Throws.ArgumentException); - Assert.That(() => c.Version = string.Empty, Throws.ArgumentException); - }); - } - [Test] public void ProductId_and_Version_HaveDefaultValues() { diff --git a/Ical.Net/Calendar.cs b/Ical.Net/Calendar.cs index 28d19c00..1bf64e51 100644 --- a/Ical.Net/Calendar.cs +++ b/Ical.Net/Calendar.cs @@ -165,14 +165,7 @@ public override int GetHashCode() public virtual string Version { get => Properties.Get("VERSION"); - set - { - if (string.IsNullOrEmpty(value)) - { - throw new ArgumentException("Version to set must not be null or empty"); - } - Properties.Set("VERSION", value); - } + set => Properties.Set("VERSION", value); } /// @@ -187,14 +180,7 @@ public virtual string Version public virtual string ProductId { get => Properties.Get("PRODID"); - set - { - if (string.IsNullOrEmpty(value)) - { - throw new ArgumentException("Product ID to set must not be null or empty"); - } - Properties.Set("PRODID", value); - } + set => Properties.Set("PRODID", value); } public virtual string Scale