jkbrzt / rrule

JavaScript library for working with recurrence rules for calendar dates as defined in the iCalendar RFC and more.
https://jkbrzt.github.io/rrule
Other
3.34k stars 513 forks source link

Invalid UNTIL value #385

Open Rockatweb opened 4 years ago

Rockatweb commented 4 years ago

Hello, i found a bug in the lib when you use the UNTIL Value with a year that is smaller than 10. You can comprehend this bug on the demo page if you put the following value in the UNTIL Input:

10.10.0009, 10:10 will generate RRULE:FREQ=WEEKLY;UNTIL=00,91010T101000Z

And if you want to parse this string with the fromString-function the lib throws the error:

=> Error: Invalid UNTIL value: 00,91010T101000Z

For me this bug is critical, because when you use the date-input and start typing the year, the lib throws this error.

espen commented 4 years ago

This is not a bug but an error message. To avoid it just wait with sending the value until you have the full date in your input.

Rockatweb commented 4 years ago

ahm, excuse me but i think it is a bug, because when i put the year 0010 in it everything works just fine! Everything lower than 0010 does not work and trows an error. I think it should also work even if almost nobody will set this as a date-value.

espen commented 4 years ago
rrule js demo 2020-02-25 00-21-29
Rockatweb commented 4 years ago

so, will there be a fix for this issue?

espen commented 4 years ago

@Rockatweb please describe the issue in detail and provide a replicable way of producing the error. Getting an error from an invalid formatted date is not a bug. I will close this issue for now. Please reopen if you can provide details.

Rockatweb commented 4 years ago

ok ok, i understand. You need more details then the details in the opening post...

First of all the date 01.01.0001 is a valid date, at least here in germany. So this string ist correct, because it is the year 0001 AD. So the given string would be the following iso-date 0001-01-01. In our project we use momentjs to work with dates. With momentjs i use the .toDate() function to get a js date object which i put to the rrule until value. The code is this:

_rule.origOptions['until'] = moment(props.target.value).toDate();

If i log the given date-object the value ist this:

Mon Jan 01 0001 00:00:00 GMT+0053 (Mitteleuropäische Normalzeit)

But the library throws the error:

Error: Invalid UNTIL value: 00,81231T230632Z

because it seems that it calculates the wrong year. If i put the following date into the until-value everything works fine:

Mon Jan 01 0010 00:00:00 GMT+0053 (Mitteleuropäische Normalzeit)

So everything until this date does not work in the lib, even this one doesn't work:

Thu Dec 31 0009 00:00:00 GMT+0053 (Mitteleuropäische Normalzeit)

I hope it's now more clearly.

espen commented 4 years ago

First of all the date 01.01.0001 is a valid date

Yes, but your initial post used '10.10.0009, 10:10' which is not a valid date format (using comma).

I am unable to replicate 'invalid date'. Make sure you are using ISO8601 formatted date. Please let me know the full date you are trying. Are you also setting DTSTART to a lower value?

I do see two issues here:

1) Dates before year 10 results in a comma in the generated ical date. Example: "DTSTART:00,91010T103000Z"

2) Dates before year 100 are not generating any occurrences. For DTSTART: Example working: 0100-10-10T10:30 Example not working: 0099-10-10T10:30

Rockatweb commented 4 years ago

ok, than i am sry, that i provided a wrong date in the initial Post. I just copied it from a date-input.

So it is easy to reproduce this behaviour in the demo:

  1. Just put the date 01.01.0009 10:10 in the until-input.
  2. Than copy the string from the toString() method on the right side...

1

  1. ...and put it in the RRULE String-Parser
  2. you see that there is an error on the right side

2

It is the same way i use the library in our project. When is start typing in the date-input for example 17.03.0002 (because when you type the year you start with 2) the lib will parse this as the year 0002 and it will "crash" because of the below year 10 issue. And in react i listen on every change in the input, so it will crash wenn you just type 2.

So what am i doing? First i set all the options. Than i put it to an calendar event (we work on a calendar app) and because i use react it gets parsed right after the first keystroke. than i use the toText() methode. I just uploaded you a little movie so you can se whats happening

https://www.dropbox.com/s/wdx1eliwjgch754/video.mov?dl=0 I just type the number 2 in the input and it crashes :)

The issues that you see are "correct".

  1. generates a wrong string with a comma
  2. Years before 100 will be parsed as 2001-2099 and not 0001-0099

I hope that you can work with this informations :)

herrdommel commented 8 months ago

I also encountered this bug. Here's an example

const { RRule } = require("rrule");

const rule = new RRule({ until: new Date("0001-01-01"), freq: RRule.WEEKLY, });
const ruleString = rule.toString();

console.log(ruleString);
// =>
// RRULE:UNTIL=00,10101T000000Z;FREQ=WEEKLY

RRule.fromString(ruleString);
// =>
// Error: Invalid UNTIL value: 00,10101T000000Z

I think this is due to some strange padding in https://github.com/jkbrzt/rrule/blob/9f2061febeeb363d03352efe33d30c33073a0242/src/helpers.ts#L73

As @Rockatweb already said, this happens for all dates smaller than "0010-01-01".

Background: Calendar events from Microsoft calendars will have end date of "0001-01-01" if no end date is set... Thanks MS.

macmillen commented 4 months ago

Happened to us as well in production. It's very unfortunate that it's still not fixed after 4 years

espen commented 4 months ago

@macmillen pull requests are welcome :)

macmillen commented 3 months ago

@macmillen pull requests are welcome :)

Hey @espen I created a PR a while ago. Is there still anything you need from my side?

espen commented 3 months ago

great to see your contribution @macmillen. I added some comments. Maybe I am misunderstanding here but the test value seems to be invalid. If so please add a valid DTSTART with a year lower than 10. Also, I am just responding to issues here and not the maintainer.