Open
Bug 605201
Opened 14 years ago
Updated 3 years ago
Updating an event results in a PUT to the wrong CalDav URL
Categories
(Calendar :: Provider: CalDAV, defect)
Tracking
(Not tracked)
NEW
People
(Reporter: jdc, Unassigned)
References
Details
(Whiteboard: [CalDAV server: DAViCal][calconnect31-haspatch])
Attachments
(2 files, 2 obsolete files)
3.08 KB,
patch
|
Fallen
:
review+
|
Details | Diff | Splinter Review |
2.68 KB,
patch
|
Fallen
:
review+
|
Details | Diff | Splinter Review |
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US) AppleWebKit/534.7 (KHTML, like Gecko) Chrome/7.0.517.41 Safari/534.7 Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2.9) Gecko/20100915 Lightning/1.0b2 Thunderbird/3.1.4 I can add new events to my CalDav calendars, but when I try to edit or delete them, the operation fails with a MODIFICATION_FAILED error code. The calendar I am subscribed to is at /user/calname, but looking at the backend webserver logs (Davical on Apache), I see that the PUT operation is happening at /user/calname/calname/some-id-here. (the calname part is doubled). The real davical URL for the calendar is /caldav.php/user/calname If I use this URL, everything works, and the backend web logs indicate the PUT request is being sent to /caldav.php/user/calname/some-id-here as expected. I've setup the URL rewrites (suggested as part of the davical install) to get the friendlier URLs, and it's only when I use these URLs that I encounter the problem. Subscribing to the calendar and adding new events works as expected. Reproducible: Always Steps to Reproduce: 1. Subscribe to a calendar at http://server/user/calname 2. Add a new event 3. Delete or modify the event Actual Results: MODIFICATION_FAILED error is indicated. Looking at the backend web logs, I see that the PUT request happened on the /user/calname/calname/some-id-here URL Expected Results: Event should be modified as requested. Looking at the backend web logs, I would expect the PUT request to happen on the /user/calname/some-id-here URL.
Reporter | ||
Comment 1•14 years ago
|
||
The situation described above works as expected in Evolution, suggesting it's probably not a problem with the DAViCal server or the rewritten URLs themselves.
Reporter | ||
Updated•14 years ago
|
OS: Windows XP → All
Comment 2•13 years ago
|
||
Jonathan could you please provide the URL rewrites you have setup in DAViCal.
Reporter | ||
Comment 3•13 years ago
|
||
RewriteEngine On RewriteCond %{REQUEST_URI} !^/$ RewriteCond %{REQUEST_URI} !\.(php|css|js|png|gif|jpg) RewriteRule ^(.*)$ /caldav.php/$1 [NC,L]
Comment 4•13 years ago
|
||
I've seen this kind of thing happening before, and I am pretty sure it is a configuration issue rather than a Lightning issue. If you set in your config that: $c->base_url = "/"; for example, it might work (though that's a hack rather than a correct solution). The fact that it works with Evolution is not necessarily an indication that Lightning is doing anything wrong. The two programmes take very different approaches to accessing CalDAV servers. Something that would be a data point is whether you have caching enabled on the calendar in Lightning. More information could also be found in examing the log files in Apache if you enable the following settings in DAViCal's configuration: $c->dbg['request'] = 1; $c->dbg['response'] = 1; This will log the full request / response process in DAViCal (including headers) and will probably show that DAViCal is the first thing showing those urls... Regards, Andrew McMillan.
Updated•13 years ago
|
Whiteboard: [CalDAV server: DAViCal]
Version: unspecified → Lightning 1.0b2
Comment 5•13 years ago
|
||
Confirming that this happens to me too. I have traced the bug to an overly naive "url cleaning" algorithm in calDavCalendar.js in combination with the fact that URL rewriting does not change URLs embedded in replies, only those in HTTP headers. I have created a patch against 1.0b2 for our own use, but it has not seen much testing, however I will attach it anyway as it might help (if it isn't broken).
Comment 6•13 years ago
|
||
This patch changes the URL cleanup/replacement code in function caldav_addTargetCalendarItem() in calDavCalendar.js, so it is less likely to mess up server returned URLs if their embedded base URL is longer/shorter than the user configured calendar URL. This patch is *very* untested! Note: I am not a regular Mozilla developer, and I do not have the setup to run the Mozilla test suite or create a test for it, the Bug report above and the comments in the code (including the unchanged comment just before the biggest part of the patch) should inspire some relevant tests.
Comment 7•13 years ago
|
||
Comment on attachment 532960 [details] [diff] [review] [after beta] Patch against 1.0b2 which might solve this Putting this patch in Philipp's review queue, so it doesn't get lost. ;)
Attachment #532960 -
Flags: review?(philipp)
Updated•13 years ago
|
Assignee: nobody → jb-mozilla
Comment 8•13 years ago
|
||
Note to Martin: As I said, I am not a regular Mozilla developer and I don't have the time nor training to set up my own Mercurial mirror of the entire Mozilla tree, create Mozilla-format testcase nor learn any of the other procedures to make my patch suitable for release. I am just a sophisticated end user who submitted a quick and dirty traditional unified-diff patch against a released file in the hope that it might help other affected users and/or help a real Mozilla Developer create a proper fix for inclusion in a later release. I am removing myself as assignee of the bug so a real Mozilla Developer can pick it up.
Updated•13 years ago
|
Assignee: jb-mozilla → nobody
Comment 9•13 years ago
|
||
I can confirm everything mentioned before. I am seeing this issue myself for about 11 months now. I also had discovered that everything is working fine in Evolution, but in Thunderbird, using the rewrite URL will produce errors when changing events. Read access is totally fine, and I also can create events. When I try to change something existing however, Thunderbird will report a MODIFICATION_FAILED. Looking at the error log reveals the following: "CalDAV: Unexpected status on modifying item on mycalendar home: 500" So the server is returning an error code for "internal server error", which - like Andrew said before - might suggest a server configuration error. Setting '$c->base_url = "/";' in the DAViCal server configuration does not change anything with respect to that error. I still need to test the patch provided above. Going to give feedback once I have done so.
Updated•13 years ago
|
Attachment #532960 -
Attachment description: Patch against 1.0b2 which might solve this → [after beta] Patch against 1.0b2 which might solve this
Comment 10•13 years ago
|
||
This patch replaces 532960 for users running 1.0b4 and for any devs trying to port this to trunk. I am keeping the old patch available as an end user workaround for users of 1.0b2, which (despite its beta status) is the shipping version for end users on Tb 3.x, while 1.0b4 is the shipping version for Tb 5.0. Once again this is a traditional UniDiff patch against componets/calDavCalendar.js in the shipping ligtning .xpi, as I do not have a personal Hg mirror of Mozilla's build environment.
Attachment #532960 -
Attachment is obsolete: true
Attachment #532960 -
Flags: review?(philipp)
Comment 11•13 years ago
|
||
Comment on attachment 548147 [details] [diff] [review] [after beta] Updated patch matching the 1.0b4 code Thanks for the patch. Unfortunately its too risky to take this for the current beta (due very soon), but we'll consider this afterwards. I think this is ok given it can be fixed by server config.
Attachment #548147 -
Attachment description: Updated patch matching the 1.0b4 code → [after beta] Updated patch matching the 1.0b4 code
Attachment #548147 -
Flags: review?(philipp)
Comment 12•13 years ago
|
||
Note: For the specific scenario of DaviCal with DaviCal's recommended Apache configuration, the only "server side" workaround is to remove the Apache rewrite rules, thus breaking all non-Lightning clients. There is however a client side workaround for this scenario: In the Lightning config, type in the URL as https://yourserver.example.com/cal/caldav.php/you@example.com/somecal, rather than the server-preferred https://yourserver.example.com/cal/you@example.com/somecal . This prevents Lightning from getting confused when temporary URLs sent back from the server included the "caldav.php/" part, but means you will have to reconfigure your client(s) if the server is upgraded to a different CalDav implementation.
Comment 13•12 years ago
|
||
Comment on attachment 548147 [details] [diff] [review] [after beta] Updated patch matching the 1.0b4 code Review of attachment 548147 [details] [diff] [review]: ----------------------------------------------------------------- Given this works for most server types, the patch looks fine. Could you whip up a patch with the below nits fixed? I'll do some testing in the meanwhile. ::: lightning.orig/components/calDavCalendar.js @@ +109,5 @@ > + * > + * @param s1 First string > + * @param s2 Second string > + */ > +function iseithersubstring(s1,s2) { I think we can make this an inline function where its used and a one-liner, i.e. function isEitherSubstring(s1, s2) (s1.length < s2.length ? s2.indexOf(s1) ? s1.indexOf(s2)) >= 0; I'd also prefer the casing as above, i.e isEitherSubstring instead of iseithersubstring. @@ +920,5 @@ > + // a valid path that just has additional redundant components, > + // like /dav/davical.php/user@example/Calendar > + // The original 1.0b4 code would incorrectly mangle this to > + // /dav/user@example/Calendar/Calendar and then get a failure back > + // from the server. Lets try to keep comments about old errors out of the code, the first paragraph is fine though. @@ +931,5 @@ > + x = uriPathComponents[uriPathComponentLength - 1]; > + } > + x = x.toLowerCase(); > + let j = resPathComponents.length - 2; > + //cal.LOG("Looking for '" + x + "' in '" + path + "'"); remove commented out lines, please
Attachment #548147 -
Flags: review?(philipp) → review+
Updated•12 years ago
|
Status: UNCONFIRMED → NEW
Ever confirmed: true
Whiteboard: [CalDAV server: DAViCal] → [needs updated patch] [CalDAV server: DAViCal]
Comment 14•12 years ago
|
||
Jakob Bohm removed himself from the subscribers list, so it's unlikely you'll get an improved patch from him, I guess. How has your testing been? I'm not so fond of javascript, but I tried to improve the patch with your comments, while still maintaining readability. I think having that renamed function inline is confusing, but feel free to adapt. I made a diff against the 1.5.1, which works fine for me. Making a similar note as Jakob Bohm: I'm not a mozilla developer nor do I have a Mozilla build/test environment here.
Comment 15•12 years ago
|
||
Someone told me it would be good to tell which version we're on: Thunderbird 13.01 and lightning 1.5.1.
Comment 16•12 years ago
|
||
Comment on attachment 637023 [details] [diff] [review] Updated patch against current stable Please, ask for review next time, so the patch does not stay unnoticed.
Attachment #637023 -
Flags: review?(philipp)
Updated•12 years ago
|
Assignee: nobody → kwadronaut+bugzilla.mozilla
Status: NEW → ASSIGNED
Updated•12 years ago
|
Hardware: x86 → All
Whiteboard: [needs updated patch] [CalDAV server: DAViCal] → [CalDAV server: DAViCal]
Updated•12 years ago
|
Version: Lightning 1.0b2 → Lightning 1.5
Comment 17•12 years ago
|
||
+1 experiencing this same issue on Win7, TBird 12.0, Lightning 1.4 (I don't see 1.5 available). Our calendar paths are getting duplicated if Thunderbird is closed then re-opened. Afterward, we begin to get modify and delete requests with a path of <http request>/Calendar/Calendar which results in 404 due to incorrect path. HOWEVER, note that user can still create new events and doing so seems to 'reset' the path issue thereby allowing users to once again modify and delete events.
Comment 18•12 years ago
|
||
rgnkjnva@aol.com: Lightning 1.5 is available with Thunderbird 13.0 (and 1.6 with TB 14.0).
Updated•12 years ago
|
Whiteboard: [CalDAV server: DAViCal] → [CalDAV server: DAViCal][calconnect25]
Updated•12 years ago
|
Attachment #637023 -
Attachment is obsolete: true
Attachment #637023 -
Flags: review?(philipp)
Comment 19•12 years ago
|
||
I've done some slight modifications and style fixes and have tested very simple operations (add/edit/delete) with these caldav servers: Google,Yahoo,SOGo,Zimbra,Kerio,eGroupware,DAViCal and it seems to be working. r=philipp
Attachment #686757 -
Flags: review+
Comment 20•12 years ago
|
||
Pushed to comm-central changeset 83ac919fe88d
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → 2.2
Comment 21•11 years ago
|
||
Patch was backed out due to regressions in bug 849797 for Lightning 2.2, Lightning 2.3, and Lightning 2.4 branches.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Comment 22•11 years ago
|
||
(In reply to Stefan Sitter from comment #21) > Patch was backed out due to regressions in bug 849797 for Lightning 2.2, > Lightning 2.3, and Lightning 2.4 branches. Let me clarify all this: 1. My original patch was written 2 years ago against Lightning 1.0, and without access to Mozilla's full testsuite. It is thus very likely that either a referenced variable was renamed by a different patch and/or an untested code path got invoked in a scenario I cannot test. The patch (as modified by others) needs to be reviewed and tested against the current code base by someone with access to the latest tools and the hazmat training needed for working with Mercury. 2. The fundamental bug here is that Lightning contains code which changes URLs in an *attempt* to work around a bug in some server other than DaviCal. In doing so the original Lightning 1.0 code would corrupt valid URLs, thus creating invalid requests that get rejected with HTTP error 500. If Lightning simply accepted whatever URL the server sent it, things would work just fine with DaviCal, so this is most definitely a Lightning bug, not a server problem. 3. Removing the offending "url cleaning" code would fix things for DaviCal users, but might break with whatever buggy server the original "url cleaning" code was trying to work around. Thus my original patch tried to preserve as much as possible of the apparently intended behavior of the code while avoiding the specific kind of incorrect url changing that caused the issue with DaviCal. This made the patch rather ugly and complex, which is why I used helper functions such as iseithersubstring() to keep things readable. 4. It has been some time since I tested if the original broken code is still active in Lightning. 5. I am still subscribed to the bug (as an interested and affected end user), but I never wanted to be assigned as the responsible developer and removed that incorrect assignment when I got inundated with requests to make my patch fit the internal rules for Mozilla Corporation employees.
Comment 23•11 years ago
|
||
Removing myself as assignee 1. I have moved, changed and dumped some of the calendaring servers I was using; so I can't test anymore 2. There is some code in davical too to work around specific client implementation issues, best is that someone tests who is at least aware of such things and/or corner cases 3. No hammocks or other testbeds laying around for Lightning code to be tested 4. I'm still subscribed (as an interested user).
Assignee: kwadronaut+bugzilla.mozilla → nobody
Comment 24•11 years ago
|
||
Don't worry, this being backed out doesn't mean its your fault. We just needed to do it since it wasn't fully working. I'm sure someone will step up eventually and provide a patch that fixes the issue. I agree the testbed isn't great, especially for caldav server testing. I've always wanted to do something about this, but you know how it is, there are always other things to take care of. I would imagine some sort of fakeserver that provides the exact responses the server does, along with a creation script that allows mocking a server with one easy python script. This does mean we need to extract all requests from the caldav code somehow so the test script doesn't have to be changed each time a request changes in the .js code. Maybe something worth a student-project.
Updated•11 years ago
|
Target Milestone: 2.2 → ---
Updated•10 years ago
|
Whiteboard: [CalDAV server: DAViCal][calconnect25] → [CalDAV server: DAViCal][calconnect31]
Updated•10 years ago
|
Whiteboard: [CalDAV server: DAViCal][calconnect31] → [CalDAV server: DAViCal][calconnect31-haspatch]
Comment 25•7 years ago
|
||
Thunderbird 52.3.0 Lightning 5.4.3 DaviCal with rewrite rules. Bug still exists. Will it be fixed?
Comment 26•6 years ago
|
||
Almost year passed by and no answer ( I found this article - http://codeverge.com/mozilla.support.calendar/lightning-1.0b2-caldav-problem-with/1513822 Patch for that version is included. Maybe this help to fix this odd bug with 8 years lifetime and release fixed Lightning version?
Comment 27•6 years ago
|
||
I am also affected by this bug with Thunderbird 52.9.1 (64-bit) and lightning 5.4.9.1 . The link provided by Dron is similar to https://groups.google.com/forum/#!topic/mozilla.support.calendar/2cNId3kT6y8 The solution and patch proposed there used to work for me years ago, but not anymore. This patch patches the function addTargetCalendarItem in calDavCalendar.js, which is not used for modifying events (anymore). So (probably) there is another bug in the code regardig the handling of CalDAV-URLs in another function. This is what I experience when trying to modify an event: Everything works just fine for calendars like this: https://calendar.example.com/calendar/caldav.php/username/calendar Modifications do not work for calendars like this (addtitions and deletions are still OK): https://calendar.example.com/username/calendar For the latter kind of calendar I can see the following in ther Thunderbird Error Console, when I modify an item: > PUT https://calendar.example.com/username/calendar/.php/username/calendar/778d0468-e6c3-456e-8bcc-ec92adffc15f.ics [HTTP/1.1 405 Method Not Allowed 1904ms] The correct URL should be: https://calendar.example.com/username/calendar/778d0468-e6c3-456e-8bcc-ec92adffc15f.ics This behavior is 100% reproducible and happens to all my accounts (following the abovementioned calendar URL pattern). This is the call stack, as far as I can see: > authSuccess file:///usr/lib/lightning/components/calDavCalendar.js:381:13 > calDavCalendar.prototype.sendHttpRequest file:///usr/lib/lightning/components/calDavCalendar.js:398:13 > calDavCalendar.prototype.doModifyItem file:///usr/lib/lightning/components/calDavCalendar.js:842:9 > calDavCalendar.prototype.modifyItem file:///usr/lib/lightning/components/calDavCalendar.js:740:16 > calCachedCalendar.prototype.modifyItem/flagListener.onOperationComplete file:///usr/lib/lightning/calendar-js/calCachedCalendar.js:719:21 > calStorageCalendar.prototype.getItemOfflineFlag/listener.handleCompletion file:///usr/lib/lightning/components/calStorageCalendar.js:959:21 Regards, Ferdinand Rau
Updated•4 years ago
|
Status: REOPENED → NEW
You need to log in
before you can comment on or make changes to this bug.
Description
•