simplify error messages

jub0bs · Feb 29, 2024 · 9855a0f · 9855a0f
1 parent e0b3a11
commit 9855a0f
Show file tree

Hide file tree

Showing 3 changed files with 42 additions and 54 deletions.
diff --git a/fcors_invalid_policies_test.go b/fcors_invalid_policies_test.go
@@ -27,16 +27,16 @@ func TestInvalidPoliciesForAllowAccess(t *testing.T) {
 				fcors.FromOrigins("http://example.com:6060"),
 				risky.PrivateNetworkAccess(),
 			},
-			errorMsg: `fcors: insecure origin pattern "http://example.com:6060" requires option ` +
-				`risky.TolerateInsecureOrigins when Private-Network Access is enabled`,
+			errorMsg: `fcors: insecure origin patterns like "http://example.com:6060" ` +
+				`are by default prohibited when Private-Network Access is enabled`,
 		}, {
 			desc: "option PrivateNetworkAccessInNoCORSModeOnly is used and specified origin is insecure",
 			options: []fcors.OptionAnon{
 				fcors.FromOrigins("http://example.com:6060"),
 				risky.PrivateNetworkAccessInNoCORSModeOnly(),
 			},
-			errorMsg: `fcors: insecure origin pattern "http://example.com:6060" requires option ` +
-				`risky.TolerateInsecureOrigins when Private-Network Access is enabled`,
+			errorMsg: `fcors: insecure origin patterns like "http://example.com:6060" ` +
+				`are by default prohibited when Private-Network Access is enabled`,
 		}, {
 			desc:     "specified origin's host is an invalid IP address",
 			options:  []fcors.OptionAnon{fcors.FromOrigins("http://[::1]1:6060")},
@@ -106,8 +106,8 @@ func TestInvalidPoliciesForAllowAccess(t *testing.T) {
 				),
 				risky.PrivateNetworkAccess(),
 			},
-			errorMsg: `fcors: insecure origin patterns "http://example.com:6060", "http://*.example.com:6060" ` +
-				`require option risky.TolerateInsecureOrigins when Private-Network Access is enabled`,
+			errorMsg: `fcors: insecure origin patterns like "http://example.com:6060", "http://*.example.com:6060" ` +
+				`are by default prohibited when Private-Network Access is enabled`,
 		}, {
 			desc: "option PrivateNetworkAccessInNoCORSModeOnly is used and some origin patterns are insecure",
 			options: []fcors.OptionAnon{
@@ -117,8 +117,8 @@ func TestInvalidPoliciesForAllowAccess(t *testing.T) {
 				),
 				risky.PrivateNetworkAccessInNoCORSModeOnly(),
 			},
-			errorMsg: `fcors: insecure origin patterns "http://example.com:6060", "http://*.example.com:6060" ` +
-				`require option risky.TolerateInsecureOrigins when Private-Network Access is enabled`,
+			errorMsg: `fcors: insecure origin patterns like "http://example.com:6060", "http://*.example.com:6060" ` +
+				`are by default prohibited when Private-Network Access is enabled`,
 		}, {
 			desc:     "specified base origin's host is an invalid IP address",
 			options:  []fcors.OptionAnon{fcors.FromOrigins("http://*.[::1]1:6060")},
@@ -179,8 +179,8 @@ func TestInvalidPoliciesForAllowAccess(t *testing.T) {
 		}, {
 			desc:    "specified base origin's host is a public suffix",
 			options: []fcors.OptionAnon{fcors.FromOrigins("https://*.github.io")},
-			errorMsg: `fcors: origin patterns like "https://*.github.io" that allow arbitrary ` +
-				`subdomains of public suffix "github.io" are by default prohibited`,
+			errorMsg: `fcors: origin patterns like "https://*.github.io" that encompass ` +
+				`subdomains of a public suffix are by default prohibited`,
 		}, {
 			desc:     "missing call to FromOrigins or FromAnyOrigin",
 			options:  []fcors.OptionAnon{fcors.WithAnyMethod()},
@@ -613,25 +613,25 @@ func TestInvalidPoliciesForAllowAccessWithCredentials(t *testing.T) {
 		}, {
 			desc:    "specified origin is insecure",
 			options: []fcors.Option{fcors.FromOrigins("http://example.com:6060")},
-			errorMsg: `fcors: insecure origin pattern "http://example.com:6060" requires ` +
-				`option risky.TolerateInsecureOrigins when credentialed access is enabled`,
+			errorMsg: `fcors: insecure origin patterns like "http://example.com:6060" ` +
+				`are by default prohibited when credentialed access is enabled`,
 		}, {
 			desc: "option PrivateNetworkAccess is used and specified origin is insecure",
 			options: []fcors.Option{
 				fcors.FromOrigins("http://example.com:6060"),
 				risky.PrivateNetworkAccess(),
 			},
-			errorMsg: `fcors: insecure origin pattern "http://example.com:6060" requires option ` +
-				`risky.TolerateInsecureOrigins when credentialed access is enabled and/or ` +
+			errorMsg: `fcors: insecure origin patterns like "http://example.com:6060" ` +
+				`are by default prohibited when credentialed access is enabled and/or ` +
 				`Private-Network Access is enabled`,
 		}, {
 			desc: "option PrivateNetworkAccessInNoCORSModeOnly is used and specified origin is insecure",
 			options: []fcors.Option{
 				fcors.FromOrigins("http://example.com:6060"),
 				risky.PrivateNetworkAccessInNoCORSModeOnly(),
 			},
-			errorMsg: `fcors: insecure origin pattern "http://example.com:6060" requires option ` +
-				`risky.TolerateInsecureOrigins when credentialed access is enabled and/or ` +
+			errorMsg: `fcors: insecure origin patterns like "http://example.com:6060" ` +
+				`are by default prohibited when credentialed access is enabled and/or ` +
 				`Private-Network Access is enabled`,
 		}, {
 			desc:     "specified origin's host is an invalid IP address",
@@ -696,8 +696,8 @@ func TestInvalidPoliciesForAllowAccessWithCredentials(t *testing.T) {
 		}, {
 			desc:    "specified base origin is insecure",
 			options: []fcors.Option{fcors.FromOrigins("http://*.example.com:6060")},
-			errorMsg: `fcors: insecure origin pattern "http://*.example.com:6060" requires option ` +
-				`risky.TolerateInsecureOrigins when credentialed access is enabled`,
+			errorMsg: `fcors: insecure origin patterns like "http://*.example.com:6060" ` +
+				`are by default prohibited when credentialed access is enabled`,
 		}, {
 			desc: "option PrivateNetworkAccess is used and some origin patterns are insecure",
 			options: []fcors.Option{
@@ -707,8 +707,8 @@ func TestInvalidPoliciesForAllowAccessWithCredentials(t *testing.T) {
 				),
 				risky.PrivateNetworkAccess(),
 			},
-			errorMsg: `fcors: insecure origin patterns "http://example.com:6060", "http://*.example.com:6060" ` +
-				`require option risky.TolerateInsecureOrigins when credentialed access is enabled and/or ` +
+			errorMsg: `fcors: insecure origin patterns like "http://example.com:6060", "http://*.example.com:6060" ` +
+				`are by default prohibited when credentialed access is enabled and/or ` +
 				`Private-Network Access is enabled`,
 		}, {
 			desc: "option PrivateNetworkAccessInNoCORSModeOnly is used and some origin patterns are insecure",
@@ -719,8 +719,8 @@ func TestInvalidPoliciesForAllowAccessWithCredentials(t *testing.T) {
 				),
 				risky.PrivateNetworkAccessInNoCORSModeOnly(),
 			},
-			errorMsg: `fcors: insecure origin patterns "http://example.com:6060", "http://*.example.com:6060" ` +
-				`require option risky.TolerateInsecureOrigins when credentialed access is enabled and/or ` +
+			errorMsg: `fcors: insecure origin patterns like "http://example.com:6060", "http://*.example.com:6060" ` +
+				`are by default prohibited when credentialed access is enabled and/or ` +
 				`Private-Network Access is enabled`,
 		}, {
 			desc:     "specified base origin's host is an invalid IP address",
@@ -782,8 +782,8 @@ func TestInvalidPoliciesForAllowAccessWithCredentials(t *testing.T) {
 		}, {
 			desc:    "specified base origin's host is a public suffix",
 			options: []fcors.Option{fcors.FromOrigins("https://*.github.io")},
-			errorMsg: `fcors: origin patterns like "https://*.github.io" that allow arbitrary ` +
-				`subdomains of public suffix "github.io" are by default prohibited`,
+			errorMsg: `fcors: origin patterns like "https://*.github.io" that encompass ` +
+				`subdomains of a public suffix are by default prohibited`,
 		}, {
 			desc:     "missing call to FromOrigins",
 			options:  []fcors.Option{fcors.WithAnyMethod()},
@@ -1107,8 +1107,8 @@ func TestInvalidPoliciesForAllowAccessWithCredentials(t *testing.T) {
 					`fcors: option WithRequestHeaders used multiple times`,
 					`fcors: specified max-age value 86401 exceeds upper bound 86400`,
 					`fcors: option MaxAgeInSeconds used multiple times`,
-					`fcors: insecure origin pattern "http://example.com" requires option ` +
-						`risky.TolerateInsecureOrigins when credentialed access is enabled`,
+					`fcors: insecure origin patterns like "http://example.com" ` +
+						`are by default prohibited when credentialed access is enabled`,
 				}, "\n"),
 		},
 	}

diff --git a/internal/middleware.go b/internal/middleware.go
@@ -57,7 +57,7 @@ func init() {
 }
 
 type TempConfig struct {
-	PublicSuffixError               error
+	PublicSuffixes                  []string
 	InsecureOriginPatterns          []string
 	SingleNonWildcardOrigin         string
 	AllowedMethods                  util.Set[string]
@@ -118,23 +118,9 @@ func (cfg *Config) validate() error {
 		// which itself doesn't require risky.TolerateInsecureOrigins.
 		var errorMsg strings.Builder
 		var patterns = cfg.tmp.InsecureOriginPatterns
-		if len(cfg.tmp.InsecureOriginPatterns) == 1 {
-			errorMsg.WriteString("insecure origin pattern ")
-			errorMsg.WriteByte('"')
-			errorMsg.WriteString(patterns[0])
-			errorMsg.WriteString(`" requires `)
-		} else {
-			errorMsg.WriteString("insecure origin patterns ")
-			for i := 0; i < len(patterns)-1; i++ {
-				errorMsg.WriteByte('"')
-				errorMsg.WriteString(patterns[i])
-				errorMsg.WriteString(`", `)
-			}
-			errorMsg.WriteByte('"')
-			errorMsg.WriteString(patterns[len(patterns)-1])
-			errorMsg.WriteString(`" require `)
-		}
-		errorMsg.WriteString("option risky." + optTIO + " when ")
+		errorMsg.WriteString(`insecure origin patterns like "`)
+		errorMsg.WriteString(strings.Join(patterns, `", "`))
+		errorMsg.WriteString(`" are by default prohibited when `)
 		if cfg.AllowCredentials {
 			errorMsg.WriteString("credentialed access is enabled")
 		}
@@ -147,9 +133,14 @@ func (cfg *Config) validate() error {
 		err := util.NewError(errorMsg.String())
 		errs = append(errs, err)
 	}
-	if cfg.tmp.PublicSuffixError != nil &&
-		!cfg.tmp.SkipPublicSuffixCheck {
-		errs = append(errs, cfg.tmp.PublicSuffixError)
+	if len(cfg.tmp.PublicSuffixes) > 0 && !cfg.tmp.SkipPublicSuffixCheck {
+		var errorMsg strings.Builder
+		errorMsg.WriteString(`origin patterns like "`)
+		errorMsg.WriteString(strings.Join(cfg.tmp.PublicSuffixes, `", "`))
+		errorMsg.WriteString(`" that encompass subdomains of a public suffix`)
+		errorMsg.WriteString(" are by default prohibited")
+		err := util.NewError(errorMsg.String())
+		errs = append(errs, err)
 	}
 	if cfg.tmp.FromOriginsCalled && cfg.AllowAnyOrigin {
 		const msg = "incompatible options " + optFO + " and " + optFAO

diff --git a/internal/options.go b/internal/options.go
@@ -89,7 +89,7 @@ func NewMiddleware[A applier](cred bool, one A, others ...A) (Middleware, error)
 func FromOrigins(one string, others ...string) Option {
 	var (
 		setOfPatterns          = make(util.Set[origin.Pattern])
-		publicSuffixError      error
+		publicSuffixes         []string
 		insecureOriginPatterns []string
 		nonWildcardOrigin      string
 	)
@@ -105,11 +105,8 @@ func FromOrigins(one string, others ...string) Option {
 			nonWildcardOrigin = raw
 		}
 		if pattern.Kind == origin.PatternKindSubdomains {
-			eTLD, isEffectiveTLD := pattern.HostIsEffectiveTLD()
-			if isEffectiveTLD && publicSuffixError == nil {
-				const tmpl = "origin patterns like %q that allow arbitrary " +
-					"subdomains of public suffix %q are by default prohibited"
-				publicSuffixError = util.Errorf(tmpl, raw, eTLD)
+			if _, isEffectiveTLD := pattern.HostIsEffectiveTLD(); isEffectiveTLD {
+				publicSuffixes = append(publicSuffixes, raw)
 			}
 		}
 		setOfPatterns.Add(*pattern)
@@ -131,7 +128,7 @@ func FromOrigins(one string, others ...string) Option {
 		}
 		cfg.tmp.FromOriginsCalled = true
 		cfg.tmp.InsecureOriginPatterns = insecureOriginPatterns
-		cfg.tmp.PublicSuffixError = publicSuffixError
+		cfg.tmp.PublicSuffixes = publicSuffixes
 		if len(errs) != 0 {
 			return errors.Join(errs...)
 		}