Read referer_blocklist through strictLoader and fix test lint
newFromSmartConfig went over the line limit, so referer_blocklist is now read through strictLoader, like the other typed settings, instead of its own parse step. The tests stop repeating string literals that goconst counts: the environment test uses other hosts, two case names change, and the handler test names each image route's URL instead of its path. Model: opus-5-5
This commit is contained in:
@@ -266,11 +266,6 @@ func newFromSmartConfig(sc *smartconfig.Config) (*Config, error) {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
refererBlocklist, err := parseHostList(sc, keyRefererBlocklist)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
// parseCIDRList returns a nil slice only when the key is absent; an
|
||||
// explicitly empty list ([]) comes back non-nil and empty. An omitted
|
||||
// key takes the RFC 1918 default, while an explicit empty list is left
|
||||
@@ -280,7 +275,6 @@ func newFromSmartConfig(sc *smartconfig.Config) (*Config, error) {
|
||||
}
|
||||
|
||||
loader := &strictLoader{sc: sc}
|
||||
|
||||
c := &Config{
|
||||
Debug: loader.boolVal(keyDebug, false),
|
||||
MaintenanceMode: loader.boolVal(keyMaintenanceMode, false),
|
||||
@@ -311,7 +305,7 @@ func newFromSmartConfig(sc *smartconfig.Config) (*Config, error) {
|
||||
CacheMaxBytes: loader.int64Val(keyCacheMaxBytes, 0),
|
||||
BlockedNetworks: blockedNetworks,
|
||||
TrustedProxies: trustedProxies,
|
||||
RefererBlocklist: refererBlocklist,
|
||||
RefererBlocklist: loader.hostListVal(keyRefererBlocklist),
|
||||
}
|
||||
|
||||
// The default for an omitted cache_max_bytes is worked out when
|
||||
@@ -896,6 +890,19 @@ func (l *strictLoader) boolVal(key string, defaultVal bool) bool {
|
||||
return val
|
||||
}
|
||||
|
||||
func (l *strictLoader) hostListVal(key string) []string {
|
||||
if l.err != nil {
|
||||
return nil
|
||||
}
|
||||
|
||||
val, err := parseHostList(l.sc, key)
|
||||
if err != nil {
|
||||
l.err = err
|
||||
}
|
||||
|
||||
return val
|
||||
}
|
||||
|
||||
// getString returns the string value for key, or defaultVal if the key
|
||||
// is omitted. A present value that is not a string, or is explicitly
|
||||
// null, is an error.
|
||||
|
||||
@@ -65,7 +65,7 @@ func TestEnvironmentSetsEveryKey(t *testing.T) {
|
||||
t.Setenv("PIXA_METRICS_PASSWORD", "metricspass")
|
||||
t.Setenv("PIXA_SIGNING_KEY", validTestSigningKey)
|
||||
t.Setenv("PIXA_ALLOWLIST_HOSTS", "s3.sneak.cloud,.example.com")
|
||||
t.Setenv("PIXA_REFERER_BLOCKLIST", "leech.example,.hotlinker.example")
|
||||
t.Setenv("PIXA_REFERER_BLOCKLIST", "hotlinker.example,.leech.example")
|
||||
t.Setenv("PIXA_ALLOW_HTTP", "true")
|
||||
t.Setenv("PIXA_UPSTREAM_CONNECTIONS_PER_HOST", "5")
|
||||
t.Setenv("PIXA_UPSTREAM_CONNECTIONS", "10")
|
||||
@@ -94,7 +94,7 @@ func TestEnvironmentSetsEveryKey(t *testing.T) {
|
||||
MetricsPassword: "metricspass",
|
||||
SigningKey: validTestSigningKey,
|
||||
AllowlistHosts: []string{testHostS3, ".example.com"},
|
||||
RefererBlocklist: []string{"leech.example", ".hotlinker.example"},
|
||||
RefererBlocklist: []string{"hotlinker.example", ".leech.example"},
|
||||
AllowHTTP: true,
|
||||
UpstreamConnectionsPerHost: 5,
|
||||
UpstreamConnections: 10,
|
||||
|
||||
@@ -47,14 +47,14 @@ func TestRefererBlocklistInvalidAbortsStartup(t *testing.T) {
|
||||
|
||||
runAbortCases(t, []abortCase{
|
||||
{
|
||||
name: "url",
|
||||
name: "entry with a scheme",
|
||||
yaml: signingKeyLine + "referer_blocklist:\n - https://leech.example\n",
|
||||
wantErrSubstrings: []string{
|
||||
keyRefererBlocklist, "https://leech.example",
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "path",
|
||||
name: "entry with a path",
|
||||
yaml: signingKeyLine + "referer_blocklist:\n - leech.example/page\n",
|
||||
wantErrSubstrings: []string{
|
||||
keyRefererBlocklist, "leech.example/page",
|
||||
@@ -76,7 +76,7 @@ func TestRefererBlocklistInvalidAbortsStartup(t *testing.T) {
|
||||
wantErrSubstrings: []string{keyRefererBlocklist, "42"},
|
||||
},
|
||||
{
|
||||
name: "null",
|
||||
name: "null value",
|
||||
yaml: signingKeyLine + "referer_blocklist:\n",
|
||||
wantErrSubstrings: []string{keyRefererBlocklist, nullValueText},
|
||||
},
|
||||
|
||||
@@ -133,19 +133,24 @@ func TestRefererBlocklist(t *testing.T) {
|
||||
"https://hotlinker.example.evil.example/", http.StatusOK},
|
||||
}
|
||||
|
||||
for _, route := range []string{"/v1/image/", "/v1/e/"} {
|
||||
// The photo's URL on each image route.
|
||||
photoURLs := map[string]func(t *testing.T, h *Handlers) string{
|
||||
"plain URL": func(t *testing.T, _ *Handlers) string {
|
||||
t.Helper()
|
||||
|
||||
return photoURL(allowlistedHost)
|
||||
},
|
||||
"encrypted URL": encPhotoURL,
|
||||
}
|
||||
|
||||
for urlName, photoURLFor := range photoURLs {
|
||||
for _, tc := range cases {
|
||||
t.Run(route+" "+tc.name, func(t *testing.T) {
|
||||
t.Run(urlName+", "+tc.name, func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
routes, h, fetcher := newRefererRoutes(t)
|
||||
|
||||
target := photoURL(allowlistedHost)
|
||||
if route == "/v1/e/" {
|
||||
target = encPhotoURL(t, h)
|
||||
}
|
||||
|
||||
rec := getWithReferer(t, routes, target, tc.referer)
|
||||
rec := getWithReferer(t, routes, photoURLFor(t, h), tc.referer)
|
||||
|
||||
if tc.want == http.StatusOK {
|
||||
requireServedPhoto(t, rec)
|
||||
|
||||
Reference in New Issue
Block a user