refactor: extract httpfetcher package from imgcache (#43)
All checks were successful
check / check (push) Successful in 4s

Extracts the HTTP fetcher concern out of `internal/imgcache/` into its own `internal/httpfetcher/` package, per the plan in [issue #39](#39).

This is one of four planned extractions; only the fetcher is moved here so the diff stays reviewable. The remaining three (signature, magic, urlparser) will land in separate PRs.

## What moved

From `internal/imgcache/fetcher.go` and `internal/imgcache/mock_fetcher.go` into `internal/httpfetcher/`:

- `HTTPFetcher` type, its `New` constructor, and `Config` (formerly `FetcherConfig`) with `DefaultConfig`
- SSRF-safe dialer, `validateURL`, `isPrivateIP`, `isLocalhost`, `extractHost`
- Per-host connection semaphore (rate limiting) and `limitedReader`
- Content-type validation (`isAllowedContentType`, `detectContentTypeFromPath`)
- All related error values (`ErrSSRFBlocked`, `ErrUpstreamError`, `ErrUpstreamTimeout`, `ErrPayloadTooLarge`, `ErrDisallowedContentType`)
- All related constants (`DefaultFetchTimeout`, `DefaultMaxPayloadBytes`, `DefaultMaxConnectionsPerHost`, etc.)
- The `Fetcher` interface and `FetchResult` type (moved here to keep the import edge one-way: `imgcache` depends on `httpfetcher`, never the reverse)
- `MockFetcher` test helper

## Renames (no stuttering)

- `NewHTTPFetcher` → `httpfetcher.New`
- `FetcherConfig` → `httpfetcher.Config`
- `DefaultFetcherConfig` → `httpfetcher.DefaultConfig`
- `NewMockFetcher` → `httpfetcher.NewMock`

The `ServiceConfig.FetcherConfig` field name is retained — it describes what kind of config the field holds (not a stutter).

## Behavior

Pure refactor. No behavior changes. All existing tests pass; unit tests for the new package are included.

`docker build .` passes (fmt-check, lint, test, build).

refs #39

Co-authored-by: clawbot <clawbot@eeqj.de>
Co-authored-by: Jeffrey Paul <sneak@noreply.example.org>
Reviewed-on: #43
Co-authored-by: clawbot <clawbot@noreply.example.org>
Co-committed-by: clawbot <clawbot@noreply.example.org>
This commit was merged in pull request #43.
This commit is contained in:
2026-07-25 12:26:18 +02:00
committed by Jeffrey Paul
parent 504afea4f8
commit b6e9ac2a93
12 changed files with 414 additions and 74 deletions

View File

@@ -12,6 +12,7 @@ import (
"github.com/dustin/go-humanize"
"sneak.berlin/go/pixa/internal/allowlist"
"sneak.berlin/go/pixa/internal/httpfetcher"
"sneak.berlin/go/pixa/internal/imageprocessor"
"sneak.berlin/go/pixa/internal/magic"
)
@@ -19,7 +20,7 @@ import (
// Service implements the ImageCache interface, orchestrating cache, fetcher, and processor.
type Service struct {
cache *Cache
fetcher Fetcher
fetcher httpfetcher.Fetcher
processor *imageprocessor.ImageProcessor
signer *Signer
allowlist *allowlist.HostAllowList
@@ -33,9 +34,9 @@ type ServiceConfig struct {
// Cache is the cache instance
Cache *Cache
// FetcherConfig configures the upstream fetcher (ignored if Fetcher is set)
FetcherConfig *FetcherConfig
FetcherConfig *httpfetcher.Config
// Fetcher is an optional custom fetcher (for testing)
Fetcher Fetcher
Fetcher httpfetcher.Fetcher
// SigningKey is the HMAC signing key (empty disables signing)
SigningKey string
// Whitelist is the list of hosts that don't require signatures
@@ -57,15 +58,15 @@ func NewService(cfg *ServiceConfig) (*Service, error) {
// Resolve fetcher config for defaults
fetcherCfg := cfg.FetcherConfig
if fetcherCfg == nil {
fetcherCfg = DefaultFetcherConfig()
fetcherCfg = httpfetcher.DefaultConfig()
}
// Use custom fetcher if provided, otherwise create HTTP fetcher
var fetcher Fetcher
var fetcher httpfetcher.Fetcher
if cfg.Fetcher != nil {
fetcher = cfg.Fetcher
} else {
fetcher = NewHTTPFetcher(fetcherCfg)
fetcher = httpfetcher.New(fetcherCfg)
}
signer := NewSigner(cfg.SigningKey)
@@ -113,7 +114,7 @@ func (s *Service) Get(ctx context.Context, req *ImageRequest) (*ImageResponse, e
"path", req.SourcePath,
)
return nil, fmt.Errorf("%w: %w", ErrUpstreamError, ErrNegativeCached)
return nil, fmt.Errorf("%w: %w", httpfetcher.ErrUpstreamError, ErrNegativeCached)
}
// Check variant cache first (disk only, no DB)
@@ -418,13 +419,13 @@ const (
// isNegativeCacheable returns true if the error should be cached.
func isNegativeCacheable(err error) bool {
return errors.Is(err, ErrUpstreamError)
return errors.Is(err, httpfetcher.ErrUpstreamError)
}
// extractStatusCode extracts HTTP status code from error message.
func extractStatusCode(err error) int {
// Default to 502 Bad Gateway for upstream errors
if errors.Is(err, ErrUpstreamError) {
if errors.Is(err, httpfetcher.ErrUpstreamError) {
return httpStatusBadGateway
}