Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

invalid UTC-OFFSET from iCloud prevent parsing the feed #245

Closed
djoul6 opened this issue Mar 14, 2017 · 12 comments
Closed

invalid UTC-OFFSET from iCloud prevent parsing the feed #245

djoul6 opened this issue Mar 14, 2017 · 12 comments

Comments

@djoul6
Copy link

djoul6 commented Mar 14, 2017

When an iCal feed contains UTC-Offset with 4 numbers, there is a parsing exception (feed received from iCloud calendars).
Here is an example:

TZOFFSETFROM:+1730
TZOFFSETTO:+1730

As recently fixed (76025b3), the parsing is now done on 6 characters (hhmmss)

TimeSpan.TryParseExact(rawOffset, "hhmmss", CultureInfo.InvariantCulture, out ts)

But the seconds are optionnals, according to the rfc 5545:

time-numzone = ("+" / "-") time-hour time-minute [time-second]

@rianjs
Copy link
Collaborator

rianjs commented Mar 14, 2017

There's no place on Earth that has a 17 hour, 30 minute UTC offset.

https://en.wikipedia.org/wiki/List_of_UTC_time_offsets

@djoul6
Copy link
Author

djoul6 commented Mar 14, 2017

I completely agree with you, but it's what we get from iCloud calendars ! (including multiple feeds)
We can make a manual replace (before parsing), to "hard-fix" it like this:

TZOFFSETFROM:+001730
TZOFFSETTO:+001730

I've checked multiple iCloud feed, and this value (1730) seems to be always with the "BMT" Timezone, here is a sample:

END:VTIMEZONE
BEGIN:VTIMEZONE
TZID:Europe/Brussels
X-LIC-LOCATION:Europe/Brussels
BEGIN:STANDARD
DTSTART:18800101T000000
RDATE;VALUE=DATE-TIME:18800101T000000
TZNAME:BMT
TZOFFSETFROM:+1730
TZOFFSETTO:+1730
END:STANDARD

But some others invalid values also exists from iCloud feeds:

BEGIN:VTIMEZONE
TZID:Europe/Berlin
X-LIC-LOCATION:Europe/Berlin
BEGIN:STANDARD
DTSTART:18930401T000000
RDATE;VALUE=DATE-TIME:18930401T000000
TZNAME:CEST
TZOFFSETFROM:+5328
TZOFFSETTO:+0100
END:STANDARD

This was also discussed on this issue:
#147

After investigations, this issue from iCloud feed is not new, and others projects also got problem to deal with it:
collective/icalendar#155

@djoul6 djoul6 changed the title UTC-OFFSET not well parsed invalid UTC-OFFSET from iCloud prevent parsing the feed Mar 14, 2017
@rianjs
Copy link
Collaborator

rianjs commented Mar 15, 2017

If there's one thing that's certain, it's that Apple won't bother to fix their bug.

If there's another thing that's certain, it's that Apple is rubbish at writing cloud services.

@bymem
Copy link

bymem commented Mar 27, 2017

Having the same problem. with the offset being

Offset must be less than 24 hours, was +5020

is there any news on how to fix it?

@rianjs
Copy link
Collaborator

rianjs commented Mar 27, 2017

+5020 means a UTC offset of 50 hours and 20 minutes. That's not a real offset.

The problem is that the data is bad, so you should fix the data; I don't intend to allow nonsense inputs like that, sorry.

@bymem
Copy link

bymem commented Mar 27, 2017

But that is directly from a icloud calander feed, i don't have control over that.

@djoul6
Copy link
Author

djoul6 commented Mar 27, 2017

Can't it be a pragmatic solution to ignore this kind of invalid data instead of throwing an exception ?

@rianjs
Copy link
Collaborator

rianjs commented Mar 27, 2017

Can't it be a pragmatic solution to ignore this kind of invalid data instead of throwing an exception ?

When exceptional situations occur, we should throw exceptions. Wildly bad offsets are exceptional. The right thing to do is to throw.

It's your job to catch and figure out what you want to do when that happens.

But that is directly from a icloud calander feed, i don't have control over that.

Sure you do. You can check for bad offsets in your code, and do something about them. A library can't possibly know what every application developer will want to do with an exceptional situation. That's why the library throws -- to put the ball back in the app developers court to let them figure out what the right course of action is.

@rianjs rianjs closed this as completed Mar 27, 2017
@rianjs
Copy link
Collaborator

rianjs commented Mar 27, 2017

Well you two have an anonymous person who advocated on your behalf, and since he sits near me, he's harder for me to ignore. :P

By way of explanation: behind the scenes, ical.net uses a DateTimeOffset to parse offsets. The limit for that type is +/-14 hours, otherwise it throws, as you've seen. Expecting you to parse your data interchange format before you parse your data interchange format does seem a little unfair.

So I'll think about this a bit more...

@rianjs rianjs reopened this Mar 27, 2017
@aluxnimm
Copy link

I guess this was the reason why the original DDay.iCal parsed it manually with some regex.

@djoul6
Copy link
Author

djoul6 commented Mar 27, 2017

Even if the exception is catched, it will continue to fail the feed parsing, and then it's clearly not a possible solution to simply catch exception.
Of course we can adapt your code to implement it in a "future-proof" way, but it will be overkill to fork the project, just for that.

Avoid throwing exception is not an option to do bad code, but it could be a starting point of reflexion to imagine a better solution, including the management of this kind of partially incorrect feed !
For example it could be a configuration defining the parsing as "strict" (as actually) or not, or to allow the library to override the "DateTimeOffset" parser, by exposing the parser factory or any other technical entry point, like that the library can continue to be used as-is, and the application able to override the built-in parsing mechanism.

@minichma
Copy link
Collaborator

minichma commented Mar 21, 2018

The IANA tzdb says about Guam:

# Guam
# Zone	NAME		GMTOFF	RULES	FORMAT	[UNTIL]
Zone	Pacific/Guam	-14:21:00 -	LMT	1844 Dec 31
			 9:39:00 -	LMT	1901        # Agana
...

so before 1845 -14:21 seems to have been a valid offset in Guam (at least IANA says so).

libical's vzic utility makes TZOFFSETFROM:-1421 out of this data.

For Manila it says

# Zone	NAME		GMTOFF	RULES	FORMAT	[UNTIL]
Zone	Asia/Manila	-15:56:00 -	LMT	1844 Dec 31

...

@axunonb axunonb closed this as completed Oct 13, 2024
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

No branches or pull requests

6 participants