Merge pull request #78 from maxmind/mm-david/empty-ok

Require geofeeds to have content

Author: ikolesn

CHANGELOG.md

@@ -1,5 +1,10 @@
 ## CHANGELOG
 
+## 3.1.0 (2024-11-05)
+
+* Empty geofeeds will now be considered invalid, unless the (new) EmptyOK option
+  is passed to ProcessGeofeed (e.g. via the new `empty-ok` flag).
+
 ## 3.0.0 (2024-08-14)
 
 * Update interface of ProcessGeofeed in the verifier package, adding a new

main.go

@@ -30,6 +30,7 @@ type config struct {
 	db      string
 	isp     string
 	laxMode bool
+	emptyOK bool
 }
 
 func main() {
@@ -50,7 +51,7 @@ func run() error {
 		conf.gf,
 		conf.db,
 		conf.isp,
-		verify.Options{LaxMode: conf.laxMode},
+		verify.Options{LaxMode: conf.laxMode, EmptyOK: conf.emptyOK},
 	)
 	if err != nil {
 		if errors.Is(err, verify.ErrInvalidGeofeed) {
@@ -108,6 +109,11 @@ func parseFlags(program string, args []string) (c *config, output string, err er
 		"lax",
 		false,
 		"Enable lax mode: geofeed's region code may be provided without country code prefix")
+	flags.BoolVar(
+		&conf.emptyOK,
+		"empty-ok",
+		false,
+		"Allow empty geofeeds to be considered valid")
 
 	err = flags.Parse(args)
 	if err != nil {

verify/errors.go

@@ -2,9 +2,14 @@ package verify
 
 import "errors"
 
-// ErrInvalidGeofeed represents error that is returned in case of incomplete compliance
-// with RFC 8805 standards and the mode in which the program is run.
-var ErrInvalidGeofeed = errors.New("geofeed does not comply with the RFC 8805 standards")
+var (
+	// ErrInvalidGeofeed represents error that is returned in case of incomplete
+	// compliance with RFC 8805 standards and the mode in which the program is
+	// run.
+	ErrInvalidGeofeed = errors.New("geofeed does not comply with the RFC 8805 standards")
+	// ErrEmptyGeofeed indicates a Geofeed with no records.
+	ErrEmptyGeofeed = errors.New("geofeed is empty")
+)
 
 // RowInvalidity represents type of row invalidity.
 type RowInvalidity int

verify/test_data/empty.csv

@@ -47,6 +47,9 @@ type Options struct {
 	// from appearing in error messages. This reduces information leakage in
 	// contexts where the error messages might be shared.
 	HideFilePathsInErrorMessages bool
+	// EmptyOK, if set to true, will consider a geofeed with no records to be
+	// valid. The default behavior (false) requires a geofeed to not be empty.
+	EmptyOK bool
 }
 
 // ProcessGeofeed attempts to validate a given geofeedFilename.
@@ -157,6 +160,10 @@ func ProcessGeofeed(
 		return c, diffLines, asnCounts, fmt.Errorf("error while reading %s: %w", geofeedFilename, err)
 	}
 
+	if c.Total == 0 && !opts.EmptyOK {
+		return c, diffLines, asnCounts, ErrEmptyGeofeed
+	}
+
 	if c.Invalid > 0 || len(c.SampleInvalidRows) > 0 {
 		return c, diffLines, asnCounts, ErrInvalidGeofeed
 	}

verify/verify.go

@@ -14,6 +14,7 @@ type processGeofeedTest struct {
 	c       CheckResult
 	em      error
 	laxMode bool
+	emptyOK bool
 }
 
 func TestProcessGeofeed_Valid(t *testing.T) {
@@ -74,14 +75,31 @@ func TestProcessGeofeed_Valid(t *testing.T) {
 			},
 			laxMode: false,
 		},
+		{
+			gf: "test_data/empty.csv",
+			db: "test_data/GeoIP2-City-Test.mmdb",
+			c: CheckResult{
+				Total:             0,
+				SampleInvalidRows: map[RowInvalidity]string{},
+			},
+			emptyOK: true,
+		},
 	}
 
 	// Testing the full content of the difference explanation strings is likely to be
 	// tedious and brittle, so we will just check for some substrings.
 	for _, test := range goodTests {
 		t.Run(
 			test.gf+" "+test.db, func(t *testing.T) {
-				c, dl, _, err := ProcessGeofeed(test.gf, test.db, "", Options{LaxMode: test.laxMode})
+				c, dl, _, err := ProcessGeofeed(
+					test.gf,
+					test.db,
+					"",
+					Options{
+						EmptyOK: test.emptyOK,
+						LaxMode: test.laxMode,
+					},
+				)
 				require.NoError(t, err, "processGeofeed ran without error")
 				for i, s := range test.dl {
 					assert.Contains(
@@ -102,7 +120,6 @@ func TestProcessGeofeed_Invalid(t *testing.T) {
 		{
 			gf: "test_data/geofeed-invalid-missing-fields.csv",
 			db: "test_data/GeoIP2-City-Test.mmdb",
-			dl: []string{},
 			c: CheckResult{
 				Total:       2,
 				Differences: 0,
@@ -118,7 +135,6 @@ func TestProcessGeofeed_Invalid(t *testing.T) {
 		{
 			gf: "test_data/geofeed-invalid-empty-network.csv",
 			db: "test_data/GeoIP2-City-Test.mmdb",
-			dl: []string{},
 			c: CheckResult{
 				Total:       2,
 				Differences: 1,
@@ -133,7 +149,6 @@ func TestProcessGeofeed_Invalid(t *testing.T) {
 		{
 			gf: "test_data/geofeed-invalid-network.csv",
 			db: "test_data/GeoIP2-City-Test.mmdb",
-			dl: []string{},
 			c: CheckResult{
 				Total:       2,
 				Differences: 1,
@@ -149,7 +164,6 @@ func TestProcessGeofeed_Invalid(t *testing.T) {
 			// Geofeed that is valid in lax mode should not be valid if laxMode == true.
 			gf: "test_data/geofeed-valid-lax.csv",
 			db: "test_data/GeoIP2-City-Test.mmdb",
-			dl: []string{},
 			c: CheckResult{
 				Total:       3,
 				Differences: 1,
@@ -162,12 +176,30 @@ func TestProcessGeofeed_Invalid(t *testing.T) {
 			em:      ErrInvalidGeofeed,
 			laxMode: false,
 		},
+		{
+			gf: "test_data/empty.csv",
+			db: "test_data/GeoIP2-City-Test.mmdb",
+			c: CheckResult{
+				Total:             0,
+				SampleInvalidRows: map[RowInvalidity]string{},
+			},
+			em:      ErrEmptyGeofeed,
+			emptyOK: false,
+		},
 	}
 
 	for _, test := range badTests {
 		t.Run(
 			test.gf+" "+test.db, func(t *testing.T) {
-				c, _, _, err := ProcessGeofeed(test.gf, test.db, "", Options{LaxMode: test.laxMode})
+				c, _, _, err := ProcessGeofeed(
+					test.gf,
+					test.db,
+					"",
+					Options{
+						EmptyOK: test.emptyOK,
+						LaxMode: test.laxMode,
+					},
+				)
 				require.ErrorIs(
 					t,
 					err,