Skip to content

Conversation

@axunonb
Copy link
Collaborator

@axunonb axunonb commented Mar 6, 2025

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

@axunonb
Copy link
Collaborator Author

axunonb commented Mar 6, 2025

@minichma TBD: Add the current ical.net version to the PRODID

@minichma
Copy link
Collaborator

minichma commented Mar 6, 2025

The RFC says the PRODID should contain

Any text that describes the product and version

so, yes, including the version would probably be a good idea.

@axunonb
Copy link
Collaborator Author

axunonb commented Mar 6, 2025

@minichma RFC5545 says that PRODID and VERSION are mandatory. This may be the reason for the current implementation (always set the defaults when serializing).
But given this ics with missing PRODID and VERSION to deserialize:

BEGIN:VCALENDAR
BEGIN:VEVENT
DTSTART:20070406T230000Z
DTEND:20070407T010000Z
END:VEVENT
END:VCALENDAR

TBD:
Options

  • Make it compliant by adding missing values: Then we don't see the original values.
  • Leave it invalid. It's up to the user to fix, if needed before serializing (my preferred option, - breaking change to previous versions)
  • Leave the properties unchanged (invalid), but replace empty PRODID or VERSION when serializing (non-braking, but kind of unexpected magic)

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 ical-org#531
@minichma
Copy link
Collaborator

minichma commented Mar 6, 2025

Leave it invalid. It's up to the user to fix, if needed before serializing (my preferred option, - breaking change to previous versions)

My preferred option too. Only populate ProdId and Version when creating a new calendar, but not when deserializing an existing one.

@axunonb axunonb requested a review from minichma March 6, 2025 22:56
@axunonb axunonb marked this pull request as ready for review March 7, 2025 08:18
minichma
minichma previously approved these changes Mar 8, 2025
@sonarqubecloud
Copy link

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Setting own ProductId or Version has no effect

2 participants