From 471f0852ec42dc3c062d9e9066b40cf5a641c95f Mon Sep 17 00:00:00 2001
From: Colin Dellow <cldellow@gmail.com>
Date: Fri, 20 Sep 2024 22:41:04 -0400
Subject: [PATCH] Fix #750: allow no more than 512 attribute names

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.
---
 include/attribute_store.h     |  2 +-
 src/attribute_store.cpp       |  4 ++--
 test/attribute_store.test.cpp | 25 +++++++++++++++++++++++++
 3 files changed, 28 insertions(+), 3 deletions(-)

diff --git a/include/attribute_store.h b/include/attribute_store.h
index 9ac8adbf..75ff6e9c 100644
--- a/include/attribute_store.h
+++ b/include/attribute_store.h
@@ -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 {
diff --git a/src/attribute_store.cpp b/src/attribute_store.cpp
index 1f63edbe..facafdbf 100644
--- a/src/attribute_store.cpp
+++ b/src/attribute_store.cpp
@@ -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);
diff --git a/test/attribute_store.test.cpp b/test/attribute_store.test.cpp
index aa721787..aa3c842f 100644
--- a/test/attribute_store.test.cpp
+++ b/test/attribute_store.test.cpp
@@ -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() {