GNU bug report logs - #56241
icalendar doesn't process arbitrary diary sexp entries correctly

Previous Next

To reply to this bug, email your comments to 56241 AT debbugs.gnu.org.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: hokomo <hokomo <at> airmail.cc>
To: bug-gnu-emacs <at> gnu.org
Subject: [PATCH] icalendar doesn't correctly process arbitrary diary sexp
 entries
Date: Sun, 26 Jun 2022 20:36:46 +0200

[Message part 1 (text/plain, inline)]

icalendar is capable of exporting arbitrary diary sexp entries, but it 
looks like there's a bug in the code. Each conversion function 
`icalendar--convert-*-to-ical' can return either a dotted pair or a list 
of such pairs, but the code fails to differentiate the two cases properly.

To reproduce:

(require 'icalendar)

(defun test-diary-sexp (sexp)
  (message "Testing %S\n" sexp)
  (let ((file (make-temp-file "export.ics")))
    (with-temp-buffer
      (insert sexp)
      (icalendar-export-region (point-min) (point-max) file))
    (with-current-buffer (get-buffer "*icalendar-errors*")
      (message "export: %S\n" (with-temp-buffer
                            (insert-file-contents file)
                            (buffer-string)))
      (message "errors: %S" (buffer-string))))
  (terpri))

(defun my-float (&rest args)
  (apply #'diary-float args))

(let ((icalendar-export-sexp-enumeration-days 366))
  (test-diary-sexp "%%(diary-float 7 0 1) First Sunday in July 1")
  (test-diary-sexp "%%(my-float 7 0 1) First Sunday in July 2"))

Exporting the my-float sexp will fail with:

"Error in line 0 -- (wrong-type-argument listp First Sunday in July 2): 
‘%%(my-float 7 0 1) First Sunday in July 2"

because the returned list is confused for a dotted pair.

`icalendar-export-sexp-enumeration-days' is set to 366 to guarantee that 
the sexp event occurs at least once. It looks like there's a different 
bug (?) where, even if an entry is recognized as an arbitrary diary 
sexp, if it doesn't produce any events, the converter will go ahead with 
trying to interpret it in a different way and eventually fail. E.g., 
lowering the enumeration days to 0 gives:

"Error in line 0 -- (error Could not parse date): ‘%%(my-float 7 0 1) 
First Sunday in July 2’"

after exhausting all of the known entry types. Should I file this as a 
separate bug?

Regards,
hokomo

[0001-Fix-detecting-dotted-pairs.patch (text/x-patch, attachment)]

Message #8 received at 56241 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: hokomo <hokomo <at> airmail.cc>
Cc: 56241 <at> debbugs.gnu.org
Subject: Re: bug#56241: [PATCH] icalendar doesn't correctly process
 arbitrary diary sexp entries
Date: Mon, 27 Jun 2022 10:04:53 +0200

hokomo <hokomo <at> airmail.cc> writes:

> Exporting the my-float sexp will fail with:
>
> "Error in line 0 -- (wrong-type-argument listp First Sunday in July
> 2): ‘%%(my-float 7 0 1) First Sunday in July 2"
>
> because the returned list is confused for a dotted pair.

Thanks; patch pushed to Emacs 29.

> `icalendar-export-sexp-enumeration-days' is set to 366 to guarantee
> that the sexp event occurs at least once. It looks like there's a
> different bug (?) where, even if an entry is recognized as an
> arbitrary diary sexp, if it doesn't produce any events, the converter
> will go ahead with trying to interpret it in a different way and
> eventually fail. E.g., lowering the enumeration days to 0 gives:
>
> "Error in line 0 -- (error Could not parse date): ‘%%(my-float 7 0 1)
> First Sunday in July 2’"
>
> after exhausting all of the known entry types. Should I file this as a
> separate bug?

No, we can work on this problem here in this bug report.

Do you have a recipe to reproduce the problem, starting from "emacs -Q"?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Message #11 received at 56241 <at> debbugs.gnu.org (full text, mbox):

From: hokomo <hokomo <at> airmail.cc>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: 56241 <at> debbugs.gnu.org
Subject: Re: bug#56241: [PATCH] icalendar doesn't correctly process arbitrary
 diary sexp entries
Date: Mon, 27 Jun 2022 11:42:34 +0200

[Message part 1 (text/plain, inline)]

>> `icalendar-export-sexp-enumeration-days' is set to 366 to guarantee
>> that the sexp event occurs at least once. It looks like there's a
>> different bug (?) where, even if an entry is recognized as an
>> arbitrary diary sexp, if it doesn't produce any events, the converter
>> will go ahead with trying to interpret it in a different way and
>> eventually fail. E.g., lowering the enumeration days to 0 gives:
>>
>> "Error in line 0 -- (error Could not parse date): ‘%%(my-float 7 0 1)
>> First Sunday in July 2’"
>>
>> after exhausting all of the known entry types. Should I file this as a
>> separate bug?
> 
> No, we can work on this problem here in this bug report.
> 
> Do you have a recipe to reproduce the problem, starting from "emacs -Q"?

Yep, you can use the same testing script and just replace the last 
expression with:

(let ((icalendar-export-sexp-enumeration-days 0))
  (test-diary-sexp "%%(diary-float 7 0 1) First Sunday in July 1")
  (test-diary-sexp "%%(my-float 7 0 1) First Sunday in July 2"))

which should give you the error I mentioned. I don't have a patch for 
this on hand but it would probably require restructuring the surrounding 
code a little bit.

There's another bug that concerns sexps that contain more than a single 
closing parenthesis. Seems like the icalendar code uses a bunch of 
regexes to parse the sexp (see `icalendar--convert-sexp-to-ical'), 
rather than something like `read' or `read-from-string' (which is what 
diary does, as well as org-agenda). To reproduce (with the same testing 
code as before):

(let ((icalendar-export-sexp-enumeration-days 366))
  (test-diary-sexp "%%(= (calendar-day-of-week date) 0) Sunday 1")
  (test-diary-sexp "%%(= 0 (calendar-day-of-week date)) Sunday 2"))

No matter whether the closing parentheses are bunched together or 
separated, parsing fails either way. I've attached a draft of a patch 
that modifies as little as possible and makes the above two cases work, 
but that's not enough as there's special handling of `and' forms. 
Furthermore, sexps that span multiple lines fail for the same reason, 
even though diary handles them just fine of course. Ideally the whole 
function should be rewritten to just use `read-from-string'. I don't 
understand why `and' forms are handled specially though, so I'm not sure 
how to proceed here.

A last thing to note -- even though org-agenda properly handles 
multiline sexps and sexps with an arbitrary number of closing 
parentheses (because it does the parsing manually and uses 
`forward-sexp'), Org's parser itself does not properly handle multiline 
sexps (but does handle multiple closing parentheses). To confirm, use 
`org-element-at-point' at the beginning of these two diary sexps in an 
org-mode buffer:

%%(let ((a 123) (b 456)) (+ a b))

%%(let ((a 123)
        (b 456))
   (+ a b))

All of these bugs also propagate to ox-icalendar (which is how I got 
into exploring this whole thing) since it relies on the Org parse tree 
to pull out 'diary-sexp elements and on icalendar to export them into 
iCalendar format.

Let me know if you want me to report any of these as separate bugs and 
what would be the best thing to do here.

Regards,
hokomo

[0002-Draft-fix-nested-diary-sexps.patch (text/x-patch, attachment)]

Message #14 received at 56241 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: hokomo <hokomo <at> airmail.cc>
Cc: Ulf Jasper <ulf.jasper <at> web.de>, 56241 <at> debbugs.gnu.org
Subject: Re: bug#56241: [PATCH] icalendar doesn't correctly process
 arbitrary diary sexp entries
Date: Tue, 28 Jun 2022 13:21:48 +0200

hokomo <hokomo <at> airmail.cc> writes:

> Yep, you can use the same testing script and just replace the last
> expression with:
>
> (let ((icalendar-export-sexp-enumeration-days 0))
>   (test-diary-sexp "%%(diary-float 7 0 1) First Sunday in July 1")
>   (test-diary-sexp "%%(my-float 7 0 1) First Sunday in July 2"))
>
> which should give you the error I mentioned.

Right; I can reproduce that problem.  I've added Ulf to the CCs; perhaps
he has some comments.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Message #17 received at 56241 <at> debbugs.gnu.org (full text, mbox):

From: hokomo <hokomo <at> airmail.cc>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: Ulf Jasper <ulf.jasper <at> web.de>, 56241 <at> debbugs.gnu.org
Subject: Re: bug#56241: [PATCH] icalendar doesn't correctly process arbitrary
 diary sexp entries
Date: Wed, 6 Jul 2022 20:58:23 +0200

> Right; I can reproduce that problem.  I've added Ulf to the CCs; perhaps
> he has some comments.

It's been a week since then. Any ideas on how to move forward or should 
we give it more time? :-)

To summarize, the problems are:

(1) A diary sexp not generating any events for the configured number of 
enumeration days is treated effectively as a parsing failure, meaning 
that the next type of diary entry is tried instead of just not producing 
any events.

(2) Improper parsing of diary sexps (using regexes instead of e.g. 
`read') leads to diary sexps not being able to span multiple lines or 
have more than a single closing parenthesis (i.e. any sort of nested 
expression).

(3) Not strictly in the scope of icalendar itself, but still related the 
handling of diary sexps: Org's parser seems to assume that each diary 
sexp is on a single line. This and the two limitations above end up 
propagating to ox-icalendar.

hokomo

Message #20 received at 56241 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: hokomo <hokomo <at> airmail.cc>
Cc: Ulf Jasper <ulf.jasper <at> web.de>, 56241 <at> debbugs.gnu.org
Subject: Re: bug#56241: [PATCH] icalendar doesn't correctly process
 arbitrary diary sexp entries
Date: Thu, 07 Jul 2022 10:08:16 +0200

hokomo <hokomo <at> airmail.cc> writes:

> It's been a week since then. Any ideas on how to move forward or
> should we give it more time? :-)

If you have a possible patch for the issues, that'd be appreciated.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Message #23 received at 56241 <at> debbugs.gnu.org (full text, mbox):

From: Ulf Jasper <ulf.jasper <at> web.de>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: 56241 <at> debbugs.gnu.org, hokomo <hokomo <at> airmail.cc>
Subject: Re: bug#56241: [PATCH] icalendar doesn't correctly process
 arbitrary diary sexp entries
Date: Sun, 24 Jul 2022 20:14:49 +0200

Am 28.06.2022 um 13:21 (+0200) schrieb Lars Ingebrigtsen:
> hokomo <hokomo <at> airmail.cc> writes:
>
>> Yep, you can use the same testing script and just replace the last
>> expression with:
>>
>> (let ((icalendar-export-sexp-enumeration-days 0))
>>   (test-diary-sexp "%%(diary-float 7 0 1) First Sunday in July 1")
>>   (test-diary-sexp "%%(my-float 7 0 1) First Sunday in July 2"))
>>
>> which should give you the error I mentioned.
>
> Right; I can reproduce that problem.  I've added Ulf to the CCs; perhaps
> he has some comments.

Sorry for the late reply.  Thanks for the bug report, the patch and for
analysing the problems.  I'll try to have a look at the icalendar code
the next days (or so).

Message #28 received at 56241 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Kangas <stefankangas <at> gmail.com>
To: Ulf Jasper <ulf.jasper <at> web.de>
Cc: Lars Ingebrigtsen <larsi <at> gnus.org>, 56241 <at> debbugs.gnu.org,
 hokomo <hokomo <at> airmail.cc>
Subject: Re: bug#56241: [PATCH] icalendar doesn't correctly process arbitrary
 diary sexp entries
Date: Fri, 11 Nov 2022 05:38:52 -0800

Ulf Jasper <ulf.jasper <at> web.de> writes:

> Am 28.06.2022 um 13:21 (+0200) schrieb Lars Ingebrigtsen:
>> hokomo <hokomo <at> airmail.cc> writes:
>>
>>> Yep, you can use the same testing script and just replace the last
>>> expression with:
>>>
>>> (let ((icalendar-export-sexp-enumeration-days 0))
>>>   (test-diary-sexp "%%(diary-float 7 0 1) First Sunday in July 1")
>>>   (test-diary-sexp "%%(my-float 7 0 1) First Sunday in July 2"))
>>>
>>> which should give you the error I mentioned.
>>
>> Right; I can reproduce that problem.  I've added Ulf to the CCs; perhaps
>> he has some comments.
>
> Sorry for the late reply.  Thanks for the bug report, the patch and for
> analysing the problems.  I'll try to have a look at the icalendar code
> the next days (or so).

That was 15 weeks ago, so here's a friendly ping.

Message #31 received at 56241 <at> debbugs.gnu.org (full text, mbox):

From: Ulf Jasper <ulf.jasper <at> web.de>
To: Stefan Kangas <stefankangas <at> gmail.com>
Cc: Lars Ingebrigtsen <larsi <at> gnus.org>, 56241 <at> debbugs.gnu.org,
 hokomo <hokomo <at> airmail.cc>
Subject: Re: bug#56241: [PATCH] icalendar doesn't correctly process
 arbitrary diary sexp entries
Date: Wed, 23 Nov 2022 21:24:45 +0100

I added a test to icalendar-tests.el that verifies that patch
0001-Fix-detecting-dotted-pairs.patch works as expected.

I also prepared two additional tests for the other examples where
icalendar-export fails.  For the time being they are commented out as
there is no fix available yet.  Patch
0002-Draft-fix-nested-diary-sexps.patch seems to fix one of the examples
but apparently breaks test 'icalendar-real-world'.  So a look at
'icalendar--convert-sexp-to-ical' seems to be necessary.

Best,
Ulf

Message #34 received at 56241 <at> debbugs.gnu.org (full text, mbox):

From: Ulf Jasper <ulf.jasper <at> web.de>
To: Stefan Kangas <stefankangas <at> gmail.com>
Cc: Lars Ingebrigtsen <larsi <at> gnus.org>, 56241 <at> debbugs.gnu.org,
 hokomo <hokomo <at> airmail.cc>
Subject: Re: bug#56241: [PATCH] icalendar doesn't correctly process
 arbitrary diary sexp entries
Date: Thu, 24 Nov 2022 19:07:12 +0100

Am 23.11.2022 um 21:24 (+0100) schrieb Ulf Jasper:
> I added a test to icalendar-tests.el that verifies that patch
> 0001-Fix-detecting-dotted-pairs.patch works as expected.
>
> I also prepared two additional tests for the other examples where
> icalendar-export fails.  For the time being they are commented out as
> there is no fix available yet.  Patch
> 0002-Draft-fix-nested-diary-sexps.patch seems to fix one of the examples
> but apparently breaks test 'icalendar-real-world'.  So a look at
> 'icalendar--convert-sexp-to-ical' seems to be necessary.

There was a testcase in 'icalendar-real-world' that relied on
icalendar-export to fail on a particular sexp.  With patch 0002... that
testcase behaves differently (i.e. export works correctly now).  In
other words: breaking that test was correct.

Applied patch 0002... and updated icalendar-tests.

Best,
Ulf

Message #37 received at 56241 <at> debbugs.gnu.org (full text, mbox):

From: Brendan O'Dea <bod <at> debian.org>
To: Ulf Jasper <ulf.jasper <at> web.de>
Cc: Lars Ingebrigtsen <larsi <at> gnus.org>, 56241 <at> debbugs.gnu.org,
 Stefan Kangas <stefankangas <at> gmail.com>, hokomo <hokomo <at> airmail.cc>
Subject: Re: bug#56241: [PATCH] icalendar doesn't correctly process arbitrary
 diary sexp entries
Date: Tue, 4 Jul 2023 20:29:03 +1000

[Message part 1 (text/plain, inline)]

On Wed, Nov 23, 2022 at 09:24:45PM +0100, Ulf Jasper wrote:
>I added a test to icalendar-tests.el that verifies that patch
>0001-Fix-detecting-dotted-pairs.patch works as expected.

Note that this test is failing on both the current 'master' and
'emacs-29' branch (excerpt, full log attached):

  "Error in line 0 -- (error Could not parse date): `%%(icalendar-tests--diary-float 7 0 1) First Sunday in July 2'\n"
  FAILED  25/42  icalendar-export-bug-56241-dotted-pair (0.003671 sec) at lisp/calendar/icalendar-tests.el:1000

From what I can tell, the problem is that while these calls look like
they should be identical:

  (diary-float 7 0 1)
  (icalendar-tests--diary-float 7 0 1)  ;; calls diary-float

in practice, they are handled by different code paths:

  icalendar--convert-float-to-ical
  icalendar--convert-sexp-to-ical

resp.  The key difference being that the first one sets 'date' and
'entry' with calendar-dlet when calling diary-float*, and the latter
does not.  diary-float is documented as requiring those to be set.†

I'm not sure of the fix here: if the expectation is that some arbitrary
sexp should be able to indirectly call some of the diary-*‡ functions,
then it may be that icalendar--convert-sexp-to-ical may need to
similarly set those values.  Alternately, if that is not supported, then
maybe disable/remove the test.

--bod

* https://git.savannah.gnu.org/cgit/emacs.git/tree/lisp/calendar/icalendar.el?id=b42cd524b46a4f29ef13e9d03be9d3df917f9aa3#n1787
† https://git.savannah.gnu.org/cgit/emacs.git/tree/lisp/calendar/diary-lib.el?id=b42cd524b46a4f29ef13e9d03be9d3df917f9aa3#n1891
‡ diary-date, diary-block, diary-float, diary-anniversary, diary-cyclic,
  and diary-offset (there may be others, those were the ones with
  an explicit comment)

[icalendar-tests.log.gz (application/gzip, attachment)]

Message #40 received at 56241 <at> debbugs.gnu.org (full text, mbox):

From: Mattias Engdegård <mattias.engdegard <at> gmail.com>
To: Brendan O'Dea <bod <at> debian.org>
Cc: Ulf Jasper <ulf.jasper <at> web.de>, Lars Ingebrigtsen <larsi <at> gnus.org>,
 56241 <at> debbugs.gnu.org, Stefan Kangas <stefankangas <at> gmail.com>,
 hokomo <hokomo <at> airmail.cc>
Subject: bug#56241: [PATCH] icalendar doesn't correctly process arbitrary
 diary sexp entries
Date: Tue, 4 Jul 2023 16:20:39 +0200

Thank you for looking into these failures.

The test started failing around the first Sunday of July, which is unlikely to be a coincidence.
Calendar tests had better not depend on the time or date when the test is run. If necessary, these should be mocked up by the tests.

Message #43 received at 56241 <at> debbugs.gnu.org (full text, mbox):

From: Mattias Engdegård <mattias.engdegard <at> gmail.com>
To: Brendan O'Dea <bod <at> debian.org>
Cc: Ulf Jasper <ulf.jasper <at> web.de>, Lars Ingebrigtsen <larsi <at> gnus.org>,
 56241 <at> debbugs.gnu.org, Stefan Kangas <stefankangas <at> gmail.com>,
 hokomo <hokomo <at> airmail.cc>
Subject: Re: bug#56241: [PATCH] icalendar doesn't correctly process arbitrary
 diary sexp entries
Date: Fri, 7 Jul 2023 15:15:13 +0200

The `icalendar-export-bug-56241-dotted-pair` test has now been marked :unstable, which means that it is not run automatically by `make check`.
The problem still remains, and it would be good to have a working regression test for the bug, assuming it has indeed been fixed.

Message #48 received at 56241 <at> debbugs.gnu.org (full text, mbox):

From: Jeremy Harrison <jeremy <at> harrisonbundrock.com>
To: "56241 <at> debbugs.gnu.org" <56241 <at> debbugs.gnu.org>
Subject: Re: bug#56241: [PATCH] icalendar doesn't correctly process arbitrary
 diary sexp entries
Date: Sun, 28 Jan 2024 17:09:52 +0000

[Message part 1 (text/plain, inline)]

Not sure if this helps but I was wondering why exporting '%%(org-anniversary 2020 02 02) blah %d blah' worked but '%%(org-anniversary 2020 03 02) blah %d blah' did not.

Long story short, icalendar-export-sexp-enumeration-days was set to 14 by default. I have changed this to 366 and the export works again.

If relevant, I noticed that one of the tests mentioned above does not set the value explicitly while the others do.

I'm not a programmer so take my observations with due caution!

Jeremy

[Message part 2 (text/html, inline)]

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.