Factor out code to read mailboxes, and make read more robust

New tel_read_header(), tel_read_body().  Use them in rea(),
show_first_tel(), copy_and_expire().

rea() now stops when it encounters a corrupt telegram, and logs the
problem.  Before, error detection was incomplete, and errors were not
logged.  Corrupt mailboxes could make it crash.

show_first_tel() and copy_and_expire() can now cope with telegrams of
arbitrary length, like rea(), and sanity-check the header fields they
don't actually use.

include/tel.h

@@ -34,6 +34,7 @@
 #ifndef TEL_H
 #define TEL_H
 
+#include <stdio.h>
 #include <time.h>
 #include "types.h"
 
@@ -55,5 +56,9 @@ struct telstr {
 };
 
 extern char *mailbox(char *buf, natid cn);
+extern int tel_read_header(FILE *, char *, struct telstr *);
+extern int tel_read_body(FILE *, char *, struct telstr *,
+			 int (*sink)(char *, size_t, void *),
+			 void *);
 
 #endif

src/lib/commands/rea.c

@@ -43,6 +43,8 @@
 #include "optlist.h"
 #include "tel.h"
 
+static int print_sink(char *, size_t, void *);
+
 int
 rea(void)
 {
@@ -57,23 +59,20 @@ rea(void)
     FILE *telfp;
     int teles;
     int size;
-    unsigned nbytes;
     char buf[1024];
-    char msgbuf[4096];		/* UTF-8 */
     int lasttype;
     int lastcnum;
     time_t lastdate;
     int header;
     int filelen;
     char *kind;
-    int n;
+    int n, res;
     int num = player->cnum;
     struct natstr *np = getnatp(player->cnum);
     time_t now;
     time_t then;
     time_t delta;
     int first = 1;
-    int readit;
     int may_delete = 1; /* may messages be deleted? */
 
     now = time(NULL);
@@ -112,21 +111,14 @@ rea(void)
     lastdate = 0;
     lastcnum = -1;
     lasttype = -1;
-    while (fread(&tgm, sizeof(tgm), 1, telfp) == 1) {
-	readit = 1;
-	if (tgm.tel_type > TEL_LAST) {
-	    pr("Bad telegram header.  Skipping telegram...\n");
-	    readit = 0;
-	    goto skip;
-	}
+    while ((res = tel_read_header(telfp, mbox, &tgm)) > 0) {
 	if (*kind == 'a') {
-	    if (!player->god && (getrejects(tgm.tel_from, np) & REJ_ANNO)) {
-		readit = 0;
-		goto skip;
-	    }
-	    if (tgm.tel_date < then) {
-		readit = 0;
-		goto skip;
+	    if ((!player->god && (getrejects(tgm.tel_from, np) & REJ_ANNO))
+		|| tgm.tel_date < then) {
+		res = tel_read_body(telfp, mbox, &tgm, NULL, NULL);
+		if (res < 0)
+		    break;
+		continue;
 	    }
 	}
 	if (first && *kind == 'a') {
@@ -151,18 +143,13 @@ rea(void)
 	    lastdate = tgm.tel_date;
 	}
 	teles++;
-      skip:
-	while (tgm.tel_length > 0) {
-	    nbytes = tgm.tel_length;
-	    if (nbytes > sizeof(msgbuf) - 1)
-		nbytes = sizeof(msgbuf) - 1;
-	    fread(msgbuf, 1, nbytes, telfp);
-	    msgbuf[nbytes] = 0;
-	    if (readit)
-		uprnf(msgbuf);
-	    tgm.tel_length -= nbytes;
-	}
+	res = tel_read_body(telfp, mbox, &tgm, print_sink, NULL);
+	if (res < 0)
+	    break;
     }
+    if (res < 0)
+	pr("\n> Mailbox corrupt, tell the deity.\n");
+
     if (teles > 0 && player->cnum == num && may_delete) {
 	pr("\n");
 	if (teles == 1) {
@@ -212,3 +199,10 @@ rea(void)
     }
     return RET_OK;
 }
+
+static int
+print_sink(char *chunk, size_t sz, void *arg)
+{
+    uprnf(chunk);
+    return 0;
+}

src/lib/common/mailbox.c

@@ -36,6 +36,7 @@
 #include <stdio.h>
 #include "optlist.h"
 #include "tel.h"
+#include "prototypes.h"
 
 char *
 mailbox(char *buf, natid cn)
@@ -43,3 +44,57 @@ mailbox(char *buf, natid cn)
     sprintf(buf, "%s/tel%d", teldir, cn);
     return buf;
 }
+
+/*
+ * Read telegram header from FP into TEL.
+ * MBOX is the file name, it is used for logging errors.
+ * Return 1 on success, 0 on EOF, -1 on error.
+ */
+int
+tel_read_header(FILE *fp, char *mbox, struct telstr *tel)
+{
+    size_t n;
+
+    n = fread(tel, 1, sizeof(*tel), fp);
+    if (n == 0 && feof(fp))
+	return 0;
+    if (n != sizeof(*tel)
+	|| tel->tel_type > TEL_LAST || tel->tel_from > MAXNOC) {
+	logerror("Mailbox %s corrupt: bad header", mbox);
+	return -1;
+    }
+    return 1;
+}
+
+/*
+ * Read telegram body from FP.
+ * MBOX is the file name, it is used for logging errors.
+ * TEL is the header.
+ * Unless SINK is null, it is called like SINK(CHUNK, SZ, ARG) to
+ * consume the body, chunk by chunk.  The chunks are UTF-8, and
+ * CHUNK[SZ} is 0.  Reading fails when SINK() returns a negative
+ * value.
+ * Return 0 on success, -1 on failure.
+ */
+int
+tel_read_body(FILE *fp, char *mbox, struct telstr *tel,
+	      int (*sink)(char *, size_t, void *),
+	      void *arg)
+{
+    char buf[4096];
+    size_t left, sz;
+
+    left = tel->tel_length;
+    while (left) {
+	sz = MIN(left, sizeof(buf) - 1);
+	if (fread(buf, 1, sz, fp) != sz) {
+	    logerror("Mailbox %s corrupt: can't read body", mbox);
+	    return -1;
+	}
+	buf[sz] = 0;
+	if (sink && sink(buf, sz, arg) < 0)
+	    return -1;
+	left -= sz;
+    }
+    return 0;
+}

src/lib/player/player.c

@@ -53,6 +53,7 @@
 
 static int command(void);
 static int status(void);
+static int print_sink(char *, size_t, void *);
 
 struct player *player;
 
@@ -269,7 +270,6 @@ show_first_tel(char *fname)
 {
     FILE *fp;
     struct telstr tgm;
-    char buf[MAXTELSIZE + 1];	/* UTF-8 */
 
     if ((fp = fopen(fname, "rb")) == NULL) {
 	if (errno == ENOENT)
@@ -279,28 +279,21 @@ show_first_tel(char *fname)
 	    return -1;
 	}
     }
-    if (fread(&tgm, sizeof(tgm), 1, fp) != 1) {
-	logerror("bad header on login message (%s)", fname);
-	fclose(fp);
+    if (tel_read_header(fp, fname, &tgm) < 0)
 	return -1;
-    }
-    if (tgm.tel_length >= sizeof(buf)) {
-	logerror("text length (%u) is too long for login message (motdfil)",
-		 tgm.tel_length);
-	fclose(fp);
+    if (tel_read_body(fp, fname, &tgm, print_sink, NULL) < 0)
 	return -1;
-    }
-    if (fread(buf, tgm.tel_length, 1, fp) != 1) {
-	logerror("bad length %u on login message", tgm.tel_length);
-	fclose(fp);
-	return -1;
-    }
-    buf[tgm.tel_length] = 0;
-    uprnf(buf);
     fclose(fp);
     return 0;
 }
 
+static int
+print_sink(char *chunk, size_t sz, void *arg)
+{
+    uprnf(chunk);
+    return 0;
+}
+
 int
 quit(void)
 {

src/lib/update/anno.c

@@ -46,6 +46,7 @@
 
 static int copy_and_expire(FILE *annfp, FILE *tmpfp,
 			   char *tmp_filename, time_t expiry_time);
+static int copy_sink(char *, size_t, void *);
 
 void
 delete_old_announcements(void)
@@ -110,32 +111,18 @@ copy_and_expire(FILE *annfp, FILE *tmpfp, char *tmp_filename,
 		time_t expiry_time)
 {
     struct telstr tgm;
-    int writeit;
-    char message[MAXTELSIZE];	/* UTF-8 */
+    int res, writeit;
     int deleted = 0;
     int saved = 0;
     int first = 1;
 
-    while (fread(&tgm, sizeof(tgm), 1, annfp) == 1) {
-	writeit = 1;
-	if (tgm.tel_length > MAXTELSIZE) {
-	    logerror("bad telegram file header (length=%d)",
-		     tgm.tel_length);
-	    return 0;
-	}
-	if (tgm.tel_type > TEL_LAST) {
-	    logerror("bad telegram file header (type=%d)", tgm.tel_type);
-	    return 0;
-	}
-
+    while ((res = tel_read_header(annfp, annfil, &tgm)) > 0) {
+	writeit = tgm.tel_date >= expiry_time;
 	if (first) {
 	    first = 0;
-	    if (tgm.tel_date >= expiry_time)
+	    if (writeit)
 		return 0;
 	}
-	if (tgm.tel_date < expiry_time)
-	    writeit = 0;
-
 	if (writeit) {
 	    if (fwrite(&tgm, sizeof(tgm), 1, tmpfp) != 1) {
 		logerror("error writing header to temporary "
@@ -145,21 +132,25 @@ copy_and_expire(FILE *annfp, FILE *tmpfp, char *tmp_filename,
 	    ++saved;
 	} else
 	    ++deleted;
-	if (fread(message, 1, tgm.tel_length, annfp) != tgm.tel_length) {
-	    logerror("error reading body from telegram file %s",
-		     annfil);
+	res = tel_read_body(annfp, annfil, &tgm,
+			    writeit ? copy_sink : NULL, tmpfp);
+	if (res < 0)
 	    return 0;
-	}
-	if (writeit) {
-	    if (fwrite(message, 1, tgm.tel_length, tmpfp)
-		!= tgm.tel_length) {
-		logerror("error writing body to temporary telegram "
-			 "file %s", tmp_filename);
-		return 0;
-	    }
-	}
     }
+
+    if (res < 0)
+	return 0;
     logerror("%d announcements deleted; %d announcements saved",
 	     deleted, saved);
     return 1;
 }
+
+static int
+copy_sink(char *chunk, size_t sz, void *fp)
+{
+    if (fwrite(chunk, 1, sz, fp) != sz) {
+	logerror("error writing to %s.tmp", annfil);
+	return -1;
+    }
+    return 0;
+}