mirror of
https://github.com/johannesjo/super-productivity.git
synced 2026-07-17 16:37:43 +00:00
fix(sync): force no-cache on native WebDAV reads (iOS data loss) (#7982)
iOS reads a stale sync-data.json on every WebDAV sync: URLSession's default configuration caches GET responses, so the client never sees remote changes and — because the pre-upload conflict check re-reads the same cached body — blindly overwrites newer remote data. Android (OkHttp, no cache) and web (fetch `cache: 'no-store'`) are unaffected, which is why this is iOS-only. - iOS WebDavHttpPlugin: disable the HTTP cache (urlCache=nil + reloadIgnoringLocalCacheData) so sync reads always hit the network. - WebDavHttpAdapter: send `Cache-Control: no-cache, no-store` + `Pragma` on the native request path, covering an upstream proxy/CDN cache too. - Log allowlisted, non-sensitive cache-relevant response headers so a future log capture localises a stale read (client cache vs proxy). Fixes #7144
This commit is contained in:
parent
89d5fd39ba
commit
5aea4b0143
3 changed files with 166 additions and 2 deletions
|
|
@ -13,6 +13,15 @@ public class WebDavHttpPlugin: CAPPlugin, CAPBridgedPlugin {
|
|||
let config = URLSessionConfiguration.default
|
||||
config.timeoutIntervalForRequest = 30
|
||||
config.timeoutIntervalForResource = 30
|
||||
// Sync correctness (#7144): never serve WebDAV responses from the HTTP
|
||||
// cache. A cached GET of sync-data.json hides remote changes AND defeats
|
||||
// the content-hash conflict check (the pre-upload GET returns the same
|
||||
// stale body), so the client silently overwrites newer remote data and
|
||||
// loses it. URLSessionConfiguration.default ships a shared URLCache and
|
||||
// the default .useProtocolCachePolicy, so iOS caches these GETs while
|
||||
// Android (OkHttp, no cache) and web (fetch `cache: 'no-store'`) do not.
|
||||
config.requestCachePolicy = .reloadIgnoringLocalCacheData
|
||||
config.urlCache = nil
|
||||
return URLSession(configuration: config)
|
||||
}()
|
||||
|
||||
|
|
|
|||
|
|
@ -10,7 +10,7 @@ import {
|
|||
TooManyRequestsAPIError,
|
||||
} from '../../errors';
|
||||
import { errorMeta } from '../../log/error-meta';
|
||||
import { WebDavHttpStatus } from './webdav.const';
|
||||
import { WebDavHttpHeader, WebDavHttpStatus } from './webdav.const';
|
||||
|
||||
export interface WebDavHttpRequest {
|
||||
url: string;
|
||||
|
|
@ -42,6 +42,44 @@ export interface WebDavHttpAdapterDeps {
|
|||
export class WebDavHttpAdapter {
|
||||
private static readonly L = 'WebDavHttpAdapter';
|
||||
|
||||
/**
|
||||
* Sync correctness (#7144): force revalidation on the NATIVE HTTP path. iOS
|
||||
* `URLSession` caches GETs by default and an upstream reverse proxy / CDN may
|
||||
* cache too — a stale `sync-data.json` both hides remote changes and defeats
|
||||
* the content-hash conflict check, silently overwriting newer remote data.
|
||||
*
|
||||
* Native-only, intentionally: the web/fetch path uses the CORS-safe fetch
|
||||
* `cache: 'no-store'` option instead. `Cache-Control` is not a CORS-safelisted
|
||||
* request header, so adding it to a cross-origin browser request would require
|
||||
* the WebDAV server to allow it in preflight and could break web sync. Native
|
||||
* HTTP is not subject to CORS, so the headers are safe here and additionally
|
||||
* instruct upstream proxies to revalidate.
|
||||
*/
|
||||
private static readonly NO_CACHE_HEADERS: Record<string, string> = {
|
||||
[WebDavHttpHeader.CACHE_CONTROL]: 'no-cache, no-store',
|
||||
Pragma: 'no-cache',
|
||||
};
|
||||
|
||||
/**
|
||||
* Non-sensitive, cache-relevant response headers worth surfacing in a shared
|
||||
* log capture so we can tell whether a stale read came from a client cache or
|
||||
* an upstream proxy. Deliberately excludes anything that can carry user data
|
||||
* or secrets (set-cookie, www-authenticate, content-location, location).
|
||||
*/
|
||||
private static readonly _CACHE_HEADER_ALLOWLIST = [
|
||||
'etag',
|
||||
'age',
|
||||
'cache-control',
|
||||
'date',
|
||||
'last-modified',
|
||||
'expires',
|
||||
'x-cache',
|
||||
'cf-cache-status',
|
||||
'x-served-by',
|
||||
'via',
|
||||
'x-proxy-cache',
|
||||
] as const;
|
||||
|
||||
constructor(private readonly _deps: WebDavHttpAdapterDeps) {}
|
||||
|
||||
async request(options: WebDavHttpRequest): Promise<WebDavHttpResponse> {
|
||||
|
|
@ -63,7 +101,10 @@ export class WebDavHttpAdapter {
|
|||
const nativeResp = await this._deps.nativeHttp({
|
||||
url: options.url,
|
||||
method: options.method,
|
||||
headers: options.headers ?? {},
|
||||
headers: {
|
||||
...(options.headers ?? {}),
|
||||
...WebDavHttpAdapter.NO_CACHE_HEADERS,
|
||||
},
|
||||
data: options.body ?? undefined,
|
||||
responseType: 'text',
|
||||
});
|
||||
|
|
@ -101,6 +142,17 @@ export class WebDavHttpAdapter {
|
|||
}
|
||||
}
|
||||
|
||||
// Diagnostic (#7144): surface cache-relevant response headers so a shared
|
||||
// log capture reveals whether a stale read originated from the client
|
||||
// cache or an upstream proxy. Allowlisted, non-sensitive metadata only.
|
||||
this._deps.logger.normal(`${WebDavHttpAdapter.L}.response()`, {
|
||||
method: options.method,
|
||||
url: scrubbedUrl,
|
||||
status: response.status,
|
||||
bodyChars: response.data.length,
|
||||
cacheHeaders: WebDavHttpAdapter._pickCacheHeaders(response.headers),
|
||||
});
|
||||
|
||||
// Check for common HTTP errors
|
||||
this._checkHttpStatus(response.status, scrubbedUrl, response.data);
|
||||
|
||||
|
|
@ -166,6 +218,26 @@ export class WebDavHttpAdapter {
|
|||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Serialize the allowlisted cache-relevant headers (case-insensitive) into a
|
||||
* compact, log-safe string like `etag=W/"a1"; age=3; x-cache=HIT`. Returns
|
||||
* `'none'` when the response carries no such headers (already a useful
|
||||
* signal). `SyncLogMeta` only holds primitives, hence a string not an object.
|
||||
*/
|
||||
private static _pickCacheHeaders(headers: Record<string, string>): string {
|
||||
const lower: Record<string, string> = {};
|
||||
for (const [key, value] of Object.entries(headers)) {
|
||||
lower[key.toLowerCase()] = value;
|
||||
}
|
||||
const parts: string[] = [];
|
||||
for (const key of WebDavHttpAdapter._CACHE_HEADER_ALLOWLIST) {
|
||||
if (lower[key] !== undefined) {
|
||||
parts.push(`${key}=${lower[key]}`);
|
||||
}
|
||||
}
|
||||
return parts.length > 0 ? parts.join('; ') : 'none';
|
||||
}
|
||||
|
||||
private async _convertFetchResponse(response: Response): Promise<WebDavHttpResponse> {
|
||||
const headers: Record<string, string> = {};
|
||||
response.headers.forEach((value, key) => {
|
||||
|
|
|
|||
|
|
@ -63,6 +63,34 @@ describe('WebDavHttpAdapter', () => {
|
|||
);
|
||||
});
|
||||
|
||||
it('sends no-cache request headers on the native path (#7144)', async () => {
|
||||
// Regression guard: iOS URLSession / upstream proxies otherwise serve a
|
||||
// stale sync-data.json, which hides remote changes and defeats the
|
||||
// content-hash conflict check, causing silent overwrite/data loss.
|
||||
const nativeHttp = vi.fn().mockResolvedValue({
|
||||
status: 200,
|
||||
headers: {},
|
||||
data: 'native-body',
|
||||
});
|
||||
const adapter = new WebDavHttpAdapter(
|
||||
makeDeps({ isNativePlatform: true, nativeHttp }),
|
||||
);
|
||||
|
||||
await adapter.request({
|
||||
url: 'https://dav.example.com/sync/file',
|
||||
method: 'GET',
|
||||
headers: { Authorization: 'Basic abc' },
|
||||
});
|
||||
|
||||
const sentHeaders = (
|
||||
nativeHttp.mock.calls[0][0] as { headers: Record<string, string> }
|
||||
).headers;
|
||||
expect(sentHeaders['Cache-Control']).toBe('no-cache, no-store');
|
||||
expect(sentHeaders['Pragma']).toBe('no-cache');
|
||||
// Caller headers are preserved.
|
||||
expect(sentHeaders['Authorization']).toBe('Basic abc');
|
||||
});
|
||||
|
||||
it('uses fetch when not native', async () => {
|
||||
const fetchImpl = vi.fn().mockResolvedValue(okFetchResponse(200, 'web-body'));
|
||||
const adapter = new WebDavHttpAdapter(
|
||||
|
|
@ -237,6 +265,61 @@ describe('WebDavHttpAdapter', () => {
|
|||
expect(JSON.stringify(loggedMessages)).not.toContain('sync/file');
|
||||
});
|
||||
|
||||
it('logs allowlisted cache headers but never sensitive ones (#7144 diagnostic)', async () => {
|
||||
/* eslint-disable @typescript-eslint/naming-convention */
|
||||
const nativeHttp = vi.fn().mockResolvedValue({
|
||||
status: 200,
|
||||
headers: {
|
||||
ETag: 'W/"v15"',
|
||||
Age: '7',
|
||||
'X-Cache': 'HIT',
|
||||
'Set-Cookie': 'session=topsecret',
|
||||
'WWW-Authenticate': 'Basic realm="x"',
|
||||
},
|
||||
data: 'body',
|
||||
});
|
||||
/* eslint-enable @typescript-eslint/naming-convention */
|
||||
const adapter = new WebDavHttpAdapter(
|
||||
makeDeps({ isNativePlatform: true, nativeHttp, logger }),
|
||||
);
|
||||
|
||||
await adapter.request({
|
||||
url: 'https://dav.example.com/sync/file',
|
||||
method: 'GET',
|
||||
});
|
||||
|
||||
const responseLog = loggedMessages.find((l) => l.msg.includes('.response()'));
|
||||
expect(responseLog).toBeTruthy();
|
||||
const cacheHeaders = (responseLog?.meta as { cacheHeaders?: string }).cacheHeaders;
|
||||
// Allowlisted, case-insensitive.
|
||||
expect(cacheHeaders).toContain('etag=W/"v15"');
|
||||
expect(cacheHeaders).toContain('age=7');
|
||||
expect(cacheHeaders).toContain('x-cache=HIT');
|
||||
// Sensitive headers never surface in the log.
|
||||
expect(JSON.stringify(loggedMessages)).not.toContain('topsecret');
|
||||
expect(JSON.stringify(loggedMessages)).not.toContain('set-cookie');
|
||||
expect(JSON.stringify(loggedMessages)).not.toContain('Set-Cookie');
|
||||
});
|
||||
|
||||
it('logs cacheHeaders=none when the response has no cache headers', async () => {
|
||||
const nativeHttp = vi.fn().mockResolvedValue({
|
||||
status: 200,
|
||||
headers: {},
|
||||
data: 'body',
|
||||
});
|
||||
const adapter = new WebDavHttpAdapter(
|
||||
makeDeps({ isNativePlatform: true, nativeHttp, logger }),
|
||||
);
|
||||
|
||||
await adapter.request({
|
||||
url: 'https://dav.example.com/sync/file',
|
||||
method: 'GET',
|
||||
});
|
||||
|
||||
const responseLog = loggedMessages.find((l) => l.msg.includes('.response()'));
|
||||
expect((responseLog?.meta as { cacheHeaders?: string }).cacheHeaders).toBe('none');
|
||||
});
|
||||
|
||||
it('logs structured errorMeta on unexpected error (no raw Error)', async () => {
|
||||
const fetchImpl = vi.fn().mockRejectedValue(new Error('boom-url-leak'));
|
||||
const adapter = new WebDavHttpAdapter(
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue