Skip to content

Commit

Permalink
Add option to toggle file paths in error messages
Browse files Browse the repository at this point in the history
  • Loading branch information
mm-david committed Aug 13, 2024
1 parent 2cababd commit 309cdb0
Show file tree
Hide file tree
Showing 4 changed files with 43 additions and 14 deletions.
7 changes: 7 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,12 @@
## CHANGELOG

## 2.5.0 (2024-08-14)

* Add option to verifier.ProcessGeofeed reduce verbosity of error messages,
toggling whether file paths are included.
* Update interface of verifier package, adding a new struct to hold any
verification options.

## 2.4.0 (2023-07-13)

* Update files to comply with major release version 2
Expand Down
2 changes: 1 addition & 1 deletion main.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ func run() error {
return err
}

c, diffLines, asnCounts, err := verify.ProcessGeofeed(conf.gf, conf.db, conf.isp, conf.laxMode)
c, diffLines, asnCounts, err := verify.ProcessGeofeed(conf.gf, conf.db, conf.isp, verify.Options{LaxMode: conf.laxMode})
if err != nil {
if errors.Is(err, verify.ErrInvalidGeofeed) {
log.Printf("Found %d invalid rows out of %d rows in total, examples by type:", c.Invalid, c.Total)
Expand Down
44 changes: 33 additions & 11 deletions verify/verify.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,14 +37,23 @@ func NewCheckResult() CheckResult {
}
}

type Options struct {
// // LaxMode controls validation for region codes. If LaxMode is false
// (default), ISO-3166-2 region codes format is required. Otherwise region
// code is accepted both with or without country code.
LaxMode bool
// HideFilePathsInErrorMessages, if set to true, will prevent file paths
// from appearing in error messages. This reduces information leakage in
// contexts where the error messages might be shared.
HideFilePathsInErrorMessages bool
}

// ProcessGeofeed attempts to validate a given geofeedFilename.
// If laxMode is false (default), ISO-3166-2 region codes format is required.
// Otherwise region code is accepted both with or without country code.
func ProcessGeofeed(
geofeedFilename,
mmdbFilename,
ispFilename string,
laxMode bool,
opts Options,
) (CheckResult, []string, map[uint]int, error) { //nolint:unparam // false positive on map[uint]int
c := NewCheckResult()
var diffLines []string
Expand All @@ -53,6 +62,9 @@ func ProcessGeofeed(
// See https://github.com/golang/go/issues/33887.
geofeedFH, err := utfutil.OpenFile(filepath.Clean(geofeedFilename), utfutil.UTF8)
if err != nil {
if opts.HideFilePathsInErrorMessages {
return c, diffLines, nil, fmt.Errorf("unable to open file: %w", err)
}
return c, diffLines, nil, fmt.Errorf("unable to open %s: %w", geofeedFilename, err)
}
defer func() {
Expand All @@ -63,6 +75,9 @@ func ProcessGeofeed(

db, err := geoip2.Open(filepath.Clean(mmdbFilename))
if err != nil {
if opts.HideFilePathsInErrorMessages {
return c, diffLines, nil, fmt.Errorf("unable to open MMDB: %w", err)
}
return c, diffLines, nil, fmt.Errorf("unable to open MMDB %s: %w", mmdbFilename, err)
}
defer db.Close()
Expand All @@ -71,7 +86,10 @@ func ProcessGeofeed(
if ispFilename != "" {
ispdb, err = geoip2.Open(filepath.Clean(ispFilename))
if err != nil {
return c, diffLines, nil, fmt.Errorf("unable to open MMDB %s: %w", ispFilename, err)
if opts.HideFilePathsInErrorMessages {
return c, diffLines, nil, fmt.Errorf("unable to open ISP MMDB: %w", err)
}
return c, diffLines, nil, fmt.Errorf("unable to open ISP MMDB %s: %w", ispFilename, err)
}
defer ispdb.Close()
}
Expand All @@ -91,8 +109,10 @@ func ProcessGeofeed(
break
}
if err != nil {
return c, diffLines, asnCounts,
fmt.Errorf("unable to read next row in %s: %w", geofeedFilename, err)
if opts.HideFilePathsInErrorMessages {
return c, diffLines, asnCounts, fmt.Errorf("unable to read next row: %w", err)
}
return c, diffLines, asnCounts, fmt.Errorf("unable to read next row in %s: %w", geofeedFilename, err)
}

c.Total++
Expand All @@ -111,7 +131,7 @@ func ProcessGeofeed(
continue
}

diffLine, result := verifyCorrection(row[:expectedFieldsPerRecord], db, ispdb, asnCounts, laxMode)
diffLine, result := verifyCorrection(row[:expectedFieldsPerRecord], db, ispdb, asnCounts, opts.LaxMode)

Check failure on line 134 in verify/verify.go

View workflow job for this annotation

GitHub Actions / Build 1.21.x test on ubuntu-latest

cannot use opts.LaxMode (variable of type bool) as Options value in argument to verifyCorrection

Check failure on line 134 in verify/verify.go

View workflow job for this annotation

GitHub Actions / Build 1.21.x test on macos-latest

cannot use opts.LaxMode (variable of type bool) as Options value in argument to verifyCorrection

Check failure on line 134 in verify/verify.go

View workflow job for this annotation

GitHub Actions / Build 1.22.x test on ubuntu-latest

cannot use opts.LaxMode (variable of type bool) as Options value in argument to verifyCorrection

Check failure on line 134 in verify/verify.go

View workflow job for this annotation

GitHub Actions / Build 1.22.x test on macos-latest

cannot use opts.LaxMode (variable of type bool) as Options value in argument to verifyCorrection
if !result.valid {
if _, ok := c.SampleInvalidRows[result.invalidityType]; !ok {
c.SampleInvalidRows[result.invalidityType] = fmt.Sprintf(
Expand All @@ -130,8 +150,10 @@ func ProcessGeofeed(
}
}
if err != nil && !errors.Is(err, io.EOF) {
return c, diffLines, asnCounts,
fmt.Errorf("error while reading %s: %w", geofeedFilename, err)
if opts.HideFilePathsInErrorMessages {
return c, diffLines, asnCounts, fmt.Errorf("error reading file: %w", err)
}
return c, diffLines, asnCounts, fmt.Errorf("error while reading %s: %w", geofeedFilename, err)
}

if c.Invalid > 0 || len(c.SampleInvalidRows) > 0 {
Expand All @@ -151,7 +173,7 @@ func verifyCorrection(
correction []string,
db, ispdb *geoip2.Reader,
asnCounts map[uint]int,
laxMode bool,
opts Options,
) (string, verificationResult) {
/*
0: network (CIDR or single IP)
Expand Down Expand Up @@ -207,7 +229,7 @@ func verifyCorrection(
// In "--lax" mode both region code formats (with or without country code) are accepted.
if strings.Contains(correction[2], "-") {
mostSpecificSubdivision = mmdbRecord.Country.IsoCode + "-" + mostSpecificSubdivision
} else if correction[2] != "" && !laxMode {
} else if correction[2] != "" && !opts.LaxMode {
return "", verificationResult{
valid: false,
invalidityType: InvalidRegionCode,
Expand Down
4 changes: 2 additions & 2 deletions verify/verify_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ func TestProcessGeofeed_Valid(t *testing.T) {
for _, test := range goodTests {
t.Run(
test.gf+" "+test.db, func(t *testing.T) {
c, dl, _, err := ProcessGeofeed(test.gf, test.db, "", test.laxMode)
c, dl, _, err := ProcessGeofeed(test.gf, test.db, "", Options{LaxMode: test.laxMode})
require.NoError(t, err, "processGeofeed ran without error")
for i, s := range test.dl {
assert.Contains(
Expand Down Expand Up @@ -167,7 +167,7 @@ func TestProcessGeofeed_Invalid(t *testing.T) {
for _, test := range badTests {
t.Run(
test.gf+" "+test.db, func(t *testing.T) {
c, _, _, err := ProcessGeofeed(test.gf, test.db, "", test.laxMode)
c, _, _, err := ProcessGeofeed(test.gf, test.db, "", Options{LaxMode: test.laxMode})
require.ErrorIs(
t,
err,
Expand Down

0 comments on commit 309cdb0

Please sign in to comment.