Skip to content

Conversation

@edds
Copy link

@edds edds commented Nov 27, 2025

This technically is invalid as RFC-5455 states:

icalbody   = calprops component
calprops   = ­*(
              ;
              ; The following are REQUIRED,
              ; but MUST NOT occur more than once.
              ;
              prodid / version /
              ;
              ; The following are OPTIONAL,
              ; but MUST NOT occur more than once.
              ;
              calscale / method /
              ;
              ; The following are OPTIONAL,
              ; and MAY occur more than once.
              ;
              x-prop / iana-prop
              ;
              )

component  = 1*(eventc / todoc / journalc / freebusyc /
              timezonec / iana-comp / x-comp)

However, in the wild we're getting people upload ics files with properties after components. In the spirit of being strict about what you what you send and liberal in what you accept (Postel's law), allow the properties to appear after the components.

This technically is invalid as RFC-5455 states:

```
icalbody   = calprops component
calprops   = ­*(
              ;
              ; The following are REQUIRED,
              ; but MUST NOT occur more than once.
              ;
              prodid / version /
              ;
              ; The following are OPTIONAL,
              ; but MUST NOT occur more than once.
              ;
              calscale / method /
              ;
              ; The following are OPTIONAL,
              ; and MAY occur more than once.
              ;
              x-prop / iana-prop
              ;
              )

component  = 1*(eventc / todoc / journalc / freebusyc /
              timezonec / iana-comp / x-comp)
```

However, in the wild we're getting people upload ics files with
properties after components. In the spirit of being strict about what
you what you send and liberal in what you accept (Postel's law), allow
the properties to appear after the components.
@arran4
Copy link
Owner

arran4 commented Nov 27, 2025

@edds Do you have any examples, or suspect programs etc?

@arran4 arran4 self-assigned this Nov 27, 2025
@arran4 arran4 self-requested a review November 27, 2025 22:18
@edds
Copy link
Author

edds commented Nov 27, 2025

Yeah, we're seeing this in ics files produced by vacationtracker.io I'm reaching out to them also to see if they can fix it to be stricter. That test case I added is a very stripped down version of almost exactly the file they are producing though (with the same VTIMEZONE component before the TIMEZONE-ID property. Happy if you don't want to merge this as I appreciate it's an edge case.

@arran4
Copy link
Owner

arran4 commented Nov 27, 2025

Personally I would like to introduce toggles for breaking standards rather than all or nothing (either way.)

Kind of what is holding me applying more updates to this repo (as well as time in general) is the v0 v1 v2 debate as a result of this pr:

and

Which spiralled out of into a very big change, and I have one (potential?) user who is against it. No one else who uses it has weighed in so they are sort of in limbo. (Very ashamed.)

In one of the above I introduced the vararg option style toggles however there was concern I would be breaking interfaces as a result and thus pushback.

Ie:

Parse() to Parse(options... Options)

Let me know what you think, and if you're willing to adapt the PR from that. Or if you can think of another way that doesn't involve globals.

@edds
Copy link
Author

edds commented Nov 28, 2025

Given you're pre v1, personally I'm less concerned about the breaking change (especially as the break would be caught at compile time), but equally there are a number of people that depend on it. I can also see the benefits of drawing a line in the sand, cutting a v1 now and then having the benefits of major version numbers to handle this. It would clear your conscious to make all kinds of changes.

I'd be very happy to move this to a allowPropertiesAfterComponents or equivalent option if you'd be open to that making it in.

@edds
Copy link
Author

edds commented Dec 5, 2025

Hey @arran4 how about something like this, where we maintain the original parse but introduce a new one where you can allow relaxed parsing rules.

@arran4
Copy link
Owner

arran4 commented Dec 7, 2025

Hey @arran4 how about something like this, where we maintain the original parse but introduce a new one where you can allow relaxed parsing rules.

That sounds like a lot of work and duplication. Are you kind of suggestiing a V2?

@edds
Copy link
Author

edds commented Dec 7, 2025

Sorry, I mean I've pushed an extra commit so you can maintain the current interface while also having an options passed in.

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.

2 participants