Skip to content

Commit

Permalink
Fix extension for large uint32 values on 32 bit machines
Browse files Browse the repository at this point in the history
This also updates the extension to return an integer for uint64 values that are
less than LONG_MAX.

Closes #79.
  • Loading branch information
oschwald committed Jan 3, 2019
1 parent 2f1086e commit 04d2fab
Show file tree
Hide file tree
Showing 4 changed files with 76 additions and 7 deletions.
12 changes: 12 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,18 @@
CHANGELOG
=========

1.4.1
------------------

* The `maxminddb` extension now returns a string when a `uint32`
value is greater than `LONG_MAX`. Previously, the value would
overflow. This generally only affects 32-bit machines. Reported
by Remi Collet. GitHub #79.
* For `uint64` values, the `maxminddb` extension now returns an
integer rather than a string when the value is less than or equal
to `LONG_MAX`. This more closely matches the behavior of the pure
PHP reader.

1.4.0 (2018-11-20)
------------------

Expand Down
44 changes: 40 additions & 4 deletions ext/maxminddb.c
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,8 @@ static void handle_uint128(const MMDB_entry_data_list_s *entry_data_list,
zval *z_value TSRMLS_DC);
static void handle_uint64(const MMDB_entry_data_list_s *entry_data_list,
zval *z_value TSRMLS_DC);
static void handle_uint32(const MMDB_entry_data_list_s *entry_data_list,
zval *z_value TSRMLS_DC);
static zend_class_entry * lookup_class(const char *name TSRMLS_DC);

#define CHECK_ALLOCATED(val) \
Expand Down Expand Up @@ -338,7 +340,7 @@ static const MMDB_entry_data_list_s *handle_entry_data_list(
ZVAL_LONG(z_value, entry_data_list->entry_data.uint16);
break;
case MMDB_DATA_TYPE_UINT32:
ZVAL_LONG(z_value, entry_data_list->entry_data.uint32);
handle_uint32(entry_data_list, z_value TSRMLS_CC);
break;
case MMDB_DATA_TYPE_BOOLEAN:
ZVAL_BOOL(z_value, entry_data_list->entry_data.boolean);
Expand Down Expand Up @@ -451,17 +453,51 @@ static void handle_uint128(const MMDB_entry_data_list_s *entry_data_list,
efree(num_str);
}

static void handle_uint32(const MMDB_entry_data_list_s *entry_data_list,
zval *z_value TSRMLS_DC)
{
uint32_t val = entry_data_list->entry_data.uint32;

#if LONG_MAX >= UINT32_MAX
ZVAL_LONG(z_value, val);
return;
#else
if (val <= LONG_MAX) {
ZVAL_LONG(z_value, val);
return;
}

char *int_str;
spprintf(&int_str, 0, "%" PRIu32, val);
CHECK_ALLOCATED(int_str);

_ZVAL_STRING(z_value, int_str);
efree(int_str);
#endif
}


static void handle_uint64(const MMDB_entry_data_list_s *entry_data_list,
zval *z_value TSRMLS_DC)
{
// We return it as a string because PHP uses signed longs
uint64_t val = entry_data_list->entry_data.uint64;

#if LONG_MAX >= UINT64_MAX
ZVAL_LONG(z_value, val);
return;
#else
if (val <= LONG_MAX) {
ZVAL_LONG(z_value, val);
return;
}

char *int_str;
spprintf(&int_str, 0, "%" PRIu64,
entry_data_list->entry_data.uint64);
spprintf(&int_str, 0, "%" PRIu64, val);
CHECK_ALLOCATED(int_str);

_ZVAL_STRING(z_value, int_str);
efree(int_str);
#endif
}

static zend_class_entry *lookup_class(const char *name TSRMLS_DC)
Expand Down
25 changes: 23 additions & 2 deletions tests/MaxMind/Db/Test/ReaderTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,8 @@ public function testDecoder()

$this->assertSame(-268435456, $record['int32']);
$this->assertSame(100, $record['uint16']);
$this->assertSame(PHP_INT_MAX < 4294967295 ? '268435456' : 268435456, $record['uint32']);
$this->assertSame('1152921504606846976', $record['uint64']);
$this->assertSame(PHP_INT_MAX < 4294967295 && !\extension_loaded('maxminddb') ? '268435456' : 268435456, $record['uint32']);
$this->assertSame(PHP_INT_MAX > 1152921504606846976 && \extension_loaded('maxminddb') ? 1152921504606846976 : '1152921504606846976', $record['uint64']);

$uint128 = $record['uint128'];

Expand Down Expand Up @@ -100,6 +100,27 @@ public function testZeros()
$this->assertSame('0', $uint128);
}

public function testMax()
{
$reader = new Reader('tests/data/test-data/MaxMind-DB-test-decoder.mmdb');
$record = $reader->get('::255.255.255.255');

$this->assertSame(INF, $record['double']);
$this->assertSame(INF, $record['float'], 'float');
$this->assertSame(2147483647, $record['int32']);
$this->assertSame(0xFFFF, $record['uint16']);
$this->assertSame(PHP_INT_MAX < 0xFFFFFFFF ? '4294967295' : 0xFFFFFFFF, $record['uint32']);
$this->assertSame('18446744073709551615', $record['uint64'] . '');

$uint128 = $record['uint128'];
if (\extension_loaded('gmp')) {
$uint128 = gmp_strval($uint128);
} else {
$this->markTestIncomplete('Requires gmp extension to check value of uint128');
}
$this->assertSame('340282366920938463463374607431768211455', $uint128);
}

public function testNoIpV4SearchTree()
{
$reader = new Reader(
Expand Down
2 changes: 1 addition & 1 deletion tests/data
Submodule data updated 59 files
+1 −0 .gitattributes
+2 −0 .gitconfig
+1 −0 .gitignore
+40 −29 MaxMind-DB-spec.md
+ MaxMind-DB-test-metadata-pointers.mmdb
+7 −0 bad-data/README.md
+ bad-data/libmaxminddb/libmaxminddb-offset-integer-overflow.mmdb
+ bad-data/maxminddb-golang/cyclic-data-structure.mmdb
+1 −0 bad-data/maxminddb-golang/invalid-bytes-length.mmdb
+ bad-data/maxminddb-golang/invalid-data-record-offset.mmdb
+ bad-data/maxminddb-golang/invalid-map-key-length.mmdb
+1 −0 bad-data/maxminddb-golang/invalid-string-length.mmdb
+1 −0 bad-data/maxminddb-golang/metadata-is-an-uint128.mmdb
+ bad-data/maxminddb-golang/unexpected-bytes.mmdb
+12 −0 perltidyrc
+14 −5 source-data/GeoIP2-Anonymous-IP-Test.json
+476 −0 source-data/GeoIP2-City-Test.json
+372 −0 source-data/GeoIP2-Country-Test.json
+14 −0 source-data/GeoIP2-DensityIncome-Test.json
+673 −0 source-data/GeoIP2-Enterprise-Test.json
+1 −1 source-data/GeoIP2-ISP-Test.json
+0 −12,454 source-data/GeoIP2-Precision-City-Test.json
+1,598 −0 source-data/GeoIP2-Precision-Enterprise-Test.json
+2,824 −0 source-data/GeoIP2-User-Count-Test.json
+37 −0 source-data/GeoLite2-ASN-Test.json
+10 −3 source-data/README
+ test-data/GeoIP2-Anonymous-IP-Test.mmdb
+ test-data/GeoIP2-City-Test-Broken-Double-Format.mmdb
+ test-data/GeoIP2-City-Test-Invalid-Node-Count.mmdb
+ test-data/GeoIP2-City-Test.mmdb
+ test-data/GeoIP2-Connection-Type-Test.mmdb
+ test-data/GeoIP2-Country-Test.mmdb
+ test-data/GeoIP2-DensityIncome-Test.mmdb
+ test-data/GeoIP2-Domain-Test.mmdb
+ test-data/GeoIP2-Enterprise-Test.mmdb
+ test-data/GeoIP2-ISP-Test.mmdb
+ test-data/GeoIP2-Precision-City-Test.mmdb
+ test-data/GeoIP2-Precision-Enterprise-Test.mmdb
+ test-data/GeoIP2-User-Count-Test.mmdb
+ test-data/GeoLite2-ASN-Test.mmdb
+ test-data/MaxMind-DB-no-ipv4-search-tree.mmdb
+ test-data/MaxMind-DB-string-value-entries.mmdb
+ test-data/MaxMind-DB-test-broken-pointers-24.mmdb
+ test-data/MaxMind-DB-test-broken-search-tree-24.mmdb
+ test-data/MaxMind-DB-test-decoder.mmdb
+ test-data/MaxMind-DB-test-ipv4-24.mmdb
+ test-data/MaxMind-DB-test-ipv4-28.mmdb
+ test-data/MaxMind-DB-test-ipv4-32.mmdb
+ test-data/MaxMind-DB-test-ipv6-24.mmdb
+ test-data/MaxMind-DB-test-ipv6-28.mmdb
+ test-data/MaxMind-DB-test-ipv6-32.mmdb
+ test-data/MaxMind-DB-test-metadata-pointers.mmdb
+ test-data/MaxMind-DB-test-mixed-24.mmdb
+ test-data/MaxMind-DB-test-mixed-28.mmdb
+ test-data/MaxMind-DB-test-mixed-32.mmdb
+ test-data/MaxMind-DB-test-nested.mmdb
+17 −0 test-data/README.md
+202 −68 test-data/write-test-data.pl
+5 −0 tidyall.ini

0 comments on commit 04d2fab

Please sign in to comment.