From e84c54da7d9dbaa3d1af3ea16da1d20f310ec7ec Mon Sep 17 00:00:00 2001 From: Art Cancro Date: Thu, 30 Jun 2011 22:47:57 -0400 Subject: [PATCH] Generate wiki diffs using a temp file instead of a big string of realloc() calls. Noted a block of code in which we are smashing the stack and need to fix. --- citadel/context.c | 4 +-- citadel/modules/wiki/serv_wiki.c | 50 ++++++++++++++++++++++---------- citadel/serv_extensions.c | 10 ++++--- 3 files changed, 43 insertions(+), 21 deletions(-) diff --git a/citadel/context.c b/citadel/context.c index d890e1c13..c77f3cdd2 100644 --- a/citadel/context.c +++ b/citadel/context.c @@ -359,9 +359,9 @@ void RemoveContext (CitContext *con) /* * If the client is still connected, blow 'em away. - * if the socket is 0, its already gone or was never there. + * if the socket is 0 or -1, its already gone or was never there. */ - if (con->client_socket != 0) + if (con->client_socket > 0) { syslog(LOG_NOTICE, "Closing socket %d\n", con->client_socket); close(con->client_socket); diff --git a/citadel/modules/wiki/serv_wiki.c b/citadel/modules/wiki/serv_wiki.c index 49a039809..62b2132d5 100644 --- a/citadel/modules/wiki/serv_wiki.c +++ b/citadel/modules/wiki/serv_wiki.c @@ -83,6 +83,7 @@ int wiki_upload_beforesave(struct CtdlMessage *msg) { struct CtdlMessage *history_msg = NULL; char diff_old_filename[PATH_MAX]; char diff_new_filename[PATH_MAX]; + char diff_out_filename[PATH_MAX]; char diff_cmd[PATH_MAX]; FILE *fp; int rv; @@ -90,7 +91,6 @@ int wiki_upload_beforesave(struct CtdlMessage *msg) { char boundary[256]; char prefixed_boundary[258]; char buf[1024]; - int nbytes = 0; char *diffbuf = NULL; size_t diffbuf_len = 0; char *ptr = NULL; @@ -154,6 +154,7 @@ int wiki_upload_beforesave(struct CtdlMessage *msg) { */ CtdlMakeTempFileName(diff_old_filename, sizeof diff_old_filename); CtdlMakeTempFileName(diff_new_filename, sizeof diff_new_filename); + CtdlMakeTempFileName(diff_out_filename, sizeof diff_out_filename); if (old_msg != NULL) { fp = fopen(diff_old_filename, "w"); @@ -166,29 +167,37 @@ int wiki_upload_beforesave(struct CtdlMessage *msg) { rv = fwrite(msg->cm_fields['M'], strlen(msg->cm_fields['M']), 1, fp); fclose(fp); - diffbuf_len = 0; - diffbuf = NULL; snprintf(diff_cmd, sizeof diff_cmd, - "diff -u %s %s", + "diff -u %s %s >%s", diff_new_filename, - ((old_msg != NULL) ? diff_old_filename : "/dev/null") + ((old_msg != NULL) ? diff_old_filename : "/dev/null"), + diff_out_filename ); - fp = popen(diff_cmd, "r"); + syslog(LOG_DEBUG, "diff cmd: %s", diff_cmd); + rv = system(diff_cmd); + syslog(LOG_DEBUG, "diff cmd returned %d", rv); + + diffbuf_len = 0; + diffbuf = NULL; + fp = fopen(diff_out_filename, "r"); + if (fp == NULL) { + fp = fopen("/dev/null", "r"); + } if (fp != NULL) { - do { - diffbuf = realloc(diffbuf, diffbuf_len + 1025); - nbytes = fread(&diffbuf[diffbuf_len], 1, 1024, fp); - diffbuf_len += nbytes; - } while (nbytes == 1024); + fseek(fp, 0L, SEEK_END); + diffbuf_len = ftell(fp); + fseek(fp, 0L, SEEK_SET); + diffbuf = malloc(diffbuf_len + 1); + fread(diffbuf, diffbuf_len, 1, fp); diffbuf[diffbuf_len] = 0; - if (pclose(fp) != 0) { - syslog(LOG_ERR, "pclose() returned an error - diff failed\n"); - } + fclose(fp); } - syslog(LOG_DEBUG, "diff length is %d bytes\n", diffbuf_len); + + syslog(LOG_DEBUG, "diff length is %d bytes", diffbuf_len); unlink(diff_old_filename); unlink(diff_new_filename); + unlink(diff_out_filename); /* Determine whether this was a bogus (empty) edit */ if ((diffbuf_len = 0) && (diffbuf != NULL)) { @@ -272,6 +281,10 @@ int wiki_upload_beforesave(struct CtdlMessage *msg) { } } while ( (IsEmptyStr(boundary)) && (*ptr != 0) ); + + /****************** STACK SMASH IS SOMEWHERE BELOW THIS LINE **************/ + + /* Now look for the first boundary. That is where we need to insert our fun. */ if (!IsEmptyStr(boundary)) { @@ -311,6 +324,9 @@ int wiki_upload_beforesave(struct CtdlMessage *msg) { } history_msg->cm_fields['T'] = realloc(history_msg->cm_fields['T'], 32); + if (history_msg->cm_fields['T'] == NULL) { + syslog(LOG_EMERG, "*** REALLOC FAILED *** %s", strerror(errno)); + } snprintf(history_msg->cm_fields['T'], 32, "%ld", time(NULL)); CtdlSubmitMsg(history_msg, NULL, "", 0); @@ -319,6 +335,10 @@ int wiki_upload_beforesave(struct CtdlMessage *msg) { syslog(LOG_ALERT, "Empty boundary string in history message. No history!\n"); } + + /****************** STACK SMASH IS SOMEWHERE BELOW THIS LINE **************/ + + free(diffbuf); free(history_msg); return(0); diff --git a/citadel/serv_extensions.c b/citadel/serv_extensions.c index 33c83f738..76dbacb4a 100644 --- a/citadel/serv_extensions.c +++ b/citadel/serv_extensions.c @@ -969,8 +969,7 @@ int PerformMessageHooks(struct CtdlMessage *msg, int EventType) /* Other code may elect to protect this message from server-side * handlers; if this is the case, don't do anything. - syslog(LOG_DEBUG, "** Event type is %d, flags are %d\n", - EventType, msg->cm_flags); + syslog(LOG_DEBUG, "** Event type is %d, flags are %d\n", EventType, msg->cm_flags); */ if (msg->cm_flags & CM_SKIP_HOOKS) { syslog(LOG_DEBUG, "Skipping hooks\n"); @@ -979,10 +978,13 @@ int PerformMessageHooks(struct CtdlMessage *msg, int EventType) /* Otherwise, run all the hooks appropriate to this event type. */ + int num_hooks_processed = 0; for (fcn = MessageHookTable; fcn != NULL; fcn = fcn->next) { if (fcn->eventtype == EventType) { - total_retval = total_retval + - (*fcn->h_function_pointer) (msg); + total_retval = total_retval + (*fcn->h_function_pointer) (msg); + syslog(LOG_DEBUG, "%d hooks completed, total_retval=%d", + ++num_hooks_processed, total_retval + ); } } -- 2.30.2