Skip to content

fix(clone): a cloned document is the root of its own nodes - #792

Open
chibenwa wants to merge 1 commit into
sabre-io:masterfrom
linagora:upstream-fix-document-clone-root
Open

chibenwa wants to merge 1 commit into
sabre-io:masterfrom
linagora:upstream-fix-document-clone-root

Conversation

@chibenwa

Copy link
Copy Markdown

clone $document produced a copy whose nodes (components, properties and parameters) still had the ORIGINAL document as their root:

  • Document had no __clone(), so the copy's own root still pointed to the original (the constructor sets it to itself);
  • Component::__clone() gives each cloned child $this->root, i.e. the original, and grand-children are cloned before their parent's root is fixed;
  • Property::__clone() re-parents the cloned parameters but never sets their root.

Anything that goes through the root was then resolved against the original. In particular DateTime::getDateTimes() resolves a TZID with TimeZoneUtil::getTimeZone($tzid, $this->root): once the original is destroyed (Sabre does it at the end of validateICalendar()) or rewritten, the VTIMEZONE of the copy is not found anymore, and a non-IANA TZID (e.g. TZID=Custom Eastern with X-LIC-LOCATION:America/New_York) silently falls back to the default PHP timezone. IANA TZIDs were not affected since they resolve without a VTIMEZONE.

Document::__clone() now publishes the copy under construction for the duration of the deep copy, and every cloned component/property/parameter takes it as its root. This adds no traversal: a clone costs the same as before. Cloning a sub-component or a property (to add it back to the same document) still keeps the original document as root.

`clone $document` produced a copy whose nodes (components, properties and
parameters) still had the ORIGINAL document as their root:
- Document had no __clone(), so the copy's own root still pointed to the
  original (the constructor sets it to itself);
- Component::__clone() gives each cloned child `$this->root`, i.e. the
  original, and grand-children are cloned before their parent's root is
  fixed;
- Property::__clone() re-parents the cloned parameters but never sets their
  root.

Anything that goes through the root was then resolved against the original.
In particular DateTime::getDateTimes() resolves a TZID with
TimeZoneUtil::getTimeZone($tzid, $this->root): once the original is
destroyed (Sabre does it at the end of validateICalendar()) or rewritten,
the VTIMEZONE of the copy is not found anymore, and a non-IANA TZID
(e.g. `TZID=Custom Eastern` with `X-LIC-LOCATION:America/New_York`)
silently falls back to the default PHP timezone. IANA TZIDs were not
affected since they resolve without a VTIMEZONE.

Document::__clone() now publishes the copy under construction for the
duration of the deep copy, and every cloned component/property/parameter
takes it as its root. This adds no traversal: a clone costs the same as
before. Cloning a sub-component or a property (to add it back to the same
document) still keeps the original document as root.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants