From 1a339033f34d0ce6719ef4f5308b757ef43cbfd2 Mon Sep 17 00:00:00 2001 From: attilamolnar Date: Sun, 27 May 2012 23:05:12 +0200 Subject: Fix generating invalid UIDs after current_uid is 000Z99999 (next UID became 000[AAAAA) --- src/server.cpp | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) (limited to 'src') diff --git a/src/server.cpp b/src/server.cpp index dab920bb6..092826361 100644 --- a/src/server.cpp +++ b/src/server.cpp @@ -101,20 +101,12 @@ void InspIRCd::IncrementUID(int pos) * A again, in an iterative fashion.. so.. * AAA9 -> AABA, and so on. -- w00t */ - if (pos == 3) + if ((pos == 3) && (current_uid[3] == '9')) { // At pos 3, if we hit '9', we've run out of available UIDs, and need to reset to AAA..AAA. - if (current_uid[pos] == '9') + for (int i = 3; i < UUID_LENGTH-1; i++) { - for (int i = 3; i < (UUID_LENGTH - 1); i++) - { - current_uid[i] = 'A'; - } - } - else - { - // Buf if we haven't, just keep incrementing merrily. - current_uid[pos]++; + current_uid[i] = 'A'; } } else -- cgit v1.2.3 From ae6e056a011fc16af67e8dc311c4ace902e163fb Mon Sep 17 00:00:00 2001 From: attilamolnar Date: Sun, 27 May 2012 23:08:14 +0200 Subject: While at it, use a constant parameter for calling IncrementUID in GetUID Get rid of curindex, use a bool to determine if we need to initialize --- src/server.cpp | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) (limited to 'src') diff --git a/src/server.cpp b/src/server.cpp index 092826361..adaaa7d2c 100644 --- a/src/server.cpp +++ b/src/server.cpp @@ -138,17 +138,18 @@ void InspIRCd::IncrementUID(int pos) */ std::string InspIRCd::GetUID() { - static int curindex = -1; + static bool inited = false; /* - * If -1, we're setting up. Copy SID into the first three digits, 9's to the rest, null term at the end + * If we're setting up, copy SID into the first three digits, 9's to the rest, null term at the end * Why 9? Well, we increment before we find, otherwise we have an unnecessary copy, and I want UID to start at AAA..AA * and not AA..AB. So by initialising to 99999, we force it to rollover to AAAAA on the first IncrementUID call. * Kind of silly, but I like how it looks. * -- w */ - if (curindex == -1) + if (!inited) { + inited = true; current_uid[0] = Config->sid[0]; current_uid[1] = Config->sid[1]; current_uid[2] = Config->sid[2]; @@ -156,8 +157,6 @@ std::string InspIRCd::GetUID() for (int i = 3; i < (UUID_LENGTH - 1); i++) current_uid[i] = '9'; - curindex = UUID_LENGTH - 2; // look at the end of the string now kthx, ignore null - // Null terminator. Important. current_uid[UUID_LENGTH - 1] = '\0'; } @@ -165,7 +164,7 @@ std::string InspIRCd::GetUID() while (1) { // Add one to the last UID - this->IncrementUID(curindex); + this->IncrementUID(UUID_LENGTH - 2); if (this->FindUUID(current_uid)) { -- cgit v1.2.3 From e3e7cb89e112bbeed2b3e43798a16fe557a3992a Mon Sep 17 00:00:00 2001 From: attilamolnar Date: Sun, 27 May 2012 23:30:02 +0200 Subject: Add testsuite tests for UID generation --- include/inspircd.h | 4 +++ include/testsuite.h | 2 ++ src/testsuite.cpp | 77 +++++++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 83 insertions(+) (limited to 'src') diff --git a/include/inspircd.h b/include/inspircd.h index cabb24aa0..9c9609530 100644 --- a/include/inspircd.h +++ b/include/inspircd.h @@ -256,6 +256,8 @@ DEFINE_HANDLER1(IsSIDHandler, bool, const std::string&); DEFINE_HANDLER1(RehashHandler, void, const std::string&); DEFINE_HANDLER3(OnCheckExemptionHandler, ModResult, User*, Channel*, const std::string&); +class TestSuite; + /** The main class of the irc server. * This class contains instances of all the other classes in this software. * Amongst other things, it contains a ModeParser, a DNS object, a CommandParser @@ -855,6 +857,8 @@ class CoreExport InspIRCd { return this->ReadBuffer; } + + friend class TestSuite; }; ENTRYPOINT; diff --git a/include/testsuite.h b/include/testsuite.h index 618615dc9..f91e508c9 100644 --- a/include/testsuite.h +++ b/include/testsuite.h @@ -21,6 +21,7 @@ class TestSuite { + bool RealGenerateUIDTests(); public: TestSuite(); ~TestSuite(); @@ -29,6 +30,7 @@ class TestSuite bool DoWildTests(); bool DoCommaSepStreamTests(); bool DoSpaceSepStreamTests(); + bool DoGenerateUIDTests(); }; #endif diff --git a/src/testsuite.cpp b/src/testsuite.cpp index 064724392..59e6102e7 100644 --- a/src/testsuite.cpp +++ b/src/testsuite.cpp @@ -65,6 +65,7 @@ TestSuite::TestSuite() cout << "(5) Wildcard and CIDR tests\n"; cout << "(6) Comma sepstream tests\n"; cout << "(7) Space sepstream tests\n"; + cout << "(8) UID generation tests\n"; cout << endl << "(X) Exit test suite\n"; @@ -105,6 +106,9 @@ TestSuite::TestSuite() case '7': cout << (DoSpaceSepStreamTests() ? "\nSUCCESS!\n" : "\nFAILURE\n"); break; + case '8': + cout << (DoGenerateUIDTests() ? "\nSUCCESS!\n" : "\nFAILURE\n"); + break; case 'X': return; break; @@ -327,6 +331,79 @@ bool TestSuite::DoThreadTests() return true; } +bool TestSuite::DoGenerateUIDTests() +{ + bool success = RealGenerateUIDTests(); + + // Reset the UID generation state so running the tests multiple times won't mess things up + for (unsigned int i = 0; i < 3; i++) + ServerInstance->current_uid[i] = ServerInstance->Config->sid[i]; + for (unsigned int i = 3; i < UUID_LENGTH-1; i++) + ServerInstance->current_uid[i] = '9'; + + ServerInstance->current_uid[UUID_LENGTH-1] = '\0'; + + return success; +} + +bool TestSuite::RealGenerateUIDTests() +{ + std::string first_uid = ServerInstance->GetUID(); + if (first_uid.length() != UUID_LENGTH-1) + { + cout << "GENERATEUID: Generated UID is " << first_uid.length() << " characters long instead of " << UUID_LENGTH-1 << endl; + return false; + } + + if (ServerInstance->current_uid[UUID_LENGTH-1] != '\0') + { + cout << "GENERATEUID: The null terminator is missing from the end of current_uid" << endl; + return false; + } + + // The correct UID when generating one for the first time is ...AAAAAA + std::string correct_uid = ServerInstance->Config->sid + std::string(UUID_LENGTH - 4, 'A'); + if (first_uid != correct_uid) + { + cout << "GENERATEUID: Generated an invalid first UID: " << first_uid << " instead of " << correct_uid << endl; + return false; + } + + // Set current_uid to be ...Z99999 + ServerInstance->current_uid[3] = 'Z'; + for (unsigned int i = 4; i < UUID_LENGTH-1; i++) + ServerInstance->current_uid[i] = '9'; + + // Store the UID we'll be incrementing so we can display what's wrong later if necessary + std::string before_increment(ServerInstance->current_uid); + std::string generated_uid = ServerInstance->GetUID(); + + // Correct UID after incrementing ...Z99999 is ...0AAAAA + correct_uid = ServerInstance->Config->sid + "0" + std::string(UUID_LENGTH - 5, 'A'); + + if (generated_uid != correct_uid) + { + cout << "GENERATEUID: Generated an invalid UID after incrementing " << before_increment << ": " << generated_uid << " instead of " << correct_uid << endl; + return false; + } + + // Set current_uid to be ...999999 to see if it rolls over correctly + for (unsigned int i = 3; i < UUID_LENGTH-1; i++) + ServerInstance->current_uid[i] = '9'; + + before_increment.assign(ServerInstance->current_uid); + generated_uid = ServerInstance->GetUID(); + + // Correct UID after rolling over is the first UID we've generated (...AAAAAA) + if (generated_uid != first_uid) + { + cout << "GENERATEUID: Generated an invalid UID after incrementing " << before_increment << ": " << generated_uid << " instead of " << first_uid << endl; + return false; + } + + return true; +} + TestSuite::~TestSuite() { cout << "\n\n*** END OF TEST SUITE ***\n"; -- cgit v1.2.3