search

tritech / node-icalendar

iCalendar parser and generator for Node.js
MIT License
234 stars 50 forks source link

Replacing of newlines with \\n leaves single carriage returns #14

Closed herr-biber closed 11 years ago

herr-biber commented 11 years ago

https://github.com/tritech/node-icalendar/blob/master/lib/types.js#L174

Shouldn't we strip '\r' from the strings as well? .replace(/\r/g,'')

leider commented 11 years ago

Carriage Returns are not handled in the library at all. If you look at https://github.com/tritech/node-icalendar/blob/master/lib/types.js#L172 you can see that \r is neither correctly parsed nor correctly escaped.

It can be checked by checking the resulting String with .indexOf("\r") if it still includes CR.

james-emerton commented 11 years ago

According to http://tools.ietf.org/html/rfc5545#section-3.3.11 carriage returns (\r) is not within the list of ESCAPED-CHAR values.

herr-biber commented 11 years ago

Thanks for pointing out the RFC. I was not mentioning escaping CRs, but treating \r\n as newline as well. Are you sure, 'newline' in this RFC means \n and not 'any new line denoting sequence' (\r\n, r, \n)?

Currently the API is expecting \n terminated lines, despite this document [0] and various [1] other [2] icalendar implementations [3].

[0] http://www.rfc-editor.org/EOLstory.txt [1] https://github.com/rubyredrick/ri_cal/blob/369a4eecb22251a7e71fa127aa023b7a305a5b60/lib/ri_cal/property_value/text.rb#L34 [2] https://github.com/collective/icalendar/blob/40bcdf6effa4e75bf3b52e9c7ee7f52eb7c99d18/src/icalendar/parser.py#L29 [3] https://github.com/sam-github/vpim/blob/57b54a019942f19e3c2b24b79d3c5a94d4b89bdd/ex_fmt_convert.rb#L11