From: Art Cancro <ajc@citadel.org>
Date: Fri, 26 Jan 2024 21:51:39 +0000 (-0500)
Subject: Assert that eMessageText MUST be serialized last.
X-Git-Tag: v997~8
X-Git-Url: https://code.citadel.org/?p=citadel.git;a=commitdiff_plain;h=cd6c194901557e9e068643d01d117a7824d36533

Assert that eMessageText MUST be serialized last.

Also assert that the calculated length of the serialized message
is actually the final length.  Either of these conditions not
being true is worth crashing the server so the developer who broke
it knows immediately.
---

diff --git a/citadel/server/msgbase.c b/citadel/server/msgbase.c
index 3588ac8d0..259a3e5ab 100644
--- a/citadel/server/msgbase.c
+++ b/citadel/server/msgbase.c
@@ -2396,6 +2396,7 @@ struct ser_ret CtdlSerializeMessage(struct CtdlMessage *msg) {
 	}
 
 	ret.len = 3;
+	assert(FieldOrder[NDiskFields-1] == eMessageText);		// Message text MUST be last!
 	for (i=0; i < NDiskFields; ++i) {
 		if (msg->cm_fields[FieldOrder[i]] != NULL) {
 			ret.len += msg->cm_lengths[FieldOrder[i]] + 2;
@@ -2417,16 +2418,14 @@ struct ser_ret CtdlSerializeMessage(struct CtdlMessage *msg) {
 
 	for (i=0; i < NDiskFields; ++i) {
 		if (msg->cm_fields[FieldOrder[i]] != NULL) {
+									// future improvement: do the bigmsg check right here
 			ret.ser[wlen++] = (char)FieldOrder[i];
 			memcpy(&ret.ser[wlen], msg->cm_fields[FieldOrder[i]], msg->cm_lengths[FieldOrder[i]] + 1);
 			wlen = wlen + msg->cm_lengths[FieldOrder[i]] + 1;
 		}
 	}
 
-	if (ret.len != wlen) {
-		syslog(LOG_ERR, "msgbase: ERROR; len=%ld wlen=%ld", (long)ret.len, (long)wlen);
-	}
-
+	assert(ret.len == wlen);					// Make sure we measured it correctly
 	return(ret);
 }
 
@@ -2526,13 +2525,16 @@ long CtdlSubmitMsg(struct CtdlMessage *msg,	// message to save
 			string_trim(content_type);
 			aptr = content_type;
 			while (!IsEmptyStr(aptr)) {
-				if ((*aptr == ';')
-				    || (*aptr == ' ')
-				    || (*aptr == 13)
-				    || (*aptr == 10)) {
+				if (	(*aptr == ';')
+					|| (*aptr == ' ')
+					|| (*aptr == 13)
+					|| (*aptr == 10)
+				) {
 					*aptr = 0;
 				}
-				else aptr++;
+				else {
+					aptr++;
+				}
 			}
 		}
 	}
@@ -3020,10 +3022,12 @@ struct CtdlMessage *CtdlMakeMessageLen(
 			long IsAscii;
 			IsAscii = -1;
 			i = 0;
-			while ((subject[i] != '\0') && (IsAscii = isascii(subject[i]) != 0 ))
+			while ((subject[i] != '\0') && (IsAscii = isascii(subject[i]) != 0 )) {
 				i++;
-			if (IsAscii != 0)
+			}
+			if (IsAscii != 0) {
 				CM_SetField(msg, eMsgSubject, subject);
+			}
 			else {	// ok, we've got utf8 in the string.
 				char *rfc2047Subj;
 				rfc2047Subj = rfc2047encode(subject, length);
@@ -3203,11 +3207,7 @@ void PutMetaData(struct MetaData *smibuf) {
 
 	// Use the negative of the message number for the metadata db index
 	TheIndex = (0L - smibuf->meta_msgnum);
-
-	cdb_store(CDB_MSGMAIN,
-		  &TheIndex, (int)sizeof(long),
-		  smibuf, (int)sizeof(struct MetaData)
-	);
+	cdb_store(CDB_MSGMAIN, &TheIndex, (int)sizeof(long), smibuf, (int)sizeof(struct MetaData));
 }
 
 
diff --git a/citadel/server/msgbase.h b/citadel/server/msgbase.h
index 3e7e168d2..356e0ff9c 100644
--- a/citadel/server/msgbase.h
+++ b/citadel/server/msgbase.h
@@ -120,6 +120,12 @@ int  CM_IsValidMsg     (struct CtdlMessage *msg);
 #define CM_RANGE(Message, Which) Message->cm_fields[Which], \
 		Message->cm_fields[Which] + Message->cm_lengths[Which]
 
+// Serialization routines use this struct to return a pointer and a length
+struct ser_ret {
+	size_t len;
+	unsigned char *ser;
+};
+
 struct ser_ret CtdlSerializeMessage(struct CtdlMessage *);
 struct CtdlMessage *CtdlDeserializeMessage(long msgnum, int with_body, const char *Buffer, long Length);
 void ReplicationChecks(struct CtdlMessage *);
diff --git a/citadel/server/server.h b/citadel/server/server.h
index f58a9c3f1..8d1f7d0ff 100644
--- a/citadel/server/server.h
+++ b/citadel/server/server.h
@@ -1,6 +1,6 @@
 // Data types for the Citadel Server
 //
-// Copyright (c) 1987-2023 by the citadel.org team
+// Copyright (c) 1987-2024 by the citadel.org team
 //
 // This program is open source software.  Use, duplication, or disclosure
 // is subject to the terms of the GNU General Public License, version 3.
@@ -127,13 +127,6 @@ struct arcq {
 };
 
 
-// Serialization routines use this struct to return a pointer and a length
-struct ser_ret {
-	size_t len;
-	unsigned char *ser;
-};
-
-
 // The S_USETABLE database is used in several modules now, so we define its format here.
 struct UseTable {
 	int hash;