AdguardTeam / AdguardTeam/AdguardBrowserExtension

[investigate] Extension fails to start with ZodError after a few days due to non-atomic filter storage writes

Abierto
#3,552 7 comentarios 0 reacciones 1 asignado Asignado a @totadavid95 Ver en GitHub
Priority: P4
Lenguaje dominante
TypeScript
Estrellas
4.4k
Forks
449
Métricas de merge de PR
Sin PR fusionados en 30 d

Descripción

## Environment

- Extension version: 5.4.3.1 (MV2)
- Browser: Microsoft Edge 138 (Chromium-based), Windows 11
- Manifest: MV2

## Description

After a fresh install or storage clear, AdGuard works fine. However, after a few days of normal use, the extension gets stuck in a loading state (spinner on the icon). This is 100% reproducible — it always breaks after some days and requires reinstalling or clearing extension storage to fix.

## Error

The background page console shows:

```
[ext.Engine.start]: Start tswebextension...

ZodError: [
{ "code": "invalid_type", "expected": "string", "received": "null",
"path": ["filters", 0, "content"], "message": "Expected string, received null" },
{ "code": "invalid_type", "expected": "string", "received": "null",
"path": ["filters", 3, "content"], "message": "Expected string, received null" },
{ "code": "invalid_type", "expected": "string", "received": "null",
"path": ["filters", 4, "content"], "message": "Expected string, received null" }
]
at ZodObject.parse (tsurlfilter.js:56313)
at TsWebExtension.start (tswebextension.js:28805)
at Engine.start (background.js:48902)
at async App.asyncInit (background.js:70776)
at async App.init (background.js:70661)
```

## Root Cause Analysis

After tracing through the source code, I found the root cause is a **non-atomic write sequence** in `loadFilterRulesFromBackend` (background.js lines 58725-58778):

```
Line 58735: filterStateStorage.set(...) ← sync, in-memory, marks loaded: true
Line 58761: filterVersionStorage.set(...) ← sync, in-memory, stores new version + diffPath
Line 58777: FiltersStorage.set(...) ← async, IndexedDB — CAN FAIL
Line 58778: RawFiltersStorage.set(...) ← async, IndexedDB — CAN FAIL
```

The metadata (filter state + version) is written **synchronously to in-memory storage** BEFORE the actual filter content is written **asynchronously to IndexedDB**. If the IndexedDB write at line 58777 fails (stale IDB connection, storage pressure, browser suspending the background page, etc.), the metadata says "filter is loaded, version is updated" but `filterContent_` in IndexedDB is null/missing.

### Why it breaks "after a few days"

The automatic differential update scheduler runs periodically. After days of running, the cached `this.db` reference in `IDBStorage.getOpenedDb()` (line 33683) can become stale — the method checks `if (this.db)` and returns the cached reference without validating the connection is still alive. When `db.put()` fails on a stale connection, the error propagates up but is silently swallowed by `Promise.allSettled` in `CommonFilterApi.updateFilters` (line 58896).

### On next startup

1. `Engine.getConfiguration()` calls `FiltersStorage.get(filterId)` (line 48929)
2. `FiltersStorage.get()` reads `filterContent = null` from IndexedDB
3. Line 46951 only checks `filterContent === undefined`, **not null**, so null passes through
4. `new FilterList(null, ...)` is constructed, `getContent()` returns null
5. `configurationMV2Validator.parse()` (tswebextension.js:28805) validates against `content: z.string()` which rejects null → ZodError

## Suggested Fixes

### Fix 1: Correct the write order in `loadFilterRulesFromBackend`

Write IndexedDB content **first**, then update metadata only on success:

```javascript
// Write content to IndexedDB FIRST
await FiltersStorage.set(filterUpdateOptions.filterId, filter.join('\n'));
await RawFiltersStorage.set(filterUpdateOptions.filterId, rawFilter);

// THEN update metadata
filterStateStorage.set(filterUpdateOptions.filterId, { installed: true, loaded: true, ... });
filterVersionStorage.set(filterUpdateOptions.filterId, { version, diffPath, ... });
```

### Fix 2: Add null guard in `FiltersStorage.get()`

Line 46951 should guard against both null and undefined:

```javascript
// Before (only guards undefined):
if (filterContent === undefined) {
return undefined;
}

// After (guards both):
if (filterContent == null) {
return undefined;
}
```

### Fix 3: Validate IDB connection liveness in `IDBStorage.getOpenedDb()`

The cached `this.db` should be validated before reuse, or reconnect on failure:

```javascript
async getOpenedDb() {
if (this.db) {
try {
// Validate connection is still alive
this.db.transaction(this.store);
return this.db;
} catch {
this.db = undefined; // Connection stale, reconnect
}
}
// ... open new connection
}
```

Guía de contribución

No hay ninguna guía de contribución indexada para este repositorio

Evaluación

Este issue todavía no se ha evaluado.

Recibe los nuevos issues en tu correo

Un resumen breve de issues de GitHub para principiantes.