From 1194bf284f4e9f97d2b88b6da20270310cad068e Mon Sep 17 00:00:00 2001 From: Linus Rath <139418639+rathlinus@users.noreply.github.com> Date: Sat, 31 Oct 2026 18:10:20 +0100 Subject: [PATCH] fix: keep ios push renewal from reporting an extension the server refused --- plugins/ios-push/Jmap.swift | 23 +++++++++++++++++ plugins/ios-push/PushRenewal.swift | 17 +++++++++--- src/lib/__tests__/ios-push-renewal.test.ts | 30 ++++++++++++++++++++++ 3 files changed, 66 insertions(+), 4 deletions(-) diff --git a/plugins/ios-push/Jmap.swift b/plugins/ios-push/Jmap.swift index 5cb6fa60..f9ca3bb1 100644 --- a/plugins/ios-push/Jmap.swift +++ b/plugins/ios-push/Jmap.swift @@ -21,6 +21,8 @@ enum JmapError: Error { case needsRefresh case http(Int) case badResponse + /// A /set call listed the record under notUpdated (or not under updated). + case notUpdated(String) } /// Credentials as the app stores them (src/api/jmap-client.ts StoredCredentials). @@ -166,9 +168,30 @@ enum Jmap { ["PushSubscription/get", ["ids": [id], "properties": pushSubscriptionProperties], "1"], ] let responses = try await call(apiUrl: apiUrl, authorization: authorization, using: ["urn:ietf:params:jmap:core"], methodCalls: methodCalls) + if let refused = notUpdated(responses, method: "PushSubscription/set", callId: "0", id: id) { + throw JmapError.notUpdated(refused) + } return listedPushSubscription(responses, callId: "1", id: id) } + /// Why the /set answer `callId` didn't update `id`, or nil when it did. + /// A method error or an answer without `id` under updated counts as refused. + static func notUpdated(_ responses: [[Any]], method: String, callId: String, id: String) -> String? { + for response in responses where response.count >= 3 && response[2] as? String == callId { + guard response[0] as? String == method, let args = response[1] as? [String: Any] else { + let args = response[1] as? [String: Any] + return (args?["type"] as? String) ?? "error" + } + if let refused = (args["notUpdated"] as? [String: Any])?[id] { + let error = refused as? [String: Any] + return (error?["description"] as? String) ?? (error?["type"] as? String) ?? "notUpdated" + } + if let updated = args["updated"] as? [String: Any], updated.keys.contains(id) { return nil } + return "not in updated" + } + return "no answer" + } + static func fetch(apiUrl: URL, accountId: String, emailIds: [String], authorization: String) async throws -> JmapFetchResult { var request = URLRequest(url: apiUrl) request.httpMethod = "POST" diff --git a/plugins/ios-push/PushRenewal.swift b/plugins/ios-push/PushRenewal.swift index 68ac8523..06e8ec25 100644 --- a/plugins/ios-push/PushRenewal.swift +++ b/plugins/ios-push/PushRenewal.swift @@ -83,23 +83,32 @@ enum PushRenewal { } // The recorded expiry was stale: the server's copy has time left. if let expires = current.expires, !needsExtension(expires) { - record(key: key, account: account, expires: expires) + record(key: key, subscriptionId: subscriptionId, expires: expires) return false } let wanted = formatDate(Date().addingTimeInterval(wantedSeconds)) let extended = try await Jmap.extendPushSubscription( apiUrl: apiUrl, id: subscriptionId, expires: wanted, authorization: authorization) - if let expires = extended?.expires { record(key: key, account: account, expires: expires) } + if let expires = extended?.expires { record(key: key, subscriptionId: subscriptionId, expires: expires) } pushLog.info("extended subscription \(subscriptionId, privacy: .public)") return true + } catch JmapError.notUpdated(let reason) { + // The server kept the old expiry: not extended, nothing to record. + pushLog.error("server did not extend subscription \(subscriptionId, privacy: .public): \(reason, privacy: .public)") + return false } catch { pushLog.error("renewal failed: \(String(describing: error), privacy: .public)") return false } } - private static func record(key: String, account: [String: Any], expires: String) { - var json = account + /// Writes the expiry into the account's entry as it is now, not as it was + /// when the renewal started: the app may have replaced the subscription (or + /// the credentials key) meanwhile. Only the expiry changes, and only while + /// the entry still names the subscription that was extended. + private static func record(key: String, subscriptionId: String, expires: String) { + guard var json = SharedKeychain.readJSON(key), + json["subscriptionId"] as? String == subscriptionId else { return } json["subscriptionExpires"] = expires SharedKeychain.update(key, json: json) } diff --git a/src/lib/__tests__/ios-push-renewal.test.ts b/src/lib/__tests__/ios-push-renewal.test.ts index 8b04a8b4..a742bff8 100644 --- a/src/lib/__tests__/ios-push-renewal.test.ts +++ b/src/lib/__tests__/ios-push-renewal.test.ts @@ -143,6 +143,36 @@ describe('the native renewal', () => { expect(set).toBeGreaterThan(own); }); + it('treats an update the server refused as not extended, and records nothing', () => { + const jmap = read('plugins/ios-push/Jmap.swift'); + const extend = jmap.slice(jmap.indexOf('static func extendPushSubscription(')); + const check = extend.indexOf('notUpdated(responses, method: "PushSubscription/set", callId: "0", id: id)'); + expect(check).toBeGreaterThan(-1); + expect(extend.indexOf('throw JmapError.notUpdated(')).toBeGreaterThan(check); + expect(check).toBeLessThan(extend.indexOf('return listedPushSubscription(responses, callId: "1", id: id)')); + // The renewal stops on it before logging "extended" or recording. + const body = renewal.slice(renewal.indexOf('static func renew(key:'), renewal.indexOf('private static func record(')); + const refused = body.match(/catch JmapError\.notUpdated[\s\S]*?\}/)?.[0] ?? ''; + expect(refused).toContain('return false'); + expect(refused).not.toContain('record('); + expect(body.indexOf('try await Jmap.extendPushSubscription(')).toBeLessThan(body.indexOf('pushLog.info("extended subscription')); + }); + + it('writes back only the expiry, into the entry as it is now, while it names the same subscription', () => { + const record = renewal.slice(renewal.indexOf('private static func record(')); + expect(record).toMatch(/^private static func record\(key: String, subscriptionId: String, expires: String\)/); + expect(record).toMatch(/guard var json = SharedKeychain\.readJSON\(key\),\s*json\["subscriptionId"\] as\? String == subscriptionId else \{ return \}/); + expect(record).toMatch(/json\["subscriptionExpires"\] = expires\s*SharedKeychain\.update\(key, json: json\)/); + expect(renewal).not.toMatch(/record\(key: key, account:/); + }); + + it('reads a refused update the way JMAP reports it', () => { + const jmap = read('plugins/ios-push/Jmap.swift'); + const helper = jmap.slice(jmap.indexOf('static func notUpdated(')); + expect(helper).toMatch(/args\["notUpdated"\] as\? \[String: Any\]\)\?\[id\]/); + expect(helper).toMatch(/updated\.keys\.contains\(id\) \{ return nil \}/); + }); + it('runs from the notification extension after the fetch for the pushed account', () => { const renew = nse.indexOf('await PushRenewal.renewIfDue(jmapAccountId: accountId)'); expect(renew).toBeGreaterThan(nse.indexOf('await NotificationService.enrich('));