fix(filtering): use HTTP headers as version fallback for uBlock lists
- commit
- 046c1d7884bfc9f3f58c1a42c703387212ae24fa
- author
- bnema · · 2025-11-29 13:47 UTC
- parent
- dcff304
- signature
- unsigned
2 files · +61 -12
M internal/filtering/updater.go +39 -11
| @@ -36,6 +36,7 @@ type FilterUpdate struct { | |||
| 36 | 36 | Previous []byte | |
| 37 | 37 | Current []byte | |
| 38 | 38 | Hash string | |
| 39 | + | Version string // Version extracted from content or HTTP headers | |
| 39 | 40 | Compiled *CompiledFilters | |
| 40 | 41 | } | |
| 41 | 42 | ||
| @@ -176,39 +177,51 @@ func (fu *FilterUpdater) fetchRemoteVersion(ctx context.Context, url string) (st | |||
| 176 | 177 | return "", fmt.Errorf("failed to read response: %w", err) | |
| 177 | 178 | } | |
| 178 | 179 | ||
| 179 | - | // Parse version from header (format: "! Version: 202511291248") | |
| 180 | + | // Parse version from content header (format: "! Version: 202511291248") | |
| 180 | 181 | scanner := bufio.NewScanner(bytes.NewReader(data)) | |
| 181 | 182 | for scanner.Scan() { | |
| 182 | 183 | line := scanner.Text() | |
| 183 | 184 | if strings.HasPrefix(line, "! Version:") { | |
| 184 | 185 | version := strings.TrimSpace(strings.TrimPrefix(line, "! Version:")) | |
| 185 | - | return version, nil | |
| 186 | + | if version != "" && !strings.Contains(version, "%") { | |
| 187 | + | return version, nil | |
| 188 | + | } | |
| 186 | 189 | } | |
| 187 | 190 | } | |
| 188 | 191 | ||
| 189 | - | return "", fmt.Errorf("version header not found in filter list") | |
| 192 | + | // Fallback: use HTTP headers (ETag or Last-Modified) | |
| 193 | + | if etag := resp.Header.Get("ETag"); etag != "" { | |
| 194 | + | return "etag:" + etag, nil | |
| 195 | + | } | |
| 196 | + | if lastMod := resp.Header.Get("Last-Modified"); lastMod != "" { | |
| 197 | + | return "lastmod:" + lastMod, nil | |
| 198 | + | } | |
| 199 | + | ||
| 200 | + | return "", fmt.Errorf("no version info found for filter list") | |
| 190 | 201 | } | |
| 191 | 202 | ||
| 192 | 203 | // needsUpdate determines if a filter list needs updating based on version comparison | |
| 193 | 204 | func (fu *FilterUpdater) needsUpdate(ctx context.Context, url string) bool { | |
| 205 | + | // Extract short name for logging | |
| 206 | + | shortName := url[strings.LastIndex(url, "/")+1:] | |
| 207 | + | ||
| 194 | 208 | remoteVersion, err := fu.fetchRemoteVersion(ctx, url) | |
| 195 | 209 | if err != nil { | |
| 196 | - | logging.Debug(fmt.Sprintf("Failed to fetch remote version for %s: %v", url, err)) | |
| 197 | - | // On error, assume update needed to be safe | |
| 210 | + | logging.Info(fmt.Sprintf("[filtering] %s: fetch failed, will update (%v)", shortName, err)) | |
| 198 | 211 | return true | |
| 199 | 212 | } | |
| 200 | 213 | ||
| 201 | 214 | storedVersion := fu.manager.store.GetSourceVersion(url) | |
| 202 | 215 | if storedVersion == "" { | |
| 203 | - | logging.Debug(fmt.Sprintf("No stored version for %s, update needed", url)) | |
| 216 | + | logging.Info(fmt.Sprintf("[filtering] %s: no stored version, will update", shortName)) | |
| 204 | 217 | return true | |
| 205 | 218 | } | |
| 206 | 219 | ||
| 207 | 220 | needsUpdate := remoteVersion != storedVersion | |
| 208 | 221 | if needsUpdate { | |
| 209 | - | logging.Info(fmt.Sprintf("Filter %s has new version: %s (stored: %s)", url, remoteVersion, storedVersion)) | |
| 222 | + | logging.Info(fmt.Sprintf("[filtering] %s: new version %s (was %s)", shortName, remoteVersion, storedVersion)) | |
| 210 | 223 | } else { | |
| 211 | - | logging.Debug(fmt.Sprintf("Filter %s is up to date: %s", url, remoteVersion)) | |
| 224 | + | logging.Info(fmt.Sprintf("[filtering] %s: up to date (%s)", shortName, storedVersion)) | |
| 212 | 225 | } | |
| 213 | 226 | ||
| 214 | 227 | return needsUpdate | |
| @@ -265,12 +278,23 @@ func (fu *FilterUpdater) downloadUpdate(ctx context.Context, url, lastModified, | |||
| 265 | 278 | // Calculate content hash | |
| 266 | 279 | hash := fmt.Sprintf("%x", sha256.Sum256(currentContent)) | |
| 267 | 280 | ||
| 281 | + | // Extract version from content or use HTTP headers as fallback | |
| 282 | + | version := fu.extractVersion(currentContent) | |
| 283 | + | if version == "" { | |
| 284 | + | if etagHeader := resp.Header.Get("ETag"); etagHeader != "" { | |
| 285 | + | version = "etag:" + etagHeader | |
| 286 | + | } else if lastModHeader := resp.Header.Get("Last-Modified"); lastModHeader != "" { | |
| 287 | + | version = "lastmod:" + lastModHeader | |
| 288 | + | } | |
| 289 | + | } | |
| 290 | + | ||
| 268 | 291 | // Create update object | |
| 269 | 292 | update := &FilterUpdate{ | |
| 270 | 293 | ListID: fu.getListID(url), | |
| 271 | 294 | URL: url, | |
| 272 | 295 | Current: currentContent, | |
| 273 | 296 | Hash: hash, | |
| 297 | + | Version: version, | |
| 274 | 298 | } | |
| 275 | 299 | ||
| 276 | 300 | // Get previous version for diffing (if available) | |
| @@ -453,8 +477,8 @@ func (fu *FilterUpdater) applyUpdate(update *FilterUpdate) error { | |||
| 453 | 477 | fu.storePreviousContent(update.ListID, update.Current) | |
| 454 | 478 | ||
| 455 | 479 | // Store the version for future update checks | |
| 456 | - | if version := fu.extractVersion(update.Current); version != "" { | |
| 457 | - | if err := fu.manager.store.SetSourceVersion(update.URL, version); err != nil { | |
| 480 | + | if update.Version != "" { | |
| 481 | + | if err := fu.manager.store.SetSourceVersion(update.URL, update.Version); err != nil { | |
| 458 | 482 | logging.Warn(fmt.Sprintf("Failed to store version for %s: %v", update.URL, err)) | |
| 459 | 483 | } | |
| 460 | 484 | } | |
| @@ -470,7 +494,11 @@ func (fu *FilterUpdater) extractVersion(content []byte) string { | |||
| 470 | 494 | for i := 0; i < 20 && scanner.Scan(); i++ { | |
| 471 | 495 | line := scanner.Text() | |
| 472 | 496 | if strings.HasPrefix(line, "! Version:") { | |
| 473 | - | return strings.TrimSpace(strings.TrimPrefix(line, "! Version:")) | |
| 497 | + | version := strings.TrimSpace(strings.TrimPrefix(line, "! Version:")) | |
| 498 | + | // Skip placeholder values like %timestamp% | |
| 499 | + | if version != "" && !strings.Contains(version, "%") { | |
| 500 | + | return version | |
| 501 | + | } | |
| 474 | 502 | } | |
| 475 | 503 | } | |
| 476 | 504 | return "" | |
| @@ -36,6 +36,7 @@ type FilterUpdate struct { | |||||
| 36 | Previous []byte | 36 | Previous []byte | ||
| 37 | Current []byte | 37 | Current []byte | ||
| 38 | Hash string | 38 | Hash string | ||
| 39 | + | Version string // Version extracted from content or HTTP headers | |||
| 39 | Compiled *CompiledFilters | 40 | Compiled *CompiledFilters | ||
| 40 | } | 41 | } | ||
| 41 | 42 | ||||
| @@ -176,39 +177,51 @@ func (fu *FilterUpdater) fetchRemoteVersion(ctx context.Context, url string) (st | |||||
| 176 | return "", fmt.Errorf("failed to read response: %w", err) | 177 | return "", fmt.Errorf("failed to read response: %w", err) | ||
| 177 | } | 178 | } | ||
| 178 | 179 | ||||
| 179 | - | // Parse version from header (format: "! Version: 202511291248") | 180 | + | // Parse version from content header (format: "! Version: 202511291248") |
| 180 | scanner := bufio.NewScanner(bytes.NewReader(data)) | 181 | scanner := bufio.NewScanner(bytes.NewReader(data)) | ||
| 181 | for scanner.Scan() { | 182 | for scanner.Scan() { | ||
| 182 | line := scanner.Text() | 183 | line := scanner.Text() | ||
| 183 | if strings.HasPrefix(line, "! Version:") { | 184 | if strings.HasPrefix(line, "! Version:") { | ||
| 184 | version := strings.TrimSpace(strings.TrimPrefix(line, "! Version:")) | 185 | version := strings.TrimSpace(strings.TrimPrefix(line, "! Version:")) | ||
| 185 | - | return version, nil | 186 | + | if version != "" && !strings.Contains(version, "%") { |
| 187 | + | return version, nil | |||
| 188 | + | } | |||
| 186 | } | 189 | } | ||
| 187 | } | 190 | } | ||
| 188 | 191 | ||||
| 189 | - | return "", fmt.Errorf("version header not found in filter list") | 192 | + | // Fallback: use HTTP headers (ETag or Last-Modified) |
| 193 | + | if etag := resp.Header.Get("ETag"); etag != "" { | |||
| 194 | + | return "etag:" + etag, nil | |||
| 195 | + | } | |||
| 196 | + | if lastMod := resp.Header.Get("Last-Modified"); lastMod != "" { | |||
| 197 | + | return "lastmod:" + lastMod, nil | |||
| 198 | + | } | |||
| 199 | + | ||||
| 200 | + | return "", fmt.Errorf("no version info found for filter list") | |||
| 190 | } | 201 | } | ||
| 191 | 202 | ||||
| 192 | // needsUpdate determines if a filter list needs updating based on version comparison | 203 | // needsUpdate determines if a filter list needs updating based on version comparison | ||
| 193 | func (fu *FilterUpdater) needsUpdate(ctx context.Context, url string) bool { | 204 | func (fu *FilterUpdater) needsUpdate(ctx context.Context, url string) bool { | ||
| 205 | + | // Extract short name for logging | |||
| 206 | + | shortName := url[strings.LastIndex(url, "/")+1:] | |||
| 207 | + | ||||
| 194 | remoteVersion, err := fu.fetchRemoteVersion(ctx, url) | 208 | remoteVersion, err := fu.fetchRemoteVersion(ctx, url) | ||
| 195 | if err != nil { | 209 | if err != nil { | ||
| 196 | - | logging.Debug(fmt.Sprintf("Failed to fetch remote version for %s: %v", url, err)) | 210 | + | logging.Info(fmt.Sprintf("[filtering] %s: fetch failed, will update (%v)", shortName, err)) |
| 197 | - | // On error, assume update needed to be safe | |||
| 198 | return true | 211 | return true | ||
| 199 | } | 212 | } | ||
| 200 | 213 | ||||
| 201 | storedVersion := fu.manager.store.GetSourceVersion(url) | 214 | storedVersion := fu.manager.store.GetSourceVersion(url) | ||
| 202 | if storedVersion == "" { | 215 | if storedVersion == "" { | ||
| 203 | - | logging.Debug(fmt.Sprintf("No stored version for %s, update needed", url)) | 216 | + | logging.Info(fmt.Sprintf("[filtering] %s: no stored version, will update", shortName)) |
| 204 | return true | 217 | return true | ||
| 205 | } | 218 | } | ||
| 206 | 219 | ||||
| 207 | needsUpdate := remoteVersion != storedVersion | 220 | needsUpdate := remoteVersion != storedVersion | ||
| 208 | if needsUpdate { | 221 | if needsUpdate { | ||
| 209 | - | logging.Info(fmt.Sprintf("Filter %s has new version: %s (stored: %s)", url, remoteVersion, storedVersion)) | 222 | + | logging.Info(fmt.Sprintf("[filtering] %s: new version %s (was %s)", shortName, remoteVersion, storedVersion)) |
| 210 | } else { | 223 | } else { | ||
| 211 | - | logging.Debug(fmt.Sprintf("Filter %s is up to date: %s", url, remoteVersion)) | 224 | + | logging.Info(fmt.Sprintf("[filtering] %s: up to date (%s)", shortName, storedVersion)) |
| 212 | } | 225 | } | ||
| 213 | 226 | ||||
| 214 | return needsUpdate | 227 | return needsUpdate | ||
| @@ -265,12 +278,23 @@ func (fu *FilterUpdater) downloadUpdate(ctx context.Context, url, lastModified, | |||||
| 265 | // Calculate content hash | 278 | // Calculate content hash | ||
| 266 | hash := fmt.Sprintf("%x", sha256.Sum256(currentContent)) | 279 | hash := fmt.Sprintf("%x", sha256.Sum256(currentContent)) | ||
| 267 | 280 | ||||
| 281 | + | // Extract version from content or use HTTP headers as fallback | |||
| 282 | + | version := fu.extractVersion(currentContent) | |||
| 283 | + | if version == "" { | |||
| 284 | + | if etagHeader := resp.Header.Get("ETag"); etagHeader != "" { | |||
| 285 | + | version = "etag:" + etagHeader | |||
| 286 | + | } else if lastModHeader := resp.Header.Get("Last-Modified"); lastModHeader != "" { | |||
| 287 | + | version = "lastmod:" + lastModHeader | |||
| 288 | + | } | |||
| 289 | + | } | |||
| 290 | + | ||||
| 268 | // Create update object | 291 | // Create update object | ||
| 269 | update := &FilterUpdate{ | 292 | update := &FilterUpdate{ | ||
| 270 | ListID: fu.getListID(url), | 293 | ListID: fu.getListID(url), | ||
| 271 | URL: url, | 294 | URL: url, | ||
| 272 | Current: currentContent, | 295 | Current: currentContent, | ||
| 273 | Hash: hash, | 296 | Hash: hash, | ||
| 297 | + | Version: version, | |||
| 274 | } | 298 | } | ||
| 275 | 299 | ||||
| 276 | // Get previous version for diffing (if available) | 300 | // Get previous version for diffing (if available) | ||
| @@ -453,8 +477,8 @@ func (fu *FilterUpdater) applyUpdate(update *FilterUpdate) error { | |||||
| 453 | fu.storePreviousContent(update.ListID, update.Current) | 477 | fu.storePreviousContent(update.ListID, update.Current) | ||
| 454 | 478 | ||||
| 455 | // Store the version for future update checks | 479 | // Store the version for future update checks | ||
| 456 | - | if version := fu.extractVersion(update.Current); version != "" { | 480 | + | if update.Version != "" { |
| 457 | - | if err := fu.manager.store.SetSourceVersion(update.URL, version); err != nil { | 481 | + | if err := fu.manager.store.SetSourceVersion(update.URL, update.Version); err != nil { |
| 458 | logging.Warn(fmt.Sprintf("Failed to store version for %s: %v", update.URL, err)) | 482 | logging.Warn(fmt.Sprintf("Failed to store version for %s: %v", update.URL, err)) | ||
| 459 | } | 483 | } | ||
| 460 | } | 484 | } | ||
| @@ -470,7 +494,11 @@ func (fu *FilterUpdater) extractVersion(content []byte) string { | |||||
| 470 | for i := 0; i < 20 && scanner.Scan(); i++ { | 494 | for i := 0; i < 20 && scanner.Scan(); i++ { | ||
| 471 | line := scanner.Text() | 495 | line := scanner.Text() | ||
| 472 | if strings.HasPrefix(line, "! Version:") { | 496 | if strings.HasPrefix(line, "! Version:") { | ||
| 473 | - | return strings.TrimSpace(strings.TrimPrefix(line, "! Version:")) | 497 | + | version := strings.TrimSpace(strings.TrimPrefix(line, "! Version:")) |
| 498 | + | // Skip placeholder values like %timestamp% | |||
| 499 | + | if version != "" && !strings.Contains(version, "%") { | |||
| 500 | + | return version | |||
| 501 | + | } | |||
| 474 | } | 502 | } | ||
| 475 | } | 503 | } | ||
| 476 | return "" | 504 | return "" | ||
M internal/filtering/updater_test.go +22 -1
| @@ -18,6 +18,7 @@ func TestFetchRemoteVersion(t *testing.T) { | |||
| 18 | 18 | responseStatus int | |
| 19 | 19 | wantVersion string | |
| 20 | 20 | wantErr bool | |
| 21 | + | useETag bool | |
| 21 | 22 | }{ | |
| 22 | 23 | { | |
| 23 | 24 | name: "valid version header", | |
| @@ -41,7 +42,17 @@ func TestFetchRemoteVersion(t *testing.T) { | |||
| 41 | 42 | wantErr: false, | |
| 42 | 43 | }, | |
| 43 | 44 | { | |
| 44 | - | name: "no version header", | |
| 45 | + | name: "placeholder version falls back to ETag", | |
| 46 | + | responseBody: `! Title: uBlock filters | |
| 47 | + | ! Last modified: %timestamp% | |
| 48 | + | `, | |
| 49 | + | responseStatus: http.StatusOK, | |
| 50 | + | wantVersion: `etag:"abc123"`, | |
| 51 | + | wantErr: false, | |
| 52 | + | useETag: true, | |
| 53 | + | }, | |
| 54 | + | { | |
| 55 | + | name: "no version info returns error", | |
| 45 | 56 | responseBody: `[Adblock Plus 2.0]\n! Title: No Version\n`, | |
| 46 | 57 | responseStatus: http.StatusOK, | |
| 47 | 58 | wantVersion: "", | |
| @@ -64,6 +75,9 @@ func TestFetchRemoteVersion(t *testing.T) { | |||
| 64 | 75 | rangeHeader := r.Header.Get("Range") | |
| 65 | 76 | assert.Contains(t, rangeHeader, "bytes=0-") | |
| 66 | 77 | ||
| 78 | + | if tt.useETag { | |
| 79 | + | w.Header().Set("ETag", `"abc123"`) | |
| 80 | + | } | |
| 67 | 81 | w.WriteHeader(tt.responseStatus) | |
| 68 | 82 | _, _ = w.Write([]byte(tt.responseBody)) | |
| 69 | 83 | })) | |
| @@ -111,6 +125,13 @@ func TestExtractVersion(t *testing.T) { | |||
| 111 | 125 | content: []byte(`! Title: No Version\n! Author: Test`), | |
| 112 | 126 | want: "", | |
| 113 | 127 | }, | |
| 128 | + | { | |
| 129 | + | name: "placeholder version ignored", | |
| 130 | + | content: []byte(`! Title: uBlock filters | |
| 131 | + | ! Version: %timestamp% | |
| 132 | + | ! Expires: 5 days`), | |
| 133 | + | want: "", | |
| 134 | + | }, | |
| 114 | 135 | { | |
| 115 | 136 | name: "empty content", | |
| 116 | 137 | content: []byte{}, | |
| @@ -18,6 +18,7 @@ func TestFetchRemoteVersion(t *testing.T) { | |||||
| 18 | responseStatus int | 18 | responseStatus int | ||
| 19 | wantVersion string | 19 | wantVersion string | ||
| 20 | wantErr bool | 20 | wantErr bool | ||
| 21 | + | useETag bool | |||
| 21 | }{ | 22 | }{ | ||
| 22 | { | 23 | { | ||
| 23 | name: "valid version header", | 24 | name: "valid version header", | ||
| @@ -41,7 +42,17 @@ func TestFetchRemoteVersion(t *testing.T) { | |||||
| 41 | wantErr: false, | 42 | wantErr: false, | ||
| 42 | }, | 43 | }, | ||
| 43 | { | 44 | { | ||
| 44 | - | name: "no version header", | 45 | + | name: "placeholder version falls back to ETag", |
| 46 | + | responseBody: `! Title: uBlock filters | |||
| 47 | + | ! Last modified: %timestamp% | |||
| 48 | + | `, | |||
| 49 | + | responseStatus: http.StatusOK, | |||
| 50 | + | wantVersion: `etag:"abc123"`, | |||
| 51 | + | wantErr: false, | |||
| 52 | + | useETag: true, | |||
| 53 | + | }, | |||
| 54 | + | { | |||
| 55 | + | name: "no version info returns error", | |||
| 45 | responseBody: `[Adblock Plus 2.0]\n! Title: No Version\n`, | 56 | responseBody: `[Adblock Plus 2.0]\n! Title: No Version\n`, | ||
| 46 | responseStatus: http.StatusOK, | 57 | responseStatus: http.StatusOK, | ||
| 47 | wantVersion: "", | 58 | wantVersion: "", | ||
| @@ -64,6 +75,9 @@ func TestFetchRemoteVersion(t *testing.T) { | |||||
| 64 | rangeHeader := r.Header.Get("Range") | 75 | rangeHeader := r.Header.Get("Range") | ||
| 65 | assert.Contains(t, rangeHeader, "bytes=0-") | 76 | assert.Contains(t, rangeHeader, "bytes=0-") | ||
| 66 | 77 | ||||
| 78 | + | if tt.useETag { | |||
| 79 | + | w.Header().Set("ETag", `"abc123"`) | |||
| 80 | + | } | |||
| 67 | w.WriteHeader(tt.responseStatus) | 81 | w.WriteHeader(tt.responseStatus) | ||
| 68 | _, _ = w.Write([]byte(tt.responseBody)) | 82 | _, _ = w.Write([]byte(tt.responseBody)) | ||
| 69 | })) | 83 | })) | ||
| @@ -111,6 +125,13 @@ func TestExtractVersion(t *testing.T) { | |||||
| 111 | content: []byte(`! Title: No Version\n! Author: Test`), | 125 | content: []byte(`! Title: No Version\n! Author: Test`), | ||
| 112 | want: "", | 126 | want: "", | ||
| 113 | }, | 127 | }, | ||
| 128 | + | { | |||
| 129 | + | name: "placeholder version ignored", | |||
| 130 | + | content: []byte(`! Title: uBlock filters | |||
| 131 | + | ! Version: %timestamp% | |||
| 132 | + | ! Expires: 5 days`), | |||
| 133 | + | want: "", | |||
| 134 | + | }, | |||
| 114 | { | 135 | { | ||
| 115 | name: "empty content", | 136 | name: "empty content", | ||
| 116 | content: []byte{}, | 137 | content: []byte{}, | ||