kayg/calternal

Watch

Fork 0

Webcal refresh fails with 400 when the feed is unchanged (304 treated as a redirect) #572

New issue

Open

opened 2026-10-01 05:00:16 +00:00 by kayg · 3 comments

kayg commented

2026-10-01 05:00:16 +00:00

Owner

Found by caldav-stress round 3 (#457, 2026-10-01; repro and source trace in the #457 comment)

A webcal subscription refresh whose upstream answers 304 Not Modified (to our conditional GET with If-None-Match / If-Modified-Since) is treated as a redirect and fails with HTTP 400. So every refresh of an unchanged subscribed calendar fails: the common case.
Fix: treat 304 as "unchanged": keep the stored copy, update the last-checked time, and do not re-parse. Make the redirect handling accept only 301/302/303/307/308 with a Location (and keep the SSRF guard on each hop). Make sure the stored ETag/Last-Modified are sent on refresh.
Tests: a unit test for 304 → unchanged; an integration test with a local feed server that returns 200, then 304, then 200 with changes. Calendar plugin crate gates. Time limit 1.5 h.

## Found by caldav-stress round 3 (#457, 2026-10-01; repro and source trace in the #457 comment) A webcal subscription refresh whose upstream answers **304 Not Modified** (to our conditional GET with If-None-Match / If-Modified-Since) is treated as a **redirect** and fails with HTTP 400. So every refresh of an **unchanged** subscribed calendar fails: the common case. **Fix:** treat 304 as "unchanged": keep the stored copy, update the last-checked time, and do not re-parse. Make the redirect handling accept only 301/302/303/307/308 with a Location (and keep the SSRF guard on each hop). Make sure the stored ETag/Last-Modified are sent on refresh. **Tests:** a unit test for 304 → unchanged; an integration test with a local feed server that returns 200, then 304, then 200 with changes. Calendar plugin crate gates. Time limit 1.5 h.

kayg commented

2026-10-01 05:31:59 +00:00

Author

Owner

Starting #572 on branch job/webcal-572, based on cc25c441b7a974185622a1dee853cf38686d2b67 (origin/dev). I am tracing the Webcal refresh and existing calendar plugin tests before making the conditional refresh and redirect fix.

Starting #572 on branch `job/webcal-572`, based on `cc25c441b7a974185622a1dee853cf38686d2b67` (origin/dev). I am tracing the Webcal refresh and existing calendar plugin tests before making the conditional refresh and redirect fix.

kayg commented

2026-10-01 05:57:46 +00:00

Author

Owner

Finding for #572: fetch_calendar_inner called StatusCode::is_redirection() before its existing 304 branch. That predicate also matches 304, so unchanged feeds returned FetchError::InvalidUrl when no Location header existed. The cache already stores ETag and Last-Modified and the refresh request already sends both; the local regression server now checks both headers across 200 → 304 → changed 200 and verifies the 304 keeps the iCalendar and parsed Event bytes while advancing refreshed_ms.

Finding for #572: `fetch_calendar_inner` called `StatusCode::is_redirection()` before its existing 304 branch. That predicate also matches 304, so unchanged feeds returned `FetchError::InvalidUrl` when no Location header existed. The cache already stores ETag and Last-Modified and the refresh request already sends both; the local regression server now checks both headers across 200 → 304 → changed 200 and verifies the 304 keeps the iCalendar and parsed Event bytes while advancing `refreshed_ms`.

kayg commented

2026-10-01 06:18:12 +00:00

Author

Owner

#572 complete

Built: 304 now means unchanged. The refresh keeps the cached iCalendar and parsed Event bytes, updates refreshed_ms and the next check time, and does not parse the response again. Redirect handling now recognizes only 301, 302, 303, 307 and 308, and still requires a Location and validates every hop. The existing ETag and Last-Modified request headers are covered by the local feed test.

Files: crates/plugins/calendar/src/feeds/subscriptions.rs; bench/calendar-subscription-refresh.sh.

Head SHA: e1b364a41df0b3a702f598f977faaaa4eb59e413.

Gates (output verbatim):

cargo fmt --all -- --check: exit 0, no output.
cargo clippy -p calternal-plugin-calendar --all-targets -- -D warnings: Finished dev profile [unoptimized + debuginfo] target(s) in 8m 19s.
cargo test -p calternal-plugin-calendar:
- test result: ok. 83 passed; 0 failed; 1 ignored; 0 measured; 0 filtered out; finished in 16.62s
- test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.29s (cache integration)
- test result: ok. 3 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.16s (protocol integration)
- Doc tests: test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

Local hot-path profile, 5,000 cached Events and a 487,825-byte feed: serial refresh p50 59.470 ms / p95 96.345 ms (mean 68.147 ms); four-request burst p50 85.706 ms / p95 225.255 ms (mean 132.218 ms); process CPU 2.411 s user / 0.130 s system; peak RSS 81,220 KiB. The profile includes the initial 200 parse plus four serial and four burst 304 refreshes. docs/perf/baseline.json has no subscription-refresh metric; its closest calendar API baseline is calendar.events at p50 1.4 ms / p95 3.9 ms. That baseline measures a local API read and is not directly comparable to this local upstream-fetch and cache-write profile.

Known gap: no existing subscription-refresh baseline is available, and this measurement ran locally rather than on the single-tenant perf VM.

Decision: DESIGN §30 says external calendar data stays in the rebuildable Index but does not define 304 metadata handling. The fix follows the existing cache behavior: use the cached feed's refresh hint (one-hour fallback) and preserve stored validators when a 304 omits them.

#572 complete Built: 304 now means unchanged. The refresh keeps the cached iCalendar and parsed Event bytes, updates `refreshed_ms` and the next check time, and does not parse the response again. Redirect handling now recognizes only 301, 302, 303, 307 and 308, and still requires a Location and validates every hop. The existing ETag and Last-Modified request headers are covered by the local feed test. Files: `crates/plugins/calendar/src/feeds/subscriptions.rs`; `bench/calendar-subscription-refresh.sh`. Head SHA: `e1b364a41df0b3a702f598f977faaaa4eb59e413`. Gates (output verbatim): - `cargo fmt --all -- --check`: exit 0, no output. - `cargo clippy -p calternal-plugin-calendar --all-targets -- -D warnings`: `Finished `dev` profile [unoptimized + debuginfo] target(s) in 8m 19s`. - `cargo test -p calternal-plugin-calendar`: - `test result: ok. 83 passed; 0 failed; 1 ignored; 0 measured; 0 filtered out; finished in 16.62s` - `test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.29s` (cache integration) - `test result: ok. 3 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.16s` (protocol integration) - Doc tests: `test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s` Local hot-path profile, 5,000 cached Events and a 487,825-byte feed: serial refresh p50 59.470 ms / p95 96.345 ms (mean 68.147 ms); four-request burst p50 85.706 ms / p95 225.255 ms (mean 132.218 ms); process CPU 2.411 s user / 0.130 s system; peak RSS 81,220 KiB. The profile includes the initial 200 parse plus four serial and four burst 304 refreshes. `docs/perf/baseline.json` has no subscription-refresh metric; its closest calendar API baseline is `calendar.events` at p50 1.4 ms / p95 3.9 ms. That baseline measures a local API read and is not directly comparable to this local upstream-fetch and cache-write profile. Known gap: no existing subscription-refresh baseline is available, and this measurement ran locally rather than on the single-tenant perf VM. Decision: DESIGN §30 says external calendar data stays in the rebuildable Index but does not define 304 metadata handling. The fix follows the existing cache behavior: use the cached feed's refresh hint (one-hour fallback) and preserve stored validators when a 304 omits them.

kayg referenced this issue

2026-10-03 12:51:31 +00:00

Issue hygiene sweep 2026-10-03 #1008

No labels

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference

kayg/calternal#572

No description provided.

Rows
Columns