Skip to content

Commit

Permalink
Fix #750: allow no more than 512 attribute names
Browse files Browse the repository at this point in the history
This fixes two issues:

- use an unsigned type, so we can use the whole 9 bits and have 512
  keys, not 256
- fix the bounds check in AttributeKeyStore to reflex the lower
  threshold that was introduced in #618

Hat tip @oobayly for reporting this.

Fixes #750.
  • Loading branch information
cldellow committed Sep 21, 2024
1 parent 4dc9a15 commit 471f085
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 3 deletions.
2 changes: 1 addition & 1 deletion include/attribute_store.h
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ enum class AttributePairType: uint8_t { String = 0, Float = 1, Bool = 2 };
// AttributePair is a key/value pair (with minzoom)
#pragma pack(push, 1)
struct AttributePair {
short keyIndex : 9;
unsigned short keyIndex : 9;
AttributePairType valueType : 2;
uint8_t minzoom : 5; // Support zooms from 0..31. In practice, we expect z16 to be the biggest minzoom.
union {
Expand Down
4 changes: 2 additions & 2 deletions src/attribute_store.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,8 @@ uint16_t AttributeKeyStore::key2index(const std::string& key) {
uint16_t newIndex = keys.size();

// This is very unlikely. We expect more like 50-100 keys.
if (newIndex >= 65535)
throw std::out_of_range("more than 65,536 unique keys");
if (newIndex >= 512)
throw std::out_of_range("more than 512 unique keys");

keys2indexSize++;
keys.push_back(key);
Expand Down
25 changes: 25 additions & 0 deletions test/attribute_store.test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -89,13 +89,38 @@ MU_TEST(test_attribute_store_reuses) {

mu_check(s1aIndex == s1bIndex);
}
}

MU_TEST(test_attribute_store_capacity) {
// We support a maximum of 511 attribute name, so confirm that we can roundtrip
// this value.
AttributePair pair(511, true, 0);
mu_check(pair.keyIndex == 511);

// Confirm that the attribute store will throw if we try to add more than 511 keys.
AttributeKeyStore keys;

for (int i = 1; i <= 511; i++) {
const uint16_t keyIndex = keys.key2index("key" + std::to_string(i));
mu_check(keyIndex == i);
}

// Trying to add a 512th key should throw
bool caughtException = false;

try {
keys.key2index("key512");
} catch (std::out_of_range) {
caughtException = true;
}

mu_check(caughtException == true);
}

MU_TEST_SUITE(test_suite_attribute_store) {
MU_RUN_TEST(test_attribute_store);
MU_RUN_TEST(test_attribute_store_reuses);
MU_RUN_TEST(test_attribute_store_capacity);
}

int main() {
Expand Down

0 comments on commit 471f085

Please sign in to comment.