sabre-io / dav

sabre/dav is a CalDAV, CardDAV and WebDAV framework for PHP
http://sabre.io
BSD 3-Clause "New" or "Revised" License
1.54k stars 347 forks source link

Proppatch logic doesn't allow updating and removing nested properties simultaneously #1565

Closed skjnldsv closed 1 month ago

skjnldsv commented 1 month ago

Propatch example

Reference: http://www.webdav.org/specs/rfc2518.html#rfc.section.8.2.2

<?xml version="1.0" encoding="utf-8" ?>
<D:propertyupdate
    xmlns:D="DAV:"
    xmlns:Z="http://www.w3.com/standards/z39.50/">
    <D:set>
        <D:prop>
            <Z:authors>
                <Z:Author>Jim Whitehead</Z:Author>
                <Z:Author>Roy Fielding</Z:Author>
            </Z:authors>
        </D:prop>
    </D:set>
    <D:remove>
        <D:prop>
            <Z:authors>
                <Z:Author>John Patson</Z:Author>
            </Z:authors>
        </D:prop>
    </D:remove>
</D:propertyupdate>

Issue

When executing this proppatch request, the property gets nullified by the way we handle proppatch in sabre/dav. https://github.com/sabre-io/dav/blob/a6ff5932012d5064b4c04b4543be7c3d119a5199/lib/DAV/Xml/Request/PropPatch.php#L95-L105

Suggestion

I suggest creating a new array for removed properties

// Properties being set
$propPatch->properties = [];

// Properties being removed
$propPatch->removedProperties = [];

We can keep the legacy way of nullifying, but stop nullifying if we have the same root prop in both d:set and d:remove

skjnldsv commented 1 month ago

Hmmmmm, I might have misunderstood the rfc as @susnux pointed me out.

I think the rfc never covered nested properties updates. It's all replace or delete. To Achieve what I want with the example in the OP, the <D:set> would be enough as it would overwrite any existing content, kicking <Z:Author>John Patson</Z:Author> out already :thinking:

skjnldsv commented 1 month ago

https://datatracker.ietf.org/doc/html/rfc4918#section-14.26

Description: The 'set' element MUST contain only a 'prop' element. The elements contained by the 'prop' element inside the 'set' element MUST specify the name and value of properties that are set on the resource identified by Request-URI. If a property already exists, then its value is replaced. Language tagging information appearing in the scope of the 'prop' element (in the "xml:lang"

Important part: If a property already exists, then its value is replaced