From 321595f551d9e009f54b564b8459a293fe431fc4 Mon Sep 17 00:00:00 2001 From: Art Cancro Date: Sat, 11 Mar 2017 23:14:20 -0500 Subject: [PATCH] Removed race condition from CheckIfAlreadySeen() --- citadel/database.c | 55 +++++----------- citadel/database.h | 17 +---- citadel/include/ctdl_module.h | 16 ----- citadel/modules/network/serv_netspool.c | 5 +- .../networkclient/serv_networkclient.c | 15 +---- citadel/modules/pop3client/serv_pop3client.c | 2 +- citadel/modules/rssclient/serv_rssclient.c | 4 +- citadel/msgbase.c | 64 ------------------- citadel/netconfig.c | 32 +--------- citadel/sysdep.c | 8 +-- 10 files changed, 24 insertions(+), 194 deletions(-) diff --git a/citadel/database.c b/citadel/database.c index b55ec5fd6..3bed3c920 100644 --- a/citadel/database.c +++ b/citadel/database.c @@ -870,54 +870,29 @@ void cdb_trunc(int cdb) } -time_t CheckIfAlreadySeen(StrBuf *guid, time_t now, time_t antiexpire, eCheckType cType) + +// Has an item already been seen (is it in the CDB_USETABLE) ? +// Returns 0 if it hasn't, 1 if it has +// In either case, writes the item to the database for next time. +int CheckIfAlreadySeen(StrBuf *guid) { - time_t InDBTimeStamp = 0; + int found = 0; struct UseTable ut; struct cdbdata *cdbut; - memset(&ut, 0, sizeof(struct UseTable)); // zeroing it out makes it compress better - - if (cType != eWrite) - { - syslog(LOG_DEBUG, "Loading [%s]", ChrPtr(guid)); - cdbut = cdb_fetch(CDB_USETABLE, SKEY(guid)); - if ((cdbut != NULL) && (cdbut->ptr != NULL)) { - memcpy(&ut, cdbut->ptr, ((cdbut->len > sizeof(struct UseTable)) ? sizeof(struct UseTable) : cdbut->len)); - InDBTimeStamp = now - ut.ut_timestamp; - - if (InDBTimeStamp < antiexpire) - { - syslog(LOG_DEBUG, "Found - Not expired %ld < %ld", InDBTimeStamp, antiexpire); - cdb_free(cdbut); - return InDBTimeStamp; - } - else - { - syslog(LOG_DEBUG, "Found - Expired. %ld >= %ld", InDBTimeStamp, antiexpire); - cdb_free(cdbut); - } - } - else - { - if (cdbut) cdb_free(cdbut); - - syslog(LOG_DEBUG, "not Found"); - if (cType == eCheckUpdate) - return 0; - } - - if (cType == eCheckExist) - return InDBTimeStamp; + syslog(LOG_DEBUG, "CheckIfAlreadySeen(%s)", ChrPtr(guid)); + cdbut = cdb_fetch(CDB_USETABLE, SKEY(guid)); + 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 = now; - - /* rewrite the record anyway, to update the timestamp */ - syslog(LOG_DEBUG, "(re)writing usetable record"); + ut.ut_timestamp = time(NULL); cdb_store(CDB_USETABLE, SKEY(guid), &ut, sizeof(struct UseTable)); - return InDBTimeStamp; + return(found); } diff --git a/citadel/database.h b/citadel/database.h index 362d1724a..cfaa1bd60 100644 --- a/citadel/database.h +++ b/citadel/database.h @@ -53,22 +53,7 @@ struct CtdlCompressHeader { size_t compressed_len; }; -typedef enum __eCheckType { - eCheckExist, /* look up the item, return the timestamp if its there, 0 if not. */ - eCheckUpdate, /* if it exists, refresh in db timestamp. return the timstamp if its there, 0 if not. */ - eUpdate, /* insert/update the new value, return the old if its there, 0 if not. */ - eWrite /* write this to DB, unconditional. */ -}eCheckType; - -//time_t CheckIfAlreadySeen(const char *Facility, - //StrBuf *guid, - //time_t now, - //time_t antiexpire, - //eCheckType cType, - //long ccid, - //long ioid); -time_t CheckIfAlreadySeen(StrBuf *guid, time_t now, time_t antiexpire, eCheckType cType); - +int CheckIfAlreadySeen(StrBuf *guid); #endif /* DATABASE_H */ diff --git a/citadel/include/ctdl_module.h b/citadel/include/ctdl_module.h index 5fae2e0c8..6d7f93096 100644 --- a/citadel/include/ctdl_module.h +++ b/citadel/include/ctdl_module.h @@ -65,22 +65,6 @@ FMT_CITADEL, \ SUBJECT) - -#define CtdlAideFPMessage(TEXT, SUBJECT, N, STR, STRLEN, ccid, ioid, TIME) \ - flood_protect_quickie_message( \ - "Citadel", \ - NULL, \ - NULL, \ - AIDEROOM, \ - TEXT, \ - FMT_CITADEL, \ - SUBJECT, \ - N, \ - STR, \ - STRLEN, \ - ccid, \ - ioid, \ - TIME) /* * Hook functions available to modules. */ diff --git a/citadel/modules/network/serv_netspool.c b/citadel/modules/network/serv_netspool.c index 5d7daa7f0..0cb7c902a 100644 --- a/citadel/modules/network/serv_netspool.c +++ b/citadel/modules/network/serv_netspool.c @@ -498,8 +498,6 @@ void network_spoolout_room(SpoolControl *sc) int network_usetable(struct CtdlMessage *msg) { StrBuf *msgid; - struct CitContext *CCC = CC; - time_t now; /* Bail out if we can't generate a message ID */ if ((msg == NULL) || CM_IsEmpty(msg, emessageId)) @@ -519,8 +517,7 @@ int network_usetable(struct CtdlMessage *msg) return(0); } } - now = time(NULL); - if (CheckIfAlreadySeen(msgid, now, 0, eUpdate)) { + if (CheckIfAlreadySeen(msgid)) { FreeStrBuf(&msgid); return(1); } diff --git a/citadel/modules/networkclient/serv_networkclient.c b/citadel/modules/networkclient/serv_networkclient.c index 67fe0a779..c37f10bde 100644 --- a/citadel/modules/networkclient/serv_networkclient.c +++ b/citadel/modules/networkclient/serv_networkclient.c @@ -170,23 +170,10 @@ void DeleteNetworker(void *vptr) eNextState NWC_SendFailureMessage(AsyncIO *IO) { AsyncNetworker *NW = IO->Data; - long lens[2]; - const char *strs[2]; syslog(LOG_DEBUG, "NWC: %s\n", __FUNCTION__); - strs[0] = ChrPtr(NW->node); - lens[0] = StrLength(NW->node); - - strs[1] = ChrPtr(NW->IO.ErrMsg); - lens[1] = StrLength(NW->IO.ErrMsg); - CtdlAideFPMessage( - ChrPtr(NW->IO.ErrMsg), - "Networker error", - 2, strs, (long*) &lens, - CCID, IO->ID, - EvGetNow(IO)); - + CtdlAideMessage(ChrPtr(NW->IO.ErrMsg), "Networker error"); return eAbort; } diff --git a/citadel/modules/pop3client/serv_pop3client.c b/citadel/modules/pop3client/serv_pop3client.c index dc9fbd040..63f709e16 100644 --- a/citadel/modules/pop3client/serv_pop3client.c +++ b/citadel/modules/pop3client/serv_pop3client.c @@ -132,7 +132,7 @@ void pop3client_one_mailbox(char *room, const char *host, const char *user, cons // Make up the Use Table record so we can check if we've already seen this message. StrBuf *UT = NewStrBuf(); StrBufPrintf(UT, "pop3/%s/%s:%s@%s", room, oneuidl, user, host); - time_t already_seen = CheckIfAlreadySeen(UT, time(NULL), 0, eUpdate); + int already_seen = CheckIfAlreadySeen(UT); FreeStrBuf(&UT); // Only fetch the message if we haven't seen it before. diff --git a/citadel/modules/rssclient/serv_rssclient.c b/citadel/modules/rssclient/serv_rssclient.c index 9633614db..b7cd89ef3 100644 --- a/citadel/modules/rssclient/serv_rssclient.c +++ b/citadel/modules/rssclient/serv_rssclient.c @@ -134,7 +134,7 @@ void rss_end_element(void *data, const char *el) // check the use table StrBuf *u = NewStrBuf(); StrBufAppendPrintf(u, "rss/%s", r->item_id); - time_t already_seen = CheckIfAlreadySeen(u, time(NULL), 0, eUpdate); + int already_seen = CheckIfAlreadySeen(u); FreeStrBuf(&u); if (already_seen == 0) { @@ -185,7 +185,7 @@ void rss_end_element(void *data, const char *el) } } else { - syslog(LOG_DEBUG, "%s was already seen %ld seconds ago", r->item_id, already_seen); + syslog(LOG_DEBUG, "%s was already seen", r->item_id); } CM_Free(r->msg); diff --git a/citadel/msgbase.c b/citadel/msgbase.c index 88b21f5ad..7e66e4a87 100644 --- a/citadel/msgbase.c +++ b/citadel/msgbase.c @@ -3091,70 +3091,6 @@ long quickie_message(const char *from, } -void flood_protect_quickie_message(const char *from, - const char *fromaddr, - const char *to, - char *room, - const char *text, - int format_type, - const char *subject, - int nCriterions, - const char **CritStr, - const long *CritStrLen, - long ccid, - long ioid, - time_t NOW) -{ - int i; - u_char rawdigest[MD5_DIGEST_LEN]; - struct MD5Context md5context; - StrBuf *guid; - char timestamp[64]; - long tslen; - static const time_t tsday = (8*60*60); /* just care for a day... */ - time_t seenstamp; - - tslen = snprintf(timestamp, sizeof(timestamp), "%ld", tsday); - MD5Init(&md5context); - - for (i = 0; i < nCriterions; i++) - MD5Update(&md5context, - (const unsigned char*)CritStr[i], CritStrLen[i]); - MD5Update(&md5context, - (const unsigned char*)timestamp, tslen); - MD5Final(rawdigest, &md5context); - - guid = NewStrBufPlain(NULL, - MD5_DIGEST_LEN * 2 + 12); - StrBufHexEscAppend(guid, NULL, rawdigest, MD5_DIGEST_LEN); - StrBufAppendBufPlain(guid, HKEY("_fldpt"), 0); - if (StrLength(guid) > 40) - StrBufCutAt(guid, 40, NULL); - - seenstamp = CheckIfAlreadySeen(guid, NOW, tsday, eUpdate); - if ((seenstamp > 0) && (seenstamp < tsday)) - { - FreeStrBuf(&guid); - /* yes, we did. flood protection kicks in. */ - syslog(LOG_DEBUG, "not sending message again - %ld < %ld \n", seenstamp, tsday); - return; - } - else - { - syslog(LOG_DEBUG, "sending message. %ld >= %ld", seenstamp, tsday); - FreeStrBuf(&guid); - /* no, this message isn't sent recently; go ahead. */ - quickie_message(from, - fromaddr, - to, - room, - text, - format_type, - subject); - } -} - - /* * Back end function used by CtdlMakeMessage() and similar functions */ diff --git a/citadel/netconfig.c b/citadel/netconfig.c index cce05ef41..239796981 100644 --- a/citadel/netconfig.c +++ b/citadel/netconfig.c @@ -633,8 +633,6 @@ void cmd_netp(char *cmdbuf) StrBuf *NodeStr; long nodelen; int v; - long lens[2]; - const char *strs[2]; const StrBuf *secret = NULL; const StrBuf *nexthop = NULL; @@ -654,20 +652,7 @@ void cmd_netp(char *cmdbuf) ); syslog(LOG_WARNING, "%s", err_buf); cprintf("%d authentication failed\n", ERROR + PASSWORD_REQUIRED); - - strs[0] = CCC->cs_addr; - lens[0] = strlen(CCC->cs_addr); - - strs[1] = "SRV_UNKNOWN"; - lens[1] = sizeof("SRV_UNKNOWN") - 1; - - CtdlAideFPMessage( - err_buf, - "IGNet Networking.", - 2, strs, (long*) &lens, - CCC->cs_pid, 0, - time(NULL)); - + CtdlAideMessage(err_buf, "IGNet Networking"); DeleteHash(&working_ignetcfg); FreeStrBuf(&NodeStr); return; @@ -682,20 +667,7 @@ void cmd_netp(char *cmdbuf) syslog(LOG_WARNING, "%s", err_buf); cprintf("%d authentication failed\n", ERROR + PASSWORD_REQUIRED); - strs[0] = CCC->cs_addr; - lens[0] = strlen(CCC->cs_addr); - - strs[1] = "SRV_PW"; - lens[1] = sizeof("SRV_PW") - 1; - - CtdlAideFPMessage( - err_buf, - "IGNet Networking.", - 2, strs, - (long*) &lens, - CCC->cs_pid, 0, - time(NULL)); - + CtdlAideMessage(err_buf, "IGNet Networking"); DeleteHash(&working_ignetcfg); FreeStrBuf(&NodeStr); return; diff --git a/citadel/sysdep.c b/citadel/sysdep.c index 1bde945f4..e5a829fec 100644 --- a/citadel/sysdep.c +++ b/citadel/sysdep.c @@ -1085,8 +1085,6 @@ void checkcrash(void) if (nFireUpsNonRestart != nFireUps) { StrBuf *CrashMail; - const char *msgs[1] = {"crash"}; - const long lens[1] = {sizeof("crash") - 1}; CrashMail = NewStrBuf(); syslog(LOG_ALERT, "Posting crash message\n"); StrBufPrintf(CrashMail, @@ -1103,11 +1101,7 @@ void checkcrash(void) " If you have already done this, the core dump is likely to be found at %score.%d\n" , ctdl_run_dir, ForkedPid); - CtdlAideFPMessage(ChrPtr(CrashMail), - "Citadel server process terminated unexpectedly", - 1, msgs, lens, - 0, 0, - time(NULL)); + CtdlAideMessage(ChrPtr(CrashMail), "Citadel server process terminated unexpectedly"); FreeStrBuf(&CrashMail); } } -- 2.30.2