From 8158eaa1ea0d9443b9360140ac81d77558c03e46 Mon Sep 17 00:00:00 2001 From: Andrew McMillan Date: Mon, 16 Apr 2012 12:57:16 +1200 Subject: [PATCH] Changes to the way PROPPATCH returns errors. This is cleaner and perhaps a little more informative. --- inc/caldav-PROPPATCH.php | 108 +++++++----------- .../binding/1011-PROPPATCH-bound-fail.result | 10 +- .../0840-Spec-PROPPATCH-1.result | 13 +-- .../0842-Spec-PROPPATCH-3.result | 13 +-- .../0845-Spec-PROPPATCH-both-fail.result | 11 +- 5 files changed, 59 insertions(+), 96 deletions(-) diff --git a/inc/caldav-PROPPATCH.php b/inc/caldav-PROPPATCH.php index df8b95d0..585181fc 100644 --- a/inc/caldav-PROPPATCH.php +++ b/inc/caldav-PROPPATCH.php @@ -45,6 +45,30 @@ $rmprops = $xmltree->GetPath("/DAV::propertyupdate/DAV::remove/DAV::prop/*"); $failure = array(); $success = array(); + +/** + * Small utility function to add propstat for one failure + * @param unknown_type $tag + * @param unknown_type $status + * @param unknown_type $description + * @param unknown_type $error_tag + */ +function add_failure( $type, $tag, $status, $description=null, $error_tag = null) { + global $failure; + $propstat = array( + new XMLElement( 'prop', new XMLElement($tag)), + new XMLElement( 'status', $status ) + ); + + if ( isset($description)) + $propstat[] = new XMLElement( 'responsedescription', $description ); + if ( isset($error_tag) ) + $propstat[] = new XMLElement( 'error', new XMLElement( $error_tag ) ); + + $failure[$type.'-'.$tag] = new XMLElement('propstat', $propstat ); +} + + /** * Not much for it but to process the incoming settings in a big loop, doing * the special-case stuff as needed and falling through to a default which @@ -79,15 +103,8 @@ foreach( $setprops AS $k => $setting ) { $success[$tag] = 1; } else { - $failure['set-'.$tag] = new XMLElement( 'propstat', array( - new XMLElement( 'prop', new XMLElement($tag)), - new XMLElement( 'status', 'HTTP/1.1 403 Forbidden' ), - new XMLElement( 'responsedescription', array( - new XMLElement( 'error', new XMLElement( 'cannot-modify-protected-property') ), - translate("The displayname may only be set on collections, principals or bindings.") ) - ) - - )); + add_failure('set', $tag, 'HTTP/1.1 403 Forbidden', + translate("The displayname may only be set on collections, principals or bindings."), 'cannot-modify-protected-property'); } break; @@ -109,14 +126,8 @@ foreach( $setprops AS $k => $setting ) { $success[$tag] = 1; } else { - $failure['set-'.$tag] = new XMLElement( 'propstat', array( - new XMLElement( 'prop', new XMLElement($tag)), - new XMLElement( 'status', 'HTTP/1.1 403 Forbidden' ), - new XMLElement( 'responsedescription', array( - new XMLElement( 'error', new XMLElement( 'cannot-modify-protected-property') ), - translate("Resources may not be changed to / from collections.") ) - ) - )); + add_failure('set', $tag, 'HTTP/1.1 403 Forbidden', + translate("Resources may not be changed to / from collections."), 'cannot-modify-protected-property'); } break; @@ -129,23 +140,14 @@ foreach( $setprops AS $k => $setting ) { $success[$tag] = 1; } else { - $failure['set-'.$tag] = new XMLElement( 'propstat', array( - new XMLElement( 'prop', new XMLElement($tag)), - new XMLElement( 'status', 'HTTP/1.1 403 Forbidden' ), - new XMLElement( 'responsedescription', array( - new XMLElement( 'error', new XMLElement( 'cannot-modify-protected-property') ), - translate("The CalDAV:schedule-calendar-transp property may only be set on calendars.") ) - ) - )); + add_failure('set', $tag, 'HTTP/1.1 409 Conflict', + translate("The CalDAV:schedule-calendar-transp property may only be set on calendars.")); } break; case 'urn:ietf:params:xml:ns:caldav:calendar-free-busy-set': - $failure['set-'.$tag] = new XMLElement( 'propstat', array( - new XMLElement( 'prop', new XMLElement($tag)), - new XMLElement( 'status', 'HTTP/1.1 409 Conflict' ), - new XMLElement( 'responsedescription', translate("The calendar-free-busy-set is superseded by the schedule-transp property of a calendar collection.") ) - )); + add_failure('set', $tag, 'HTTP/1.1 409 Conflict', + translate("The calendar-free-busy-set is superseded by the schedule-calendar-transp property of a calendar collection.") ); break; case 'urn:ietf:params:xml:ns:caldav:calendar-timezone': @@ -169,14 +171,7 @@ foreach( $setprops AS $k => $setting ) { array( ':tzid' => $tzid, ':dav_name' => $dav_resource->dav_name()) ); } else { - $failure['set-'.$tag] = new XMLElement( 'propstat', array( - new XMLElement( 'prop', new XMLElement($tag)), - new XMLElement( 'status', 'HTTP/1.1 403 Forbidden' ), - new XMLElement( 'responsedescription', array( - new XMLElement( 'error', new XMLElement( 'cannot-modify-protected-property') ), - translate("calendar-timezone property is only valid for a calendar.") ) - ) - )); + add_failure('set', $tag, 'HTTP/1.1 409 Conflict', translate("calendar-timezone property is only valid for a calendar.")); } break; @@ -196,14 +191,7 @@ foreach( $setprops AS $k => $setting ) { case 'DAV::creationdate': case 'DAV::lockdiscovery': case 'DAV::supportedlock': - $failure['set-'.$tag] = new XMLElement( 'propstat', array( - new XMLElement( 'prop', new XMLElement($tag)), - new XMLElement( 'status', 'HTTP/1.1 403 Forbidden' ), - new XMLElement( 'responsedescription', array( - new XMLElement( 'error', new XMLElement( 'cannot-modify-protected-property') ), - translate("Property is read-only") ) - ) - )); + add_failure('set', $tag, 'HTTP/1.1 409 Conflict', translate("Property is read-only"), new XMLElement( 'cannot-modify-protected-property')); break; /** @@ -217,8 +205,6 @@ foreach( $setprops AS $k => $setting ) { } } - - foreach( $rmprops AS $k => $setting ) { $tag = $setting->GetTag(); $content = $setting->RenderContent(); @@ -226,14 +212,8 @@ foreach( $rmprops AS $k => $setting ) { switch( $tag ) { case 'DAV::resourcetype': - $failure['rm-'.$tag] = new XMLElement( 'propstat', array( - new XMLElement( 'prop', new XMLElement($tag)), - new XMLElement( 'status', 'HTTP/1.1 403 Forbidden' ), - new XMLElement( 'responsedescription', array( - new XMLElement( 'error', new XMLElement( 'cannot-modify-protected-property') ), - translate("DAV::resourcetype may only be set to a new value, it may not be removed.") ) - ) - )); + add_failure('rm', $tag, 'HTTP/1.1 409 Conflict', + translate("DAV::resourcetype may only be set to a new value, it may not be removed."), 'cannot-modify-protected-property'); break; case 'urn:ietf:params:xml:ns:caldav:calendar-timezone': @@ -241,14 +221,8 @@ foreach( $rmprops AS $k => $setting ) { $qry->QDo('UPDATE collection SET timezone = NULL WHERE dav_name = :dav_name', array( ':dav_name' => $dav_resource->dav_name()) ); } else { - $failure['set-'.$tag] = new XMLElement( 'propstat', array( - new XMLElement( 'prop', new XMLElement($tag)), - new XMLElement( 'status', 'HTTP/1.1 403 Forbidden' ), - new XMLElement( 'responsedescription', array( - new XMLElement( 'error', new XMLElement( 'cannot-modify-protected-property') ), - translate("calendar-timezone property is only valid for a calendar.") ) - ) - )); + add_failure('rm', $tag, 'HTTP/1.1 409 Conflict', + translate("calendar-timezone property is only valid for a calendar."), 'cannot-modify-protected-property'); } break; @@ -269,11 +243,7 @@ foreach( $rmprops AS $k => $setting ) { case 'DAV::displayname': case 'DAV::lockdiscovery': case 'DAV::supportedlock': - $failure['rm-'.$tag] = new XMLElement( 'propstat', array( - new XMLElement( 'prop', new XMLElement($tag)), - new XMLElement( 'status', 'HTTP/1.1 409 Conflict' ), - new XMLElement('responsedescription', translate("Property is read-only") ) - )); + add_failure('rm', $tag, 'HTTP/1.1 409 Conflict', translate("Property is read-only")); dbg_error_log( 'PROPPATCH', ' RMProperty %s is read only and cannot be removed', $tag); break; diff --git a/testing/tests/binding/1011-PROPPATCH-bound-fail.result b/testing/tests/binding/1011-PROPPATCH-bound-fail.result index 09f47035..f7474e0a 100644 --- a/testing/tests/binding/1011-PROPPATCH-bound-fail.result +++ b/testing/tests/binding/1011-PROPPATCH-bound-fail.result @@ -2,7 +2,7 @@ HTTP/1.1 207 Multi-Status Date: Dow, 01 Jan 2000 00:00:00 GMT DAV: 1, 2, 3, access-control, calendar-access, calendar-schedule DAV: extended-mkcol, bind, addressbook, calendar-auto-schedule, calendar-proxy -Content-Length: 623 +Content-Length: 608 Content-Type: text/xml; charset="utf-8" @@ -13,12 +13,8 @@ Content-Type: text/xml; charset="utf-8" - HTTP/1.1 403 Forbidden - - - - - + HTTP/1.1 409 Conflict + calendar-timezone property is only valid for a calendar. diff --git a/testing/tests/regression-suite/0840-Spec-PROPPATCH-1.result b/testing/tests/regression-suite/0840-Spec-PROPPATCH-1.result index 52763b93..d35ad449 100644 --- a/testing/tests/regression-suite/0840-Spec-PROPPATCH-1.result +++ b/testing/tests/regression-suite/0840-Spec-PROPPATCH-1.result @@ -2,7 +2,7 @@ HTTP/1.1 207 Multi-Status Date: Dow, 01 Jan 2000 00:00:00 GMT DAV: 1, 2, 3, access-control, calendar-access, calendar-schedule DAV: extended-mkcol, bind, addressbook, calendar-auto-schedule, calendar-proxy -Content-Length: 742 +Content-Length: 806 Content-Type: text/xml; charset="utf-8" @@ -13,12 +13,11 @@ Content-Type: text/xml; charset="utf-8" - HTTP/1.1 403 Forbidden - - - - - + HTTP/1.1 409 Conflict + DAV::resourcetype may only be set to a new value, it may not be removed. + + + diff --git a/testing/tests/regression-suite/0842-Spec-PROPPATCH-3.result b/testing/tests/regression-suite/0842-Spec-PROPPATCH-3.result index adc0eca9..2f2b9638 100644 --- a/testing/tests/regression-suite/0842-Spec-PROPPATCH-3.result +++ b/testing/tests/regression-suite/0842-Spec-PROPPATCH-3.result @@ -2,7 +2,7 @@ HTTP/1.1 207 Multi-Status Date: Dow, 01 Jan 2000 00:00:00 GMT DAV: 1, 2, 3, access-control, calendar-access, calendar-schedule DAV: extended-mkcol, bind, addressbook, calendar-auto-schedule, calendar-proxy -Content-Length: 469 +Content-Length: 533 Content-Type: text/xml; charset="utf-8" @@ -13,12 +13,11 @@ Content-Type: text/xml; charset="utf-8" - HTTP/1.1 403 Forbidden - - - - - + HTTP/1.1 409 Conflict + DAV::resourcetype may only be set to a new value, it may not be removed. + + + Some properties were not able to be changed. diff --git a/testing/tests/regression-suite/0845-Spec-PROPPATCH-both-fail.result b/testing/tests/regression-suite/0845-Spec-PROPPATCH-both-fail.result index 59d0553a..dbc6c8d2 100644 --- a/testing/tests/regression-suite/0845-Spec-PROPPATCH-both-fail.result +++ b/testing/tests/regression-suite/0845-Spec-PROPPATCH-both-fail.result @@ -2,7 +2,7 @@ HTTP/1.1 207 Multi-Status Date: Dow, 01 Jan 2000 00:00:00 GMT DAV: 1, 2, 3, access-control, calendar-access, calendar-schedule DAV: extended-mkcol, bind, addressbook, calendar-auto-schedule, calendar-proxy -Content-Length: 469 +Content-Length: 513 Content-Type: text/xml; charset="utf-8" @@ -14,11 +14,10 @@ Content-Type: text/xml; charset="utf-8" HTTP/1.1 403 Forbidden - - - - - + Resources may not be changed to / from collections. + + + Some properties were not able to be changed.