search

arran4 / golang-ical

A ICS / ICal parser and serialiser for Golang.
Apache License 2.0
279 stars 72 forks source link

Fix various text escaping issues #86

Closed frereit closed 4 months ago

frereit commented 4 months ago
  1. Make sure that text escapes are handled correctly in all cases.
  2. Normalize property.Value to always contain unescaped values. They are escaped when serializing, and unescaped when parsing.
arran4 commented 4 months ago

@frereit Do you have an example / test of where this fails in the current implementation?

frereit commented 4 months ago

I have added a test that is fixed by 0f8a325.

3ffa099 is not directly fixing a bug but simply normalizing when escaping happens. This is so that these two statements have the same effect:

event.SetProperty(ics.ComponentPropertyDescription, "D:escription")
event.SetDescription("D:escription")

which they currently do not have, because SetDescription performs escaping, while SetProperty does not.

frereit commented 4 months ago

By the way, this closes #68

arran4 commented 4 months ago

Thanks. Running tests now.

arran4 commented 4 months ago

Seems there is a white space issue in the test on Mac OS X

frereit commented 4 months ago

I think the issue was that the order of property parameters was non-deterministic. I've removed the unneccessary property parameter, the test should now be fixed. It's a bit difficult to debug because the test passes on my local machine.

arran4 commented 4 months ago

I think the issue was that the order of property parameters was non-deterministic. I've removed the unneccessary property parameter, the test should now be fixed. It's a bit difficult to debug because the test passes on my local machine.

I can believe that.. Although I thought I had written this with very limited use of maps as I wanted it to be bidirectional and deterministic..

It didn't show up in tests and I can't see how this would create more nondeterministic behavior, plus only showing up on mac is a bit odd.

One earlier PR did add some fuzzing. I will have to make some time to do more advanced testing it looks like.

arran4 commented 4 months ago

https://github.com/arran4/golang-ical/releases/tag/v0.2.5