From 03004531ab989f6a95e5df07297c17d01618f0c0 Mon Sep 17 00:00:00 2001 From: Magnus Melin Date: Thu, 5 Sep 2024 13:42:16 +0300 Subject: [PATCH] Bug 1906077 - ICS calendar subscriptions should use the original URI for temporary redirects. r=aleca To test, subscribe to https://github.com/othyn/go-calendar/releases/latest/download/gocal__community_day.ics Also * don't fail for cases there content-type is null * accept for content-type=application/octet-stream where the Content-Disposition says it's an .ics Differential Revision: https://phabricator.services.mozilla.com/D220872 --HG-- extra : rebase_source : a3e7e2cc8f56bdf7e7968b1a2863bbfa8002b9a2 extra : absorb_source : 3e9d607dfd459e5ad8f87348838f9667711f8cd8 --- .../utils/calProviderDetectionUtils.sys.mjs | 2 +- .../providers/caldav/CalDavCalendar.sys.mjs | 2 - .../providers/caldav/CalDavProvider.sys.mjs | 2 +- calendar/providers/ics/CalICSProvider.sys.mjs | 19 +++++--- calendar/test/ICSServer.sys.mjs | 13 +++-- .../test/unit/providers/test_cal_detection.js | 48 +++++++++++++++++++ calendar/test/unit/providers/xpcshell.ini | 1 + 7 files changed, 73 insertions(+), 14 deletions(-) create mode 100644 calendar/test/unit/providers/test_cal_detection.js diff --git a/calendar/base/modules/utils/calProviderDetectionUtils.sys.mjs b/calendar/base/modules/utils/calProviderDetectionUtils.sys.mjs index a8b9d83ee9..bfbe550511 100644 --- a/calendar/base/modules/utils/calProviderDetectionUtils.sys.mjs +++ b/calendar/base/modules/utils/calProviderDetectionUtils.sys.mjs @@ -98,7 +98,7 @@ export var detection = { * @param {ProviderFilter[]} aPreDetectFilters - Functions for filtering out providers. * @param {object} aExtraProperties - Extra properties to pass on to the * providers. - * @returns {Promise>} a Map of provider type to calendars found. + * @returns {Promise>} a Map of provider type to calendars found. */ async detect( aUsername, diff --git a/calendar/providers/caldav/CalDavCalendar.sys.mjs b/calendar/providers/caldav/CalDavCalendar.sys.mjs index a32c7c4f88..f08e4458e4 100644 --- a/calendar/providers/caldav/CalDavCalendar.sys.mjs +++ b/calendar/providers/caldav/CalDavCalendar.sys.mjs @@ -99,8 +99,6 @@ CalDavCalendar.prototype = { return this != this.superCalendar; }, - mLastRedirectStatus: null, - ensureTargetCalendar() { if (!this.isCached && !this.mOfflineStorage) { // If this is a cached calendar, the actual cache is taken care of diff --git a/calendar/providers/caldav/CalDavProvider.sys.mjs b/calendar/providers/caldav/CalDavProvider.sys.mjs index 46de36dd18..6b491885d4 100644 --- a/calendar/providers/caldav/CalDavProvider.sys.mjs +++ b/calendar/providers/caldav/CalDavProvider.sys.mjs @@ -307,7 +307,7 @@ class CalDavDetector { // `request.commit()` can throw; errors should be caught by calling functions. const response = await request.commit(); - const homeSets = response.firstProps["C:calendar-home-set"]; + const homeSets = response.firstProps ? response.firstProps["C:calendar-home-set"] : null; const target = response.uri; if (response.authError) { diff --git a/calendar/providers/ics/CalICSProvider.sys.mjs b/calendar/providers/ics/CalICSProvider.sys.mjs index 3f988f296e..a3d93c6b82 100644 --- a/calendar/providers/ics/CalICSProvider.sys.mjs +++ b/calendar/providers/ics/CalICSProvider.sys.mjs @@ -277,14 +277,19 @@ class ICSDetector { // The content type header may include a charset, so use 'string.includes'. if (response.ok) { - const header = response.getHeader("Content-Type"); - + const contentType = response.getHeader("Content-Type"); + const contentDisposition = response.getHeader("Content-Disposition"); if ( - header.includes("text/calendar") || - header.includes("application/ics") || + contentType?.includes("text/calendar") || + contentType?.includes("application/ics") || + /\.ics\b/i.test(contentDisposition) || (response.text && response.text.includes("BEGIN:VCALENDAR")) ) { - const target = response.uri; + let target = response.uri; + // Set up calendar for original URI for temporal redirects. + if (response.lastRedirectStatus == 302 || response.lastRedirectStatus == 307) { + target = response.nsirequest.originalURI; + } cal.LOG(`[calICSProvider] ${target.spec} has valid content type (via ${method} request)`); return [this.handleCalendar(target)]; } @@ -410,8 +415,8 @@ class ICSDetector { * Set up and return a new ICS calendar object. * * @param {nsIURI} uri - The location of the calendar. - * @param {Set} [props] - For CalDav calendars, these are the props - * parsed from the response. + * @param {Set} [props] - For CalDav calendars, these are the props parsed + * from the response. * @returns {calICalendar} A new calendar. */ handleCalendar(uri, props = new Set()) { diff --git a/calendar/test/ICSServer.sys.mjs b/calendar/test/ICSServer.sys.mjs index 8db98b6f9f..9fcc0bfaa3 100644 --- a/calendar/test/ICSServer.sys.mjs +++ b/calendar/test/ICSServer.sys.mjs @@ -2,8 +2,6 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this * file, you can obtain one at http://mozilla.org/MPL/2.0/. */ -import { Assert } from "resource://testing-common/Assert.sys.mjs"; - import { CommonUtils } from "resource://services-common/utils.sys.mjs"; import { HttpServer } from "resource://testing-common/httpd.sys.mjs"; @@ -21,6 +19,7 @@ export var ICSServer = { this.username = username; this.password = password; this.server.registerPathHandler("/ping", this.ping); + this.server.registerPathHandler("/http302", this.http302); this.server.registerPathHandler(this.path, this.handleICS.bind(this)); this.reset(); @@ -98,6 +97,15 @@ export var ICSServer = { response.write("pong"); }, + http302(request, response) { + const params = new URLSearchParams(request.queryString); + const path = params.get("path"); + response.setStatusLine("1.1", 302, "Found"); + response.setHeader("Location", `${ICSServer.origin}/${path}`); + response.setHeader("Content-Type", "text/plain; charset=UTF-8"); + response.setHeader("Content-Length", "0"); + }, + handleICS(request, response) { if (!this.checkAuth(request, response)) { return; @@ -115,7 +123,6 @@ export var ICSServer = { return; } - Assert.report(true, undefined, undefined, "Should not have reached here"); response.setStatusLine("1.1", 405, "Method Not Allowed"); response.setHeader("Content-Type", "text/plain"); response.write(`Method not allowed: ${request.method}`); diff --git a/calendar/test/unit/providers/test_cal_detection.js b/calendar/test/unit/providers/test_cal_detection.js new file mode 100644 index 0000000000..e9b51bdb3f --- /dev/null +++ b/calendar/test/unit/providers/test_cal_detection.js @@ -0,0 +1,48 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, you can obtain one at http://mozilla.org/MPL/2.0/. */ + +var { ICSServer } = ChromeUtils.importESModule( + "resource://testing-common/calendar/ICSServer.sys.mjs" +); + +var { detection } = ChromeUtils.importESModule( + "resource:///modules/calendar/utils/calProviderDetectionUtils.sys.mjs" +); + +add_setup(async () => { + ICSServer.open(); + ICSServer.putICSInternal( + CalendarTestUtils.dedent` + BEGIN:VCALENDAR + BEGIN:VEVENT + UID:6714b781-920f-46f8-80ec-3d3995e2e9ce + SUMMARY:some event + DTSTART:20210401T120000Z + DTEND:20210401T130000Z + END:VEVENT + END:VCALENDAR + ` + ); + registerCleanupFunction(() => ICSServer.close()); +}); + +add_task(async function testIcsDetection() { + const url = `${ICSServer.origin}/test.ics`; + const detectedCals = await detection.detect("", "", url, false, [], {}); + Assert.ok(detectedCals, "should find calendars"); + Assert.equal(detectedCals.size, 1, "should find one calendar"); + const icsCal = detectedCals.values().next().value[0]; + Assert.equal(icsCal.uri.spec, url, "should have expected uri"); +}); + +add_task(async function testIcsDetection302() { + // This url will redirect to test.ics for the actual content. + // We still want to subscribe to the original url. + const url = `${ICSServer.origin}/http302?path=test.ics`; + const detectedCals = await detection.detect("", "", url, false, [], {}); + Assert.ok(detectedCals, "should find calendars"); + Assert.equal(detectedCals.size, 1, "should find one calendar"); + const icsCal = detectedCals.values().next().value[0]; + Assert.equal(icsCal.uri.spec, url, "should have expected uri"); +}); diff --git a/calendar/test/unit/providers/xpcshell.ini b/calendar/test/unit/providers/xpcshell.ini index 272fef7c78..61889c8e03 100644 --- a/calendar/test/unit/providers/xpcshell.ini +++ b/calendar/test/unit/providers/xpcshell.ini @@ -4,6 +4,7 @@ prefs = calendar.timezone.local=UTC calendar.timezone.useSystemTimezone=false +[test_cal_detection.js] [test_caldavCalendar_cached.js] [test_caldavCalendar_uncached.js] [test_icsCalendar_cached.js]