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.

diff --git a/citadel/server/citadel_defs.h b/citadel/server/citadel_defs.h
index 7444081410742d84331dc4e19735fe9d57424d7d..24ab1eb0b4cd94453e7d17f9dbf44e4a2e6a4d06 100644 (file)
--- 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 978ae236657b5af1646c0bb6d7b31893b7334058..7009cc0675468a9e5647b5076a2571c288ca7e93 100644 (file)
--- 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 014dfa6c7768478b4fe07073b176a7ba772e9bd0..abcf62b121f8e57d6984da64ad377211f2d26020 100644 (file)
--- 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; i<purged; ++i) {
+               struct UseTable *u = (struct UseTable *)array_get_element_at(purge_list, i);
+               cdb_delete(CDB_USETABLE, &u->hash, 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 4c25352c85c3651bd8304875b3accce9440c0a5e..2652cefcc34fa5bda91788a3402c34ecd4206f03 100644 (file)
--- 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 982a2749c71df4b68cd42b094a6ce2967fc61d47..7e380a724a454dacfc8e8d4f07b614ce2b33b0f1 100644 (file)
--- 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 fb7bdb1cd0e3517dd1510b4cb61e5f920b5125ae..7ad960a69a77ae19e7a4a7451d438dcda979602d 100644 (file)
--- 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 d7ccd8e080e5c8ac88fbc9d34119f0e70c6c3621..afb70f83548db1d7e558c4ab59626ad9b53911ba 100755 (executable)
--- 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