kewisch / ical.js

Javascript parser for ics (rfc5545) and vcard (rfc6350) data
https://kewisch.github.io/ical.js/
Mozilla Public License 2.0
992 stars 137 forks source link

Duration's fromString & toString may be incorrect acrroding to rfc5545 #732

Open FangNotFish opened 2 months ago

FangNotFish commented 2 months ago

rfc5545 says the notation of duration is:

dur-value  = (["+"] / "-") "P" (dur-date / dur-time / dur-week)
dur-date   = dur-day [dur-time]
dur-time   = "T" (dur-hour / dur-minute / dur-second)
dur-week   = 1*DIGIT "W"
dur-hour   = 1*DIGIT "H" [dur-minute]
dur-minute = 1*DIGIT "M" [dur-second]
dur-second = 1*DIGIT "S"
dur-day    = 1*DIGIT "D"

For convenience, ignore the leading (["+"] / "-") "P" and use NUM to represent 1*DIGIT, then expand it:

  1. (dur-date)
    • NUM "D"
    • NUM "D" "T" NUM "H" [NUM "M" [NUM "S"]]
    • NUM "D" "T" NUM "M" [NUM "S"]
    • NUM "D" "T" NUM "S"
  2. (dur-time)
    • "T" NUM "H" [NUM "M" [NUM "S"]]
    • "T" NUM "M" [NUM "S"]
    • "T" NUM "S"
  3. (dur-week)
    • NUM "W"

Thus, the conclusion is

  1. each parts of dur-time is ordered.
  2. dur-week is mutually exclusive with dur-date and dur-time.

In current implementation of ical.js, the order is ignored and dur-week could appear while dur-date or dur-time is displayed.

parseDurationChunk(...) do not respect conclusion#1, just assume the input string follow the rfc5545 spec. https://github.com/kewisch/ical.js/blob/04e437a80de542b181ba287533886e4ec838a93b/lib/ical/duration.js#L350

toString() do not respect conclusion#2, but after normalize() called would be fine. https://github.com/kewisch/ical.js/blob/04e437a80de542b181ba287533886e4ec838a93b/lib/ical/duration.js#L276 https://github.com/kewisch/ical.js/blob/04e437a80de542b181ba287533886e4ec838a93b/lib/ical/duration.js#L277

kewisch commented 6 days ago

Thanks for reporting, your conclusions seem right to me as well. Let's see if we can fix toString as well!