diff --git a/internal/handlers/fetcher_internal_test.go b/internal/handlers/fetcher_internal_test.go new file mode 100644 index 0000000..d78091a --- /dev/null +++ b/internal/handlers/fetcher_internal_test.go @@ -0,0 +1,61 @@ +package handlers + +import ( + "net/http" + "net/netip" + "path/filepath" + "testing" + "time" + + "github.com/go-chi/chi/v5" + "go.uber.org/fx" + "go.uber.org/fx/fxtest" + + "sneak.berlin/go/pixa/internal/config" + "sneak.berlin/go/pixa/internal/database" + "sneak.berlin/go/pixa/internal/globals" + "sneak.berlin/go/pixa/internal/healthcheck" + "sneak.berlin/go/pixa/internal/logger" +) + +// TestHandlersBuildTheirOwnFetcherWhenNoneIsProvided builds the handlers as +// pixad does, in an fx app that provides no fetcher, and requests an image +// from 192.0.2.10, which is on the allowlist and in blocked_networks. The URL +// check accepts that address; only the dialer that refuses internal +// addresses checks blocked_networks, so the answer is 403 only if the +// fetcher the handlers build from the config connects with that dialer. Any +// other dialer would try to connect until the upstream fetch timeout, which +// is short so that the test then fails quickly. +func TestHandlersBuildTheirOwnFetcherWhenNoneIsProvided(t *testing.T) { + t.Parallel() + + const host = "192.0.2.10" + + stateDir := t.TempDir() + cfg := &config.Config{ + SigningKey: testSigningKey, + StateDir: stateDir, + DBURL: "file:" + filepath.Join(stateDir, "state.sqlite3"), + AllowlistHosts: []string{host}, + BlockedNetworks: []netip.Prefix{netip.MustParsePrefix("192.0.2.0/24")}, + UpstreamFetchTimeout: 2 * time.Second, + // With no connection slots, the fetch would fail before dialing. + UpstreamConnections: config.DefaultUpstreamConnections, + } + + var h *Handlers + + app := fxtest.New(t, + fx.Supply(cfg), + fx.Provide(globals.New, logger.New, database.New, healthcheck.New, New), + fx.Populate(&h), + ) + app.RequireStart() + t.Cleanup(app.RequireStop) + + r := chi.NewRouter() + r.Get("/v1/image/*", h.HandleImage()) + + rec := sendGet(t, r, photoURL(host)) + checkErrorBody(t, rec, http.StatusForbidden, "forbidden") +} diff --git a/internal/handlers/handlers.go b/internal/handlers/handlers.go index a65f3b5..0414026 100644 --- a/internal/handlers/handlers.go +++ b/internal/handlers/handlers.go @@ -28,6 +28,11 @@ type Params struct { Healthcheck *healthcheck.Healthcheck Database *database.Database Config *config.Config + + // Fetcher, when provided, fetches upstream images in place of the + // fetcher the handlers build from the config. Only tests provide one; + // pixad does not. + Fetcher httpfetcher.Fetcher `optional:"true"` } // Handlers provides HTTP request handlers. @@ -36,6 +41,7 @@ type Handlers struct { hc *healthcheck.Healthcheck db *database.Database config *config.Config + fetcher httpfetcher.Fetcher imgSvc *imgcache.Service imgCache *imgcache.Cache sessMgr *session.Manager @@ -59,6 +65,7 @@ func New(lc fx.Lifecycle, params Params) (*Handlers, error) { hc: params.Healthcheck, db: params.Database, config: params.Config, + fetcher: params.Fetcher, csrfProtect: csrfProtect, refererBlocklist: allowlist.New(params.Config.RefererBlocklist), } @@ -128,10 +135,12 @@ func (s *Handlers) initImageService() error { fetcherCfg.MaxConnections = s.config.UpstreamConnections fetcherCfg.BlockedNetworks = s.config.BlockedNetworks - // Create the service + // Create the service. With no fetcher provided, it builds its own from + // fetcherCfg. svc, err := imgcache.NewService(&imgcache.ServiceConfig{ Cache: cache, FetcherConfig: fetcherCfg, + Fetcher: s.fetcher, SigningKey: s.config.SigningKey, Allowlist: s.config.AllowlistHosts, MaxConcurrentProcessing: s.config.MaxConcurrentProcessing, diff --git a/internal/imgcache/service.go b/internal/imgcache/service.go index 562595c..2a40c8c 100644 --- a/internal/imgcache/service.go +++ b/internal/imgcache/service.go @@ -42,7 +42,8 @@ type Service struct { type ServiceConfig struct { // Cache is the cache instance Cache *Cache - // FetcherConfig configures the upstream fetcher (ignored if Fetcher is set) + // FetcherConfig configures the upstream fetcher built when Fetcher is + // not set. Its AllowHTTP and MaxResponseSize are used either way. FetcherConfig *httpfetcher.Config // Fetcher is an optional custom fetcher (for testing) Fetcher httpfetcher.Fetcher