diff --git a/ChangeLog.md b/ChangeLog.md index 965a31a..dd2e2f9 100644 --- a/ChangeLog.md +++ b/ChangeLog.md @@ -1,5 +1,12 @@ # Change Log # +## 1.17 (2016-05-16) + +* Previously data records at the end of the database were incorrectly returned + as `null` values when using shared memory. This was due to attempting to + read beyond the end of the database. This bug only affected users using + `GEOIP_SHARED_MEMORY`. + ## 1.16 (2016-01-29) * Fixed issue that could cause a notice about using a property on a non-object diff --git a/src/geoip.inc b/src/geoip.inc index fe4566a..885a9e0 100644 --- a/src/geoip.inc +++ b/src/geoip.inc @@ -1397,12 +1397,12 @@ function _setup_segments($gi) $gi->databaseType = GEOIP_COUNTRY_EDITION; $gi->record_length = STANDARD_RECORD_LENGTH; if ($gi->flags & GEOIP_SHARED_MEMORY) { - $offset = @shmop_size($gi->shmid) - 3; + $offset = shmop_size($gi->shmid) - 3; for ($i = 0; $i < STRUCTURE_INFO_MAX_SIZE; $i++) { - $delim = @shmop_read($gi->shmid, $offset, 3); + $delim = shmop_read($gi->shmid, $offset, 3); $offset += 3; if ($delim == (chr(255) . chr(255) . chr(255))) { - $gi->databaseType = ord(@shmop_read($gi->shmid, $offset, 1)); + $gi->databaseType = ord(shmop_read($gi->shmid, $offset, 1)); if ($gi->databaseType >= 106) { $gi->databaseType -= 105; } @@ -1432,7 +1432,7 @@ function _setup_segments($gi) || ($gi->databaseType == GEOIP_ASNUM_EDITION_V6) ) { $gi->databaseSegments = 0; - $buf = @shmop_read($gi->shmid, $offset, SEGMENT_RECORD_LENGTH); + $buf = shmop_read($gi->shmid, $offset, SEGMENT_RECORD_LENGTH); for ($j = 0; $j < SEGMENT_RECORD_LENGTH; $j++) { $gi->databaseSegments += (ord($buf[$j]) << ($j * 8)); } @@ -1494,6 +1494,7 @@ function _setup_segments($gi) || ($gi->databaseType == GEOIP_ASNUM_EDITION_V6) ) { $gi->databaseSegments = 0; + $buf = fread($gi->filehandle, SEGMENT_RECORD_LENGTH); for ($j = 0; $j < SEGMENT_RECORD_LENGTH; $j++) { $gi->databaseSegments += (ord($buf[$j]) << ($j * 8)); @@ -1525,12 +1526,20 @@ function _setup_segments($gi) return $gi; } +# This should be only used for variable-length records where +# $start + $maxLength may be greater than the shared mem size +function _sharedMemRead($gi, $start, $maxLength) +{ + $readLength = min(shmop_size($gi->shmid) - $start, $maxLength); + return shmop_read($gi->shmid, $start, $readLength); +} + function geoip_open($filename, $flags) { $gi = new GeoIP; $gi->flags = $flags; if ($gi->flags & GEOIP_SHARED_MEMORY) { - $gi->shmid = @shmop_open(GEOIP_SHM_KEY, "a", 0, 0); + $gi->shmid = shmop_open(GEOIP_SHM_KEY, "a", 0, 0); } else { $gi->filehandle = fopen($filename, "rb") or trigger_error("GeoIP API: Can not open $filename\n", E_USER_ERROR); if ($gi->flags & GEOIP_MEMORY_CACHE) { @@ -1686,8 +1695,7 @@ function _geoip_seek_country_v6($gi, $ipnum) 2 * $gi->record_length ); } elseif ($gi->flags & GEOIP_SHARED_MEMORY) { - $buf = @shmop_read( - $gi->shmid, + $buf = _sharedMemRead($gi, 2 * $gi->record_length * $offset, 2 * $gi->record_length ); @@ -1733,8 +1741,8 @@ function _geoip_seek_country($gi, $ipnum) 2 * $gi->record_length ); } elseif ($gi->flags & GEOIP_SHARED_MEMORY) { - $buf = @shmop_read( - $gi->shmid, + $buf = _sharedMemRead( + $gi, 2 * $gi->record_length * $offset, 2 * $gi->record_length ); @@ -1769,7 +1777,7 @@ function _common_get_org($gi, $seek_org) { $record_pointer = $seek_org + (2 * $gi->record_length - 1) * $gi->databaseSegments; if ($gi->flags & GEOIP_SHARED_MEMORY) { - $org_buf = @shmop_read($gi->shmid, $record_pointer, MAX_ORG_RECORD_LENGTH); + $org_buf = _sharedMemRead($gi, $record_pointer, MAX_ORG_RECORD_LENGTH); } else { fseek($gi->filehandle, $record_pointer, SEEK_SET); $org_buf = fread($gi->filehandle, MAX_ORG_RECORD_LENGTH); diff --git a/src/geoipcity.inc b/src/geoipcity.inc index 88ced72..0a99681 100644 --- a/src/geoipcity.inc +++ b/src/geoipcity.inc @@ -65,7 +65,7 @@ function _common_get_record($gi, $seek_country) if ($gi->flags & GEOIP_MEMORY_CACHE) { $record_buf = substr($gi->memory_buffer, $record_pointer, FULL_RECORD_LENGTH); } elseif ($gi->flags & GEOIP_SHARED_MEMORY) { - $record_buf = @shmop_read($gi->shmid, $record_pointer, FULL_RECORD_LENGTH); + $record_buf = _sharedMemRead($gi, $record_pointer, FULL_RECORD_LENGTH); } else { fseek($gi->filehandle, $record_pointer, SEEK_SET); $record_buf = fread($gi->filehandle, FULL_RECORD_LENGTH); diff --git a/tests/CityTest.php b/tests/CityTest.php index 95b7ef6..dcce110 100644 --- a/tests/CityTest.php +++ b/tests/CityTest.php @@ -36,4 +36,18 @@ public function testCity() geoip_country_code_by_addr($gi, "64.17.254.216") ); } + + public function testCityWithSharedMemory() + { + // HHVM doesn't support shared memory + if (defined('HHVM_VERSION')) { + $this->markTestSkipped(); + } + geoip_load_shared_mem("tests/data/GeoIPCity.dat"); + + $gi = geoip_open("tests/data/GeoIPCity.dat", GEOIP_SHARED_MEMORY); + $record = geoip_record_by_addr($gi, "222.230.136.0"); + + $this->assertEquals('Setagaya', $record->city); + } } diff --git a/tests/NetspeedcellTest.php b/tests/NetspeedcellTest.php index 43fb63e..7b276a1 100644 --- a/tests/NetspeedcellTest.php +++ b/tests/NetspeedcellTest.php @@ -11,4 +11,20 @@ public function testNetspeedcell() geoip_org_by_addr($gi, "2.125.160.1") ); } + + public function testNetspeedcellWithSharedMemory() + { + // HHVM doesn't support shared memory + if (defined('HHVM_VERSION')) { + $this->markTestSkipped(); + } + geoip_load_shared_mem("tests/data/GeoIPNetSpeedCell.dat"); + + $gi = geoip_open("tests/data/GeoIPNetSpeedCell.dat", GEOIP_SHARED_MEMORY); + + $this->assertEquals( + 'Dialup', + geoip_name_by_addr($gi, "2.125.160.1") + ); + } }