dav: DELETE removes a card
The privilege set said unbind, the router said no,
so the phone deleted local and the server did not know.
One route, one guard on If-Match, and the card is gone —
the change log tells the next sync what it is syncing on.
Assisted-by: Claude Opus 5 via Claude Code
diff --git a/lib/pro_tacts/web.rb b/lib/pro_tacts/web.rb
index 17bb878..401ca67 100644
--- a/lib/pro_tacts/web.rb
+++ b/lib/pro_tacts/web.rb
@@ -298,7 +298,7 @@ module ProTacts
# support (RFC 6352 section 6.1); the header is RFC 4918 section 10.1.
r.options do
response["DAV"] = "addressbook"
- response["Allow"] = "OPTIONS, PROPFIND, REPORT, PUT"
+ response["Allow"] = "OPTIONS, PROPFIND, REPORT, PUT, DELETE"
""
end
@@ -364,9 +364,12 @@ module ProTacts
# all — the PUT they prompted was what log/unhandled
# captured them for. DAV:write covers PUT and PROPPATCH
# (RFC 3744 section 3.2), and PUT is the one of the pair
- # answered; DAV:bind is adding a member to the collection
- # (section 3.9) and DAV:unbind removing one (section 3.10),
- # neither of which has a route yet.
+ # answered; DAV:unbind is removing a member from the
+ # collection (section 3.10), which DELETE answers.
+ # DAV:bind is adding one (section 3.9), and the create it
+ # names — POST to the collection with DAV:add-member — has
+ # no route: PUT to the member URI is how both clients
+ # create, so nothing has asked for it.
collection_response = <<~XML
<d:response>
<d:href>/dav/addressbook/</d:href>
@@ -545,6 +548,14 @@ module ProTacts
# same fall-through-to-404 the GET handler gives it.
write_card(id) if id.match?(Contact::ID_FORMAT)
end
+
+ # DELETE removes the card at the member URI (RFC 4918 section
+ # 9.6) — the DAV:unbind privilege the collection advertises,
+ # and what iOS sends when a contact is deleted on the phone.
+ r.delete String do |filename|
+ id = filename.delete_suffix(".vcf")
+ remove_card(id) if id.match?(Contact::ID_FORMAT)
+ end
end
end
end
@@ -675,6 +686,38 @@ module ProTacts
nil
end
+ # The whole of the DELETE, a private method for write_card's reason:
+ # a Roda route block cannot return early.
+ #: (String id) -> String?
+ def remove_card(id)
+ # An unmapped URI is a 404, not a silent success: RFC 4918 section
+ # 9.6 gives DELETE no idempotent status, and a client deleting what
+ # it believes exists is owed the disagreement. Falls through to the
+ # not_found handler the way the GET and PUT of a bad id do.
+ existing = store.contact(id)
+ return unless existing
+
+ # The lost-update conditional, RFC 7232 section 3.1, on the same
+ # terms the PUT gets it: a client that names the representation it
+ # means to remove must not remove one that changed underneath it.
+ # No recorded session has sent one on a DELETE — the iOS delete
+ # carried none, which section 3.1 leaves unconditional and
+ # allowed, and macOS has sent no DELETE at all. Answered anyway,
+ # because ignoring the header is the lost update it exists to
+ # refuse, and the PUT's own check is right here to reuse.
+ if_match = request.env["HTTP_IF_MATCH"]
+ return plain_412 if if_match && !if_match_satisfied?(if_match, existing)
+
+ # The change-log entry Store#delete leaves in the same transaction
+ # is what a syncing client is told: sync-collection answers a
+ # removed member as href plus 404 (#missing_response).
+ store.delete(id)
+ response.status = 204
+
+ # Bodyless for the 204, write_card's reason.
+ nil
+ end
+
# The whole of the edit POST, a private method for the same reason
# write_card is one: a Roda route block cannot return early, so each
# refusal is a value the block ends with rather than a branch it
diff --git a/sig/gems/roda.rbs b/sig/gems/roda.rbs
index 016ad48..0d28283 100644
--- a/sig/gems/roda.rbs
+++ b/sig/gems/roda.rbs
@@ -26,6 +26,7 @@ class Roda
def is: (*untyped matchers) { (*untyped) -> untyped } -> void
def on: (*untyped matchers) { (*untyped) -> untyped } -> void
+ def delete: (*untyped matchers) ?{ (*untyped) -> untyped } -> void
def get: (*untyped matchers) ?{ (*untyped) -> untyped } -> void
def options: (*untyped matchers) ?{ (*untyped) -> untyped } -> void
def post: (*untyped matchers) ?{ (*untyped) -> untyped } -> void
diff --git a/test/fixtures/macos-exchange/03-options-principal/response b/test/fixtures/macos-exchange/03-options-principal/response
index 6f2b25a..9c0aab3 100644
--- a/test/fixtures/macos-exchange/03-options-principal/response
+++ b/test/fixtures/macos-exchange/03-options-principal/response
@@ -1,5 +1,5 @@
200
Content-Type: text/html
DAV: addressbook
-Allow: OPTIONS, PROPFIND, REPORT, PUT
+Allow: OPTIONS, PROPFIND, REPORT, PUT, DELETE
diff --git a/test/pro_tacts/test_web.rb b/test/pro_tacts/test_web.rb
index 0efb48d..a2004e9 100644
--- a/test/pro_tacts/test_web.rb
+++ b/test/pro_tacts/test_web.rb
@@ -40,6 +40,7 @@ class WebTest < Minitest::Test
assert_equal "addressbook", last_response["DAV"]
assert_includes last_response["Allow"], "OPTIONS"
assert_includes last_response["Allow"], "PROPFIND"
+ assert_includes last_response["Allow"], "DELETE"
end
def test_propfind_root
@@ -513,6 +514,81 @@ class WebTest < Minitest::Test
end
end
+ ## Deletes
+
+ def test_delete_removes_the_card
+ with_contacts({"aiden" => "Aiden"}) do
+ delete "/dav/addressbook/aiden.vcf"
+
+ assert_equal 204, last_response.status
+ assert_nil last_response["Content-Type"]
+ assert_nil last_response["Content-Length"]
+
+ get "/dav/addressbook/aiden.vcf"
+ assert_equal 404, last_response.status
+ end
+ end
+
+ # RFC 6578 section 3.2: the removal a client syncs on is href plus
+ # 404, with no propstat, and the token has to move for it to arrive.
+ def test_delete_reaches_a_syncing_client_as_a_removal
+ with_contacts({"aiden" => "Aiden", "znorth" => "Zoe"}) do
+ request "/dav/addressbook/", method: "REPORT", input: sync_collection("")
+ token = last_response.body[%r{<d:sync-token>([^<]+)</d:sync-token>}, 1]
+
+ delete "/dav/addressbook/aiden.vcf"
+
+ request "/dav/addressbook/", method: "REPORT", input: sync_collection(token)
+
+ assert_equal 207, last_response.status
+ assert_includes last_response.body, "<d:href>/dav/addressbook/aiden.vcf</d:href>"
+ assert_includes last_response.body, "<d:status>HTTP/1.1 404 Not Found</d:status>"
+ refute_includes last_response.body, "znorth"
+ refute_equal token, last_response.body[%r{<d:sync-token>([^<]+)</d:sync-token>}, 1]
+ end
+ end
+
+ def test_delete_of_a_card_that_is_not_there_is_a_404
+ with_contacts({}) do
+ delete "/dav/addressbook/nobody.vcf"
+
+ assert_equal 404, last_response.status
+ end
+ end
+
+ def test_delete_of_an_id_this_server_cannot_have_is_a_404
+ with_contacts({}) do
+ delete "/dav/addressbook/not.an.id.vcf"
+
+ assert_equal 404, last_response.status
+ end
+ end
+
+ # RFC 7232 section 3.1, the same conditional the PUT honors.
+ def test_delete_with_the_current_etag_removes_the_card
+ with_contacts({"aiden" => "Aiden"}) do
+ get "/dav/addressbook/aiden.vcf"
+
+ delete "/dav/addressbook/aiden.vcf", {}, "HTTP_IF_MATCH" => last_response["ETag"]
+
+ assert_equal 204, last_response.status
+ end
+ end
+
+ def test_delete_with_a_stale_etag_is_refused_and_changes_nothing
+ with_contacts({"aiden" => "Aiden"}) do
+ get "/dav/addressbook/aiden.vcf"
+ before = last_response.body
+
+ delete "/dav/addressbook/aiden.vcf", {}, "HTTP_IF_MATCH" => %("#{"0" * 64}")
+
+ assert_equal 412, last_response.status
+
+ get "/dav/addressbook/aiden.vcf"
+ assert_equal before, last_response.body
+ end
+ end
+
## Pictures
# The sizing case on the wire: a 338 KB PUT — the client does not