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.

diff --git a/include/tel.h b/include/tel.h
index aa31c3c874db58127504f3c13e59ca27e01d4042..0df505c78c139020d1c088601ae65bceb36e0ce9 100644 (file)
--- a/include/tel.h
+++ b/include/tel.h
@@ -34,6 +34,7 @@
 #ifndef TEL_H
 #define TEL_H
 
 #ifndef TEL_H
 #define TEL_H
 

+#include <stdio.h>

 #include <time.h>
 #include "types.h"
 
 #include <time.h>
 #include "types.h"
 


@@ -55,5 +56,9 @@ struct telstr {
 };
 
 extern char *mailbox(char *buf, natid cn);
 };
 
 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
 
 #endif

diff --git a/src/lib/commands/rea.c b/src/lib/commands/rea.c
index b53c8ba92ce28e8e5e62cf70ed7a2318e1cf3bd0..cf58ed9905e0e497264e2aa08fd2db10f60068aa 100644 (file)
--- a/src/lib/commands/rea.c
+++ b/src/lib/commands/rea.c
@@ -43,6 +43,8 @@
 #include "optlist.h"
 #include "tel.h"
 
 #include "optlist.h"
 #include "tel.h"
 

+static int print_sink(char *, size_t, void *);
+

 int
 rea(void)
 {
 int
 rea(void)
 {


@@ -57,23 +59,20 @@ rea(void)
     FILE *telfp;
     int teles;
     int size;
     FILE *telfp;
     int teles;
     int size;

-    unsigned nbytes;

     char buf[1024];
     char buf[1024];

-    char msgbuf[4096];         /* UTF-8 */

     int lasttype;
     int lastcnum;
     time_t lastdate;
     int header;
     int filelen;
     char *kind;
     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 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);
     int may_delete = 1; /* may messages be deleted? */
 
     now = time(NULL);


@@ -112,21 +111,14 @@ rea(void)
     lastdate = 0;
     lastcnum = -1;
     lasttype = -1;
     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 (*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') {
            }
        }
        if (first && *kind == 'a') {


@@ -151,18 +143,13 @@ rea(void)
            lastdate = tgm.tel_date;
        }
        teles++;
            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) {
     if (teles > 0 && player->cnum == num && may_delete) {
        pr("\n");
        if (teles == 1) {


@@ -212,3 +199,10 @@ rea(void)
     }
     return RET_OK;
 }
     }
     return RET_OK;
 }

+
+static int
+print_sink(char *chunk, size_t sz, void *arg)
+{
+    uprnf(chunk);
+    return 0;
+}

diff --git a/src/lib/common/mailbox.c b/src/lib/common/mailbox.c
index b317a95e18947e08765d960a1ece1ad21cdfe290..f940cad01a64beb8a2b05c80b7d48f494873c32c 100644 (file)
--- a/src/lib/common/mailbox.c
+++ b/src/lib/common/mailbox.c
@@ -36,6 +36,7 @@
 #include <stdio.h>
 #include "optlist.h"
 #include "tel.h"
 #include <stdio.h>
 #include "optlist.h"
 #include "tel.h"

+#include "prototypes.h"

 
 char *
 mailbox(char *buf, natid cn)
 
 char *
 mailbox(char *buf, natid cn)


@@ -43,3 +44,57 @@ mailbox(char *buf, natid cn)
     sprintf(buf, "%s/tel%d", teldir, cn);
     return buf;
 }
     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;
+}

diff --git a/src/lib/player/player.c b/src/lib/player/player.c
index 8a95d7dc3fbf36ea00c76416d95cebfbb63d5eb6..3852c1161e3c74cc75a79f9c86081fa81b243e72 100644 (file)
--- a/src/lib/player/player.c
+++ b/src/lib/player/player.c
@@ -53,6 +53,7 @@
 
 static int command(void);
 static int status(void);
 
 static int command(void);
 static int status(void);

+static int print_sink(char *, size_t, void *);

 
 struct player *player;
 
 
 struct player *player;
 


@@ -269,7 +270,6 @@ show_first_tel(char *fname)
 {
     FILE *fp;
     struct telstr tgm;
 {
     FILE *fp;
     struct telstr tgm;

-    char buf[MAXTELSIZE + 1];  /* UTF-8 */

 
     if ((fp = fopen(fname, "rb")) == NULL) {
        if (errno == ENOENT)
 
     if ((fp = fopen(fname, "rb")) == NULL) {
        if (errno == ENOENT)


@@ -279,28 +279,21 @@ show_first_tel(char *fname)
            return -1;
        }
     }
            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;
        return -1;

-    }
-    if (tgm.tel_length >= sizeof(buf)) {
-       logerror("text length (%u) is too long for login message (motdfil)",
-                tgm.tel_length);
-       fclose(fp);
-       return -1;
-    }
-    if (fread(buf, tgm.tel_length, 1, fp) != 1) {
-       logerror("bad length %u on login message", tgm.tel_length);
-       fclose(fp);
+    if (tel_read_body(fp, fname, &tgm, print_sink, NULL) < 0)

        return -1;
        return -1;

-    }
-    buf[tgm.tel_length] = 0;
-    uprnf(buf);

     fclose(fp);
     return 0;
 }
 
     fclose(fp);
     return 0;
 }
 

+static int
+print_sink(char *chunk, size_t sz, void *arg)
+{
+    uprnf(chunk);
+    return 0;
+}
+

 int
 quit(void)
 {
 int
 quit(void)
 {

diff --git a/src/lib/update/anno.c b/src/lib/update/anno.c
index f40c6e5055cb705cec538725bbbdbe73b1eea680..adc765735ccc88c4eda1d372a69fae82fdd1b02d 100644 (file)
--- a/src/lib/update/anno.c
+++ b/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_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)
 
 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;
                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;
 
     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 (first) {
            first = 0;

-           if (tgm.tel_date >= expiry_time)
+           if (writeit)

                return 0;
        }
                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 "
        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;
            ++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;
            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;
 }
     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;
+}