ical4j/ical4j

Text property (e.g. DESCRIPTION) escapes DQUOTE without need

Closed this issue · 3 comments

Describe the bug
PropertyCodec escapes double-quote characters in the text from "\":

String encoded = SPECIALCHAR_EX.matcher(
NEWLINE_EX.matcher(
BACKSLASH_EX.matcher(source).replaceAll(ENCODED_BACKSLASH)
).replaceAll(ENCODED_NEWLINE)
).replaceAll("\\\\$1");

(SPECIALCHAR_EX contains ")

For instance, this causes Description("Test with \"dquote\"") to be rendered as DESCRIPTION:Test with \"dquote\" (should be: DESCRIPTION:Test with "dquote")

To Reproduce

    @Test
    fun testEscapePropertyValueWithDquote() {
        assertEquals("Test \"quote\"", PropertyCodec.INSTANCE.encode("Test \"quote\""))
    }

Additional context

RFC 5545 defines the syntax of TEXT properties as:

   text       = *(TSAFE-CHAR / ":" / DQUOTE / ESCAPED-CHAR)
      ; Folded according to description above

   ESCAPED-CHAR = ("\\" / "\;" / "\," / "\N" / "\n")
      ; \\ encodes \, \N or \n encodes newline
      ; \; encodes ;, \, encodes ,

   TSAFE-CHAR = WSP / %x21 / %x23-2B / %x2D-39 / %x3C-5B /
                %x5D-7E / NON-US-ASCII
      ; Any character except CONTROLs not needed by the current
      ; character set, DQUOTE, ";", ":", "\", ","

Note that text can directly contain unescaped DQUOTE. ESCAPED-CHAR on the other side does not contain "\"".

RFC 2445 reads similarly.

Hi Ben, if you consider this as valid and are interested in a PR @ArnyminerZ could give it a try (Arnau is working with us on DAVx⁵).

Hi @rfc2822 , thanks for the suggestion definitely agree. It may have resulted from a user example but happy to change it now and see if anything breaks.

@devvv4ever yes by all means I appreciate any PRs you can provide, big or small. If you think there is an unambiguous fix to add you can just raise a PR without an issue I'll be happy to look at it.

@devvv4ever also just an FYI, soon I think we will be publishing release candidates for ical4j 4.x, so at that point the develop branch will become 4.x and 3.x will move to a maintenance branch.

Just something to keep in mind, but I will still apply fixes/updates to the maintenance branch.