From d8de66ffdc9d5f595357f76a78339bb29d094891 Mon Sep 17 00:00:00 2001 From: Art Cancro Date: Sat, 3 Jun 2023 18:41:01 -0900 Subject: [PATCH] Remediated an unrecoverable problem in CDB_USETABLE. Due to a missing null terminator, records were being stored with excessive lengths, resulting in records whose keys did not match. Unfortunately this means we have to zero out the table during an upgrade to Citadel Server 975. No user data will be lost, but incoming data such as RSS feeds and POP3 retrievals may produce some unwanted duplicates. The new implementation not only remediates this problem, but also works using a hash of the item rather than the item itself. This produces records that are only 12 bytes long. --- citadel/server/citadel_defs.h | 2 +- citadel/server/database.c | 14 ++++----- citadel/server/modules/expire/serv_expire.c | 29 ++++++------------- citadel/server/modules/upgrade/serv_upgrade.c | 5 +++- citadel/server/server.h | 4 +-- citadel/utils/ctdl3264.c | 8 ++--- do-release.sh | 2 +- 7 files changed, 28 insertions(+), 36 deletions(-) diff --git a/citadel/server/citadel_defs.h b/citadel/server/citadel_defs.h index 744408141..24ab1eb0b 100644 --- a/citadel/server/citadel_defs.h +++ b/citadel/server/citadel_defs.h @@ -21,7 +21,7 @@ #include "typesize.h" #include "ipcdef.h" -#define REV_LEVEL 973 // This version +#define REV_LEVEL 975 // This version #define REV_MIN 591 // Oldest compatible database #define EXPORT_REV_MIN 931 // Oldest compatible export files #define LIBCITADEL_MIN 951 // Minimum required version of libcitadel diff --git a/citadel/server/database.c b/citadel/server/database.c index 978ae2366..7009cc067 100644 --- a/citadel/server/database.c +++ b/citadel/server/database.c @@ -749,19 +749,19 @@ int CheckIfAlreadySeen(StrBuf *guid) { int found = 0; struct UseTable ut; struct cdbdata *cdbut; + int hash = HashLittle(ChrPtr(guid), StrLength(guid)); - syslog(LOG_DEBUG, "db: CheckIfAlreadySeen(%s)", ChrPtr(guid)); - cdbut = cdb_fetch(CDB_USETABLE, SKEY(guid)); + syslog(LOG_DEBUG, "db: CheckIfAlreadySeen(%d)", hash); + cdbut = cdb_fetch(CDB_USETABLE, &hash, sizeof(hash)); if (cdbut != NULL) { found = 1; cdb_free(cdbut); } - // (Re)write the record, to update the timestamp. Zeroing it out makes it compress better. - memset(&ut, 0, sizeof(struct UseTable)); - memcpy(ut.ut_msgid, SKEY(guid)); - ut.ut_timestamp = time(NULL); - cdb_store(CDB_USETABLE, SKEY(guid), &ut, sizeof(struct UseTable)); + // (Re)write the record, to update the timestamp. + ut.hash = hash; + ut.timestamp = time(NULL); + cdb_store(CDB_USETABLE, &hash, sizeof(hash), &ut, sizeof(struct UseTable)); return (found); } diff --git a/citadel/server/modules/expire/serv_expire.c b/citadel/server/modules/expire/serv_expire.c index 014dfa6c7..abcf62b12 100644 --- a/citadel/server/modules/expire/serv_expire.c +++ b/citadel/server/modules/expire/serv_expire.c @@ -66,11 +66,6 @@ struct ctdlroomref { long msgnum; }; -struct UPurgeList { - struct UPurgeList *next; - char up_key[256]; -}; - struct EPurgeList { struct EPurgeList *next; int ep_keylen; @@ -584,13 +579,13 @@ int PurgeVisits(void) { // Purge the use table of old entries. +// Holy crap, this is WAY better. We need to replace most linked lists with arrays. int PurgeUseTable(StrBuf *ErrMsg) { int purged = 0; int total = 0; struct cdbdata *cdbut; struct UseTable ut; - struct UPurgeList *ul = NULL; - struct UPurgeList *uptr; + Array *purge_list = array_new(sizeof(int)); // Phase 1: traverse through the table, discovering old records... @@ -606,26 +601,20 @@ int PurgeUseTable(StrBuf *ErrMsg) { } cdb_free(cdbut); - if ( (time(NULL) - ut.ut_timestamp) > USETABLE_RETAIN ) { - uptr = (struct UPurgeList *) malloc(sizeof(struct UPurgeList)); - if (uptr != NULL) { - uptr->next = ul; - safestrncpy(uptr->up_key, ut.ut_msgid, sizeof uptr->up_key); - ul = uptr; - } + if ( (time(NULL) - ut.timestamp) > USETABLE_RETAIN ) { + array_append(purge_list, &ut.hash); ++purged; } - } // Phase 2: delete the records syslog(LOG_DEBUG, "Purge use table: phase 2"); - while (ul != NULL) { - cdb_delete(CDB_USETABLE, ul->up_key, strlen(ul->up_key)); - uptr = ul->next; - free(ul); - ul = uptr; + int i; + for (i=0; ihash, sizeof(int)); } + array_free(purge_list); syslog(LOG_DEBUG, "Purge use table: finished (purged %d of %d records)", purged, total); return(purged); diff --git a/citadel/server/modules/upgrade/serv_upgrade.c b/citadel/server/modules/upgrade/serv_upgrade.c index 4c25352c8..2652cefcc 100644 --- a/citadel/server/modules/upgrade/serv_upgrade.c +++ b/citadel/server/modules/upgrade/serv_upgrade.c @@ -3,7 +3,7 @@ // guesses about what kind of data format changes need to be applied, and // we apply them transparently. // -// Copyright (c) 1987-2022 by the citadel.org team +// Copyright (c) 1987-2023 by the citadel.org team // // This program is open source software; you can redistribute it and/or // modify it under the terms of the GNU General Public License version 3. @@ -484,6 +484,9 @@ void pre_startup_upgrades(void) { if ((oldver > 000) && (oldver < 973)) { // This was the old extauth table. cdb_trunc(CDB_UNUSED1); // We will do this better someday. } + if ((oldver > 000) && (oldver < 975)) { // An unrepairable defect was found in this table. + cdb_trunc(CDB_USETABLE); // We have to discard it and start over. + } CtdlSetConfigInt("MM_hosted_upgrade_level", REV_LEVEL); diff --git a/citadel/server/server.h b/citadel/server/server.h index 982a2749c..7e380a724 100644 --- a/citadel/server/server.h +++ b/citadel/server/server.h @@ -129,8 +129,8 @@ struct ser_ret { // The S_USETABLE database is used in several modules now, so we define its format here. struct UseTable { - char ut_msgid[SIZ]; - time_t ut_timestamp; + int hash; + time_t timestamp; }; diff --git a/citadel/utils/ctdl3264.c b/citadel/utils/ctdl3264.c index fb7bdb1cd..7ad960a69 100644 --- a/citadel/utils/ctdl3264.c +++ b/citadel/utils/ctdl3264.c @@ -365,7 +365,7 @@ void convert_dir(int which_cdb, DBT *in_key, DBT *in_data, DBT *out_key, DBT *ou // convert function for a use table record void convert_usetable(int which_cdb, DBT *in_key, DBT *in_data, DBT *out_key, DBT *out_data) { - // the key is a string + // the key is an int, which is the same size (32 bits) on both 32 and 64 bit systems out_key->size = in_key->size; out_key->data = realloc(out_key->data, out_key->size); memcpy(out_key->data, in_key->data, in_key->size); @@ -378,10 +378,10 @@ void convert_usetable(int which_cdb, DBT *in_key, DBT *in_data, DBT *out_key, DB struct UseTable *use64 = (struct UseTable *)out_data->data; // the data - strcpy(use64->ut_msgid, use32->ut_msgid); - use64->ut_timestamp = (time_t) use32->ut_timestamp; + use64->hash = use32->hash; + use64->timestamp = (time_t) use32->timestamp; - printf("\033[32m\033[1muse table: %s , %s\033[0m\n", use64->ut_msgid, asctime(localtime(&use64->ut_timestamp))); + printf("\033[32m\033[1muse table: %d , %s\033[0m\n", use64->hash, asctime(localtime(&use64->timestamp))); } diff --git a/do-release.sh b/do-release.sh index d7ccd8e08..afb70f835 100755 --- a/do-release.sh +++ b/do-release.sh @@ -33,7 +33,7 @@ echo -e '\033[33m\033[1mwebcit \033[32m was version \033[33m'$webcit_ver echo -e '\033[33m\033[1mtextclient \033[32m was version \033[33m'$textclient_version'\033[0m' echo -e '\033[33m\033[1mnew release\033[32m will be version \033[33m'$NEW_VERSION'\033[0m' echo -e '' -echo -e '\033[41m\033[37m\033[1m THIS WILL INITIATE THE RELEASE PIPELINE. \033[0m' +#echo -e '\033[41m\033[37m\033[1m THIS WILL INITIATE THE RELEASE PIPELINE. \033[0m' echo -n 'Proceed (y/n) ? ' read x -- 2.30.2