Skip to content

Commit

Permalink
Require geofeeds to have content
Browse files Browse the repository at this point in the history
  • Loading branch information
mm-david committed Nov 5, 2024
1 parent a2760c6 commit a699957
Show file tree
Hide file tree
Showing 6 changed files with 65 additions and 10 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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
Expand Down
8 changes: 7 additions & 1 deletion main.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ type config struct {
db string
isp string
laxMode bool
emptyOK bool
}

func main() {
Expand All @@ -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) {
Expand Down Expand Up @@ -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 {
Expand Down
11 changes: 8 additions & 3 deletions verify/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Empty file added verify/test_data/empty.csv
Empty file.
7 changes: 7 additions & 0 deletions verify/verify.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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
}
Expand Down
44 changes: 38 additions & 6 deletions verify/verify_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ type processGeofeedTest struct {
c CheckResult
em error
laxMode bool
emptyOK bool
}

func TestProcessGeofeed_Valid(t *testing.T) {
Expand Down Expand Up @@ -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(
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand Down

0 comments on commit a699957

Please sign in to comment.