Repository navigation
fix: repeat a redirected PUT with its body - #837
Merged
Merged
Conversation
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
niquests and requests follow a 302 by sending a GET without the body, and httpx follows nothing. Zimbra redirects a PUT under a name other than <uid>.ics to <uid>.ics and stores nothing, so the data was lost without an error. A request other than GET/HEAD answered 301, 302, 307 or 308 is now repeated at the Location with method and body, on the same host, port and scheme or an http-to-https upgrade. save() and calendar creation move the url there; a 303 to a PUT counts as stored at the Location; save() raises PutError on a redirect not followed. Prompt: Look into [zimbra claims in docs/TODO.md in caldav-server-tester]`. There is something fishy there. We do have an external zimbra server which tests can be run towards - work-tests Followup-Prompt: Fix the probe, fix the caldav bug ([the caldav library], we'll do it directly to master this time), what should be unsupported you said? How to deal with the wrong port issue? We don't want a broken docker setup pollute our zimbra gradings. Prompts (comments made inline in the document while reviewing it): > (on "A 303 answer to a PUT now raises PutError") A 303 probably indicates that the URL of the object has changed? E.g. `my_unicorn_calendar/my_unicorn_ical.ics` -> `calendar-04f2c005-d51d-405d-a148-d9e531d0c16c/12345678-1234-5678-9abc-12345678.ics Assisted-By: Claude Opus 5.5 Reviewed-by: Tobias Brox <tobias@redpill-linpro.com>
The Zimbra profile said Zimbra keeps the name an object was stored under. It does for <uid>.ics only: a PUT under another name is redirected to <uid>.ics, so save-load.stable-url gets forces_uid_ics, and the feature description names Zimbra as such a server. Measured with caldav-server-tester against a production Zimbra and the docker image, which now agree. Prompt: Look into `sed -n '375,405p' [docs/TODO.md in caldav-server-tester]`. There is something fishy there. We do have an external zimbra server which tests can be run towards - work-tests Followup-Prompt: Fix the probe, fix the caldav bug ([the caldav library], we'll do it directly to master this time), what should be unsupported you said? How to deal with the wrong port issue? We don't want a broken docker setup pollute our zimbra gradings. Prompts (comments made inline in the document while reviewing it): > (on "Zimbra hint and the stable-url description disagree") This sounds like a doc fix? For changing the grades, it's needed to run the caldav-server-tester Assisted-By: Claude Opus 5.5 Reviewed-by: Tobias Brox <tobias@redpill-linpro.com>
tobixen
force-pushed
the
fix/redirected-put
branch
from
October 29, 2026 01:32
18d9a88 to
c21fb09
Compare
Code scanning flagged `calls` as unused in three of the new redirect tests. Rather than discard it, assert on it: a followed redirect makes two requests, a refused one only the first, a 307-308 chain three. Prompt: please fix and rebase [the failing test and the three unused-variable alerts on #837] Assisted-By: Claude Opus 5.5 Reviewed-by: Tobias Brox <tobias@redpill-linpro.com>
An object or calendar that moved to a redirect's Location got a bare URL, dropping the alias_at and host_aliases its old URL carried, so later joins and @-quoting on it could behave differently. Relocate the old URL instead, for 301/302/307/308 on PUT and MKCALENDAR and for a 303 to a PUT; on a redirect onto a host alias, the old host becomes the alias. Prompt: [fix issues found during the review process] Assisted-By: Claude Opus 5.5 Reviewed-by: Tobias Brox <tobias@redpill-linpro.com>
| ``host_aliases``, as every URL derived from its old one does.""" | ||
| base = "https://cal.example.com/cal/" | ||
| for status in (302, 303): | ||
| mocked.side_effect, calls = self._routes( |
tobixen
added a commit
to python-caldav/caldav-server-tester
that referenced
this pull request
Oct 30, 2026
caldav 3.7.0 has forces_uid_ics and the repeated PUT for Zimbra (python-caldav/caldav#837), so the separate note on them goes. With the PRs merged side by side, a bullet saying "the same" no longer follows the entry it meant, and the calendaring-sync credit read as covering every Added entry. Prompt: Caldav 3.7.0 is released, so we should try to merge all the pull requests. Prepare a "train". Assisted-By: Claude Opus 5.5 <noreply@anthropic.com> Reviewed-by: Tobias Brox <tobias@redpill-linpro.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Disclaimer: the code and the text below the line are AI-generated, by Claude Opus 5.5, from the prompts:
When Zimbra gets a PUT under any name other than
<uid>.ics, it answers302with aLocationpointing at<uid>.ics, and it stores nothing. niquests/requests follow a 302 by sending a GET without the body, which returns 404. The data was lost andput()raised no error. With httpx,save()even reported success and moved the object'surlto the Location.This PR changes how redirects are handled for requests other than GET and HEAD:
save()and calendar creation move the object'surlto where the request ended up.save()raisePutErrorinstead of reporting success.The Zimbra profile gets
forces_uid_ics: Trueonsave-load.stable-url. Its comment no longer claims that Zimbra keeps the name an object was stored under.Tested against a production Zimbra server and the docker image. caldav-server-tester grades both servers the same way:
save-load.stable-url: unsupported,forces_uid_ics: true. The matching probe change in caldav-server-tester will follow in a separate PR.