fix(dav): Strip DTSTAMP before etag comparison in webcal refresh#58193
fix(dav): Strip DTSTAMP before etag comparison in webcal refresh#58193Olen wants to merge 2 commits intonextcloud:masterfrom
Conversation
7df84bc to
4c5b46f
Compare
|
As the |
4c5b46f to
36fdb9a
Compare
|
Thanks for the review @tcitworld — you're absolutely right. DTSTAMP is a required property per RFC 5545 and shouldn't be removed from the stored data. Apologies for being too quick with the initial approach. I've reworked the fix to be much more minimal: instead of stripping DTSTAMP from the VObject before serialization, it now only strips the DTSTAMP lines from a copy of the serialized string used for the etag/md5 computation. The stored calendar data is completely unchanged — DTSTAMP is preserved. The diff is now just one line replaced in -$etag = md5($sObject);
+$sObjectForEtag = preg_replace('/^DTSTAMP:.*\r?\n/m', '', $sObject);
+$etag = md5($sObjectForEtag); |
Many iCal providers (Google Calendar, Outlook 365, itslearning) set DTSTAMP to the current UTC time on every feed request per RFC 5545. Since etags are computed as md5(serialized_data), every event appears modified on every refresh — generating ~189K false change rows per day for a single subscription with 2,621 events. Strip DTSTAMP from non-VTIMEZONE components before serialization, following the existing pattern used for VALARM and ATTACH stripping. Fixes: nextcloud#51120 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: olen <ola@nytt.no>
DTSTAMP is a required property per RFC 5545 and must not be removed from stored calendar data. Strip it only from the serialized string used for etag computation via preg_replace, leaving the data passed to createCalendarObject/updateCalendarObject unchanged. Signed-off-by: Olen <regopa@gmail.com> Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: olen <ola@nytt.no>
36fdb9a to
6d10452
Compare
| // to the current time on every feed request per RFC 5545, causing | ||
| // every event to appear modified on every refresh. | ||
| // DTSTAMP is kept in the stored data as it is a required property. | ||
| $sObjectForEtag = preg_replace('/^DTSTAMP:.*\r?\n/m', '', $sObject); |
There was a problem hiding this comment.
There is a risk that the line might be over 75 characters even in this case (for instance if we have a TZID), so I would handle content folding as well in the regex (new line must not be followed by a white-space character).
https://icalendar.org/iCalendar-RFC-5545/3-1-content-lines.html
Summary
DTSTAMPfrom etag computation inRefreshWebcalServiceto prevent false change detectionProblem
Many iCal providers (Google Calendar, Outlook 365, itslearning) set
DTSTAMPto the current UTC time on every feed request per RFC 5545. Since Nextcloud computes the etag asmd5(serialized_data)and DTSTAMP changes every time, every event appears modified on every refresh — even if nothing actually changed.For a Google Calendar subscription with 2,621 events refreshed every ~15 minutes, this generates ~189K false change rows per day in
oc_calendarchanges.Reported at: #51120 (comment)
Approach
Instead of stripping DTSTAMP from the VObject (which would remove a required property from stored data), the fix strips DTSTAMP lines from a copy of the serialized string used only for etag computation:
The stored calendar data (
$sObject) is completely unchanged.Test plan
testDtstampChangeDoesNotTriggerUpdateverifies a DTSTAMP-only change does not triggerupdateCalendarObjectidenticalDataProvideretag computation to match the new DTSTAMP-stripped hashingRefreshWebcalServiceTestunit testsFixes: #51120
🤖 Generated with Claude Code