Skip to content

Commit ea55fa9

Browse files
committed
Return an error if a closed reader is used
Previously, using a closed database could result in a segmentation violation when the closed memory map was accessed. See oschwald/geoip2-golang#35.
1 parent 6cf014f commit ea55fa9

File tree

4 files changed

+40
-7
lines changed

4 files changed

+40
-7
lines changed

reader.go

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,9 @@ func (r *Reader) startNode() (uint, error) {
107107
// Lookup takes an IP address as a net.IP structure and a pointer to the
108108
// result value to Decode into.
109109
func (r *Reader) Lookup(ipAddress net.IP, result interface{}) error {
110+
if r.buffer == nil {
111+
return errors.New("cannot call Lookup on a closed database")
112+
}
110113
pointer, err := r.lookupPointer(ipAddress)
111114
if pointer == 0 || err != nil {
112115
return err
@@ -120,6 +123,9 @@ func (r *Reader) Lookup(ipAddress net.IP, result interface{}) error {
120123
// is an advanced API, which exists to provide clients with a means to cache
121124
// previously-decoded records.
122125
func (r *Reader) LookupOffset(ipAddress net.IP) (uintptr, error) {
126+
if r.buffer == nil {
127+
return 0, errors.New("cannot call LookupOffset on a closed database")
128+
}
123129
pointer, err := r.lookupPointer(ipAddress)
124130
if pointer == 0 || err != nil {
125131
return NotFound, err
@@ -144,6 +150,13 @@ func (r *Reader) LookupOffset(ipAddress net.IP) (uintptr, error) {
144150
// single representative record for that country. This uintptr behavior allows
145151
// clients to leverage this normalization in their own sub-record caching.
146152
func (r *Reader) Decode(offset uintptr, result interface{}) error {
153+
if r.buffer == nil {
154+
return errors.New("cannot call Decode on a closed database")
155+
}
156+
return r.decode(offset, result)
157+
}
158+
159+
func (r *Reader) decode(offset uintptr, result interface{}) error {
147160
rv := reflect.ValueOf(result)
148161
if rv.Kind() != reflect.Ptr || rv.IsNil() {
149162
return errors.New("result param must be a pointer")
@@ -233,7 +246,7 @@ func (r *Reader) retrieveData(pointer uint, result interface{}) error {
233246
if err != nil {
234247
return err
235248
}
236-
return r.Decode(offset, result)
249+
return r.decode(offset, result)
237250
}
238251

239252
func (r *Reader) resolveDataPointer(pointer uint) (uintptr, error) {

reader_appengine.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,9 @@ func Open(file string) (*Reader, error) {
2020

2121
// Close unmaps the database file from virtual memory and returns the
2222
// resources to the system. If called on a Reader opened using FromBytes
23-
// or Open on Google App Engine, this method does nothing.
23+
// or Open on Google App Engine, this method sets the underlying buffer
24+
// to nil, returning the resources to the system.
2425
func (r *Reader) Close() error {
26+
r.buffer = nil
2527
return nil
2628
}

reader_other.go

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -52,10 +52,12 @@ func Open(file string) (*Reader, error) {
5252
// resources to the system. If called on a Reader opened using FromBytes
5353
// or Open on Google App Engine, this method does nothing.
5454
func (r *Reader) Close() error {
55-
if !r.hasMappedFile {
56-
return nil
55+
var err error
56+
if r.hasMappedFile {
57+
runtime.SetFinalizer(r, nil)
58+
r.hasMappedFile = false
59+
err = munmap(r.buffer)
5760
}
58-
runtime.SetFinalizer(r, nil)
59-
r.hasMappedFile = false
60-
return munmap(r.buffer)
61+
r.buffer = nil
62+
return err
6163
}

reader_test.go

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -396,6 +396,22 @@ func TestNilLookup(t *testing.T) {
396396
assert.Nil(t, reader.Close(), "error on close")
397397
}
398398

399+
func TestUsingClosedDatabase(t *testing.T) {
400+
reader, _ := Open("test-data/test-data/MaxMind-DB-test-decoder.mmdb")
401+
reader.Close()
402+
403+
var recordInterface interface{}
404+
405+
err := reader.Lookup(nil, recordInterface)
406+
assert.Equal(t, err.Error(), "cannot call Lookup on a closed database")
407+
408+
_, err = reader.LookupOffset(nil)
409+
assert.Equal(t, err.Error(), "cannot call LookupOffset on a closed database")
410+
411+
err = reader.Decode(0, recordInterface)
412+
assert.Equal(t, err.Error(), "cannot call Decode on a closed database")
413+
}
414+
399415
func checkMetadata(t *testing.T, reader *Reader, ipVersion uint, recordSize uint) {
400416
metadata := reader.Metadata
401417

0 commit comments

Comments
 (0)