remove invalid XML characters from vCalAddress xCal output by alhudz · Pull Request #1969 · PyCalendar/icalendar

alhudz · 2026-10-18T12:45:42Z

Linked issue

Contributes to Document/solve how to handle different characters #1888

Description

In #1888, vCalAddress is listed as the one class that can create invalid xCal output. vCalAddress.to_xcal() writes its value into <cal-address> without the _to_valid_xml_string filter that vText, vUri and vUnknown use. Parsing an .ics file strips control characters, but jCal keeps them. Because of that, an ORGANIZER/ATTENDEE value, or a SENT-BY/DELEGATED-FROM/MEMBER parameter, that contains something like U+0001 makes to_xcal() output bytes that no XML parser accepts, including our own from_xcal:

from xml.etree.ElementTree import fromstring
from icalendar import Calendar

cal = Calendar.from_jcal(["vcalendar", [], [["vevent", [
    ["organizer", {"sent-by": "mailto:s\u0002@x"}, "cal-address", "mailto:a\u0001b@example.com"],
], []]]])
fromstring(cal.to_xcal())  # ParseError: not well-formed (invalid token)

The filter itself also missed U+FFFE and U+FFFF. Both are outside the XML Char production cited in test_text_escape.py, and they reach vText from ordinary UTF-8 .ics input, for example SUMMARY:a\xef\xbf\xbeb.

The existing test_invalid_xml_characters_are_replaced test now also runs for vCalAddress and checks the two noncharacters. Lone surrogates are left alone: they already fail in to_ical() and aren't specific to xCal.

Checklist

I added a change log entry, following the instructions in all subsections under Change log requirements.
I followed icalendar's Artificial intelligence policy and disclosed my Responsible AI use in my commit messages, if applicable.
I added or updated tests, if applicable.
I ran and ensured all tests pass locally by following Run tests.
I added or edited documentation as necessary, both as docstrings to be rendered in the API documentation and narrative documentation, following the Style guide.

Additional information

The updated test fails 82 cases without the fix and passes with it. The full suite passes (21393 passed), and ruff check and ruff format --check are clean on the touched files.

vCalAddress.to_xcal() wrote the value into <cal-address> unfiltered, so a C0 control character from jCal produced XML that is not well-formed. Route it through _to_valid_xml_string like vText, vUri and vUnknown, and add U+FFFE and U+FFFF to that filter. AI use: Claude Opus 5.5 (claude-opus-5-5) via Claude Code. I asked it to look for a security-relevant bug in icalendar; it found the vCalAddress gap noted in PyCalendar#1888, wrote a jCal -> xCal reproducer, the fix, and the test changes, and ran the test suite and ruff. I reviewed the output.

github-actions · 2026-10-18T12:45:53Z

Thanks for the pull request, @alhudz! A maintainer will review it shortly.

github-actions · 2026-10-18T12:45:54Z

This pull request did not pass quality checks and AI use is suspected. Please review Contribute and make any necessary amendments.

alhudz · 2026-10-21T09:11:04Z

@faresrafat3 It is re.sub: _to_valid_xml_string returns _VALID_XML_CHARACTERS.sub("", string) (src/icalendar/parser/xcal/string.py, unchanged by this PR), so U+FFFE and U+FFFF are removed, not rejected. test_invalid_xml_characters_are_replaced checks exactly that: the output equals the input with the invalid character dropped.

niccokunzmann

Thanks for the PR. I have some questions.

niccokunzmann · 2026-10-21T10:43:38Z



-_VALID_XML_CHARACTERS = re.compile(r"[\x00-\x08\x0b\x0c\x0e-\x1f]")
+_VALID_XML_CHARACTERS = re.compile(r"[\x00-\x08\x0b\x0c\x0e-\x1f\ufffe\uffff]")


I think, this needs a docstring saying why which characters are allowed.
With references to the source.

Added in dab0b71. The docstring quotes the Char production from XML 1.0 section 2.2 and lists what the pattern matches: C0 controls apart from tab, LF and CR, plus #xFFFE and #xFFFF.

It also notes that lone surrogates are outside Char but are not matched. to_ical() rejects them, but Event with SUMMARY "a\ud800" currently gives <text>a&#55296;</text> from to_xcal(), which isn't well-formed either. I can add \ud800-\udfff to the pattern here if you want it covered in this PR, or leave it for a separate one.

niccokunzmann

Technically fine. Let's have another docs review.

alhudz · 2026-10-29T06:00:44Z

#1966 added news/1888.bugfix on main, which conflicted with the entry here, so I've moved this one to news/1888.bugfix.1 and merged main in. That also brings in the Timezone.Timezone forward-reference fix the Read the Docs build was failing on, so there should be a preview for the docs review now. The code diff is unchanged, and the full suite and the docs build with -W pass locally.

@niccokunzmann the push dismissed your approval.

stevepiercy

Thanks for your contribution, @alhuda! I've made a few suggestions. Would you please take care? Thank you!

stevepiercy · 2026-11-09T20:18:10Z

@@ -0,0 +1 @@
+Fixed :meth:`vCalAddress.to_xcal() <icalendar.prop.cal_address.vCalAddress.to_xcal>` writing characters that are not allowed in XML, such as a C0 control character in an ``ORGANIZER`` or ``SENT-BY`` value parsed from jCal, which made :meth:`Component.to_xcal() <icalendar.cal.component.Component.to_xcal>` produce XML that is not well-formed. The value now goes through the same filter as the text and URI types, and that filter also removes U+FFFE and U+FFFF. I used AI (Claude Opus 5.5, via Claude Code) to find this bug, write the fix, and write its tests; I reviewed and validated the output. @alhudz


Be concise and avoid run-on sentences from AI. Also use inline literals for characters.

Suggested change

Fixed :meth:`vCalAddress.to_xcal() <icalendar.prop.cal_address.vCalAddress.to_xcal>` writing characters that are not allowed in XML, such as a C0 control character in an ``ORGANIZER`` or ``SENT-BY`` value parsed from jCal, which made :meth:`Component.to_xcal() <icalendar.cal.component.Component.to_xcal>` produce XML that is not well-formed. The value now goes through the same filter as the text and URI types, and that filter also removes U+FFFE and U+FFFF. I used AI (Claude Opus 5.5, via Claude Code) to find this bug, write the fix, and write its tests; I reviewed and validated the output. @alhudz

Fixed :meth:`vCalAddress.to_xcal() <icalendar.prop.cal_address.vCalAddress.to_xcal>` from writing characters that are disallowed in XML. The value now goes through the same filter as the text and URI types, and that filter also removes ``U+FFFE`` and ``U+FFFF``. I used AI (Claude Opus 5.5, via Claude Code) to find this bug, write the fix, and write its tests; I reviewed and validated the output. @alhudz

Applied in 677cbc4.

stevepiercy · 2026-11-09T20:39:07Z

+"""Match the characters that XML 1.0 does not allow in a document.
+
+The ``Char`` production in section 2.2 of XML 1.0 allows only
+``#x9 | #xA | #xD | [#x20-#xD7FF] | [#xE000-#xFFFD] | [#x10000-#x10FFFF]``.
+This pattern matches the characters outside of it that a Python :class:`str`
+can contain, apart from surrogates:
+
+- the C0 control characters ``#x0`` to ``#x1F``, except tab, line feed and
+  carriage return, and
+- the noncharacters ``#xFFFE`` and ``#xFFFF``.
+
+Lone surrogates ``#xD800`` to ``#xDFFF`` are also outside ``Char``, but are not
+matched here. They cannot be encoded as UTF-8, and ``to_ical()`` rejects them.
+
+.. seealso::
+
+    `XML 1.0, section 2.2 Characters <https://www.w3.org/TR/REC-xml/#charsets>`_
+"""


Reword to improve readability and merge seealso. Also Char is not an inline literal.

Suggested change

"""Match the characters that XML 1.0 does not allow in a document.

The ``Char`` production in section 2.2 of XML 1.0 allows only

``#x9 | #xA | #xD | [#x20-#xD7FF] | [#xE000-#xFFFD] | [#x10000-#x10FFFF]``.

This pattern matches the characters outside of it that a Python :class:`str`

can contain, apart from surrogates:

- the C0 control characters ``#x0`` to ``#x1F``, except tab, line feed and

carriage return, and

- the noncharacters ``#xFFFE`` and ``#xFFFF``.

Lone surrogates ``#xD800`` to ``#xDFFF`` are also outside ``Char``, but are not

matched here. They cannot be encoded as UTF-8, and ``to_ical()`` rejects them.

.. seealso::

`XML 1.0, section 2.2 Characters <https://www.w3.org/TR/REC-xml/#charsets>`_

"""

"""Match the characters that XML 1.0 disallows in a document.

`XML 1.0, section 2.2, Characters <https://www.w3.org/TR/REC-xml/#charsets>`_,

or Char, allows only ``#x9 | #xA | #xD | [#x20-#xD7FF] | [#xE000-#xFFFD] |

[#x10000-#x10FFFF]``. This pattern matches the characters outside of it that a

Python string can contain, apart from surrogates:

- the ``C0`` control characters ``#x0`` to ``#x1F``, except tab, line feed and

carriage return, and

- the noncharacters ``#xFFFE`` and ``#xFFFF``.

Lone surrogates ``#xD800`` to ``#xDFFF`` are also outside Char, but are not

matched here. They can't be encoded as UTF-8, and ``to_ical()`` rejects them.

Applied in 677cbc4. The suggestion range included the closing """, so I kept that line as it was.

alhudz requested review from angatha, niccokunzmann and stevepiercy as code owners October 18, 2026 12:45

github-actions Bot added the ai-suspicion This contribution is possibly created with lots of AI help without enough human understanding. label Oct 18, 2026

niccokunzmann requested changes Oct 21, 2026

View reviewed changes

document and rename the invalid XML character pattern

dab0b71

niccokunzmann previously approved these changes Oct 22, 2026

View reviewed changes

niccokunzmann assigned stevepiercy Oct 22, 2026

alhudz added 2 commits October 29, 2026 11:29

move the change log entry to 1888.bugfix.1

a50c488

Merge branch 'main' into xcal-cal-address-invalid-chars

c11a1f4

alhudz dismissed niccokunzmann’s stale review via c11a1f4 October 29, 2026 06:00

stevepiercy requested changes Nov 9, 2026

View reviewed changes

apply docs review suggestions to the change log entry and docstring

677cbc4



		_VALID_XML_CHARACTERS = re.compile(r"[\x00-\x08\x0b\x0c\x0e-\x1f]")
		_VALID_XML_CHARACTERS = re.compile(r"[\x00-\x08\x0b\x0c\x0e-\x1f\ufffe\uffff]")

		@@ -0,0 +1 @@
		Fixed :meth:`vCalAddress.to_xcal() <icalendar.prop.cal_address.vCalAddress.to_xcal>` writing characters that are not allowed in XML, such as a C0 control character in an ``ORGANIZER`` or ``SENT-BY`` value parsed from jCal, which made :meth:`Component.to_xcal() <icalendar.cal.component.Component.to_xcal>` produce XML that is not well-formed. The value now goes through the same filter as the text and URI types, and that filter also removes U+FFFE and U+FFFF. I used AI (Claude Opus 5.5, via Claude Code) to find this bug, write the fix, and write its tests; I reviewed and validated the output. @alhudz

Conversation

alhudz commented Oct 18, 2026

Linked issue

Description

Checklist

Additional information

Uh oh!

github-actions Bot commented Oct 18, 2026

Uh oh!

github-actions Bot commented Oct 18, 2026

Uh oh!

alhudz commented Oct 21, 2026

Uh oh!

niccokunzmann left a comment

Choose a reason for hiding this comment

Uh oh!

niccokunzmann Oct 21, 2026

Choose a reason for hiding this comment

Uh oh!

alhudz Oct 22, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

niccokunzmann left a comment

Choose a reason for hiding this comment

Uh oh!

alhudz commented Oct 29, 2026

Uh oh!

stevepiercy left a comment

Choose a reason for hiding this comment

Uh oh!

stevepiercy Nov 9, 2026

Choose a reason for hiding this comment

Uh oh!

alhudz Nov 13, 2026

Choose a reason for hiding this comment

Uh oh!

stevepiercy Nov 9, 2026

Choose a reason for hiding this comment

Uh oh!

alhudz Nov 13, 2026

Choose a reason for hiding this comment

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants