A few days ago I reported a problem with using POP3 inc to get
messages with long lines. The problem lead to corruption of email,
and could conceivably cause inc (or any other POP3-using MH program)
to crash.
This patch fixes that problem. I have fixed up the popsbr.c getline()
and multiline() functions so that they handle long lines better.
Short lines are now terminated with \n\0; parts of long lines are
terminated with \0. getline() and multiline() now return the number
of characters copied.
I have had to go through popsbr.c and various other files which use
these functions or other popsbr.c functions in order to make them all
understand the new interface. I hope I have got everything right, but
I can't be sure as there are large parts of NMH which I don't use.
Certainly inc now works OK for me.
Nick Barnes
Director
Ravenbrook Limited
<http://www.ravenbrook.com/>
diff -bru /usr/ports/mail/nmh/work/nmh-1.0.4/h/popsbr.h ./h/popsbr.h
--- /usr/ports/mail/nmh/work/nmh-1.0.4/h/popsbr.h Fri Feb 4 01:32:12 2000
+++ ./h/popsbr.h Thu Jan 31 11:08:28 2002
@@ -25,11 +25,11 @@
int pop_init (char *, char *, char *, int, int, int);
int pop_fd (char *, int, char *, int);
int pop_stat (int *, int *);
-int pop_retr (int, int (*)());
+int pop_retr (int, int (*)(const char *, int));
int pop_dele (int);
int pop_noop (void);
int pop_rset (void);
-int pop_top (int, int, int (*)());
+int pop_top (int, int, int (*)(const char *, int));
int pop_quit (void);
int pop_done (void);
@@ -40,7 +40,7 @@
#endif
#ifdef BPOP
-int pop_xtnd (int (*)(), char *, ...);
+int pop_xtnd (int (*)(const char *, int), char *, ...);
#endif
#if defined(MPOP) && !defined(NNTP)
@@ -50,7 +50,7 @@
#if !defined(NNTP) && defined(MPOP)
/* otherwise they are static functions */
int command(const char *, ...);
-int multiline(void);
+int multiline(int);
#endif
/*
diff -bru /usr/ports/mail/nmh/work/nmh-1.0.4/uip/inc.c ./uip/inc.c
--- /usr/ports/mail/nmh/work/nmh-1.0.4/uip/inc.c Fri Mar 17 20:11:04 2000
+++ ./uip/inc.c Thu Jan 31 14:00:34 2002
@@ -153,8 +153,8 @@
#ifdef POP
int done(int);
-static int pop_action(char *);
-static int pop_pack(char *);
+static int pop_action(const char *, int);
+static int pop_pack(const char *, int);
static int map_count(void);
#endif
@@ -909,26 +909,26 @@
}
static int
-pop_action (char *s)
+pop_action (const char *s, int l)
{
- fprintf (pf, "%s\n", s);
- stop += strlen (s) + 1;
+ fprintf (pf, "%s", s);
+ stop += strlen (s);
return 0; /* Is return value used? This was missing before 1999-07-15. */
}
static int
-pop_pack (char *s)
+pop_pack (const char *s, int l)
{
int j;
char buffer[BUFSIZ];
- snprintf (buffer, sizeof(buffer), "%s\n", s);
+ strncpy (buffer, sizeof(buffer), s);
for (j = 0; (j = stringdex (mmdlm1, buffer)) >= 0; buffer[j]++)
continue;
for (j = 0; (j = stringdex (mmdlm2, buffer)) >= 0; buffer[j]++)
continue;
fputs (buffer, pf);
- size += strlen (buffer) + 1;
+ size += strlen (buffer);
return 0; /* Is return value used? This was missing before 1999-07-15. */
}
diff -bru /usr/ports/mail/nmh/work/nmh-1.0.4/uip/msh.c ./uip/msh.c
--- /usr/ports/mail/nmh/work/nmh-1.0.4/uip/msh.c Mon Mar 6 20:19:04 2000
+++ ./uip/msh.c Thu Jan 31 11:15:35 2002
@@ -192,10 +192,10 @@
#ifdef BPOP
# ifdef NNTP
-static int pop_statmsg (char *);
+static int pop_statmsg (const char *, int);
# endif /* NNTP */
static int read_pop (void);
-static int pop_action (char *);
+static int pop_action (const char *, int);
#endif /* BPOP */
static void m_gMsgs (int);
@@ -884,7 +884,7 @@
static int pop_base = 0;
static int
-pop_statmsg (char *s)
+pop_statmsg (const char *s, int l /* line length (unused) */)
{
register int i, n;
@@ -913,9 +913,9 @@
static int
-pop_action (char *s)
+pop_action (char *s, int linelen)
{
- fprintf (yp, "%s\n", s);
+ fprintf (yp, "%s", s);
}
#endif /* BPOP */
diff -bru /usr/ports/mail/nmh/work/nmh-1.0.4/uip/mshcmds.c ./uip/mshcmds.c
--- /usr/ports/mail/nmh/work/nmh-1.0.4/uip/mshcmds.c Fri Feb 4 20:28:24 2000
+++ ./uip/mshcmds.c Thu Jan 31 13:34:00 2002
@@ -2194,15 +2194,14 @@
for (;;) {
int size;
- switch (pop_multiline ()) {
+ switch (pop_multiline (0)) {
case NOTOK:
fprintf (stderr, "%s", response);
- /* and fall... */
- case DONE:
+ break;
+ case 0:
fprintf (stderr,"\n");
break;
-
- case OK:
+ default:
if (sscanf (response, "%d %d", &msgnum, &size) == 2
&& mp->lowmsg <= msgnum
&& msgnum <= mp->hghmsg
diff -bru /usr/ports/mail/nmh/work/nmh-1.0.4/uip/popi.c ./uip/popi.c
--- /usr/ports/mail/nmh/work/nmh-1.0.4/uip/popi.c Fri Feb 4 20:28:24 2000
+++ ./uip/popi.c Thu Jan 31 14:06:01 2002
@@ -84,6 +84,9 @@
*/
int sc_width (void); /* from termsbr.c */
+static int
+retr_action (const char *rsp, int linelen);
+static int retr_action_flag;
int
main (int argc, char **argv)
@@ -347,7 +350,7 @@
if (cp)
*cp = ' ';
pop_command ("%s%s", popicmds[i].sw, cp ? cp : "");
- printf ("%s\n", response);
+ printf ("%s", response);
break;
case LISTCMD:
@@ -355,19 +358,18 @@
*cp = ' ';
if (pop_command ("%s%s", popicmds[i].sw, cp ? cp : "")
== OK) {
- printf ("%s\n", response);
+ printf ("%s", response);
if (!cp)
for (;;) {
- switch (pop_multiline ()) {
- case DONE:
- strcpy (response, ".");
- /* and fall... */
+ switch (pop_multiline (0)) {
+ case 0:
+ printf(".\n");
+ break;
case NOTOK:
- printf ("%s\n", response);
+ printf ("%s", response);
break;
-
- case OK:
- printf ("%s\n", response);
+ default:
+ printf ("%s", response);
continue;
}
break;
@@ -380,10 +382,12 @@
advise (NULL, "missing argument to %s", buffer);
break;
}
- retr_action (NULL, OK);
+ retr_action_flag = OK;
+ retr_action (NULL, 0);
pop_retr (atoi (++cp), retr_action);
- retr_action (NULL, DONE);
- printf ("%s\n", response);
+ retr_action_flag = DONE;
+ retr_action (NULL, 0);
+ printf ("%s", response);
break;
case SCANCMD:
@@ -413,7 +417,7 @@
*fp = '\0';
pop_command ("xtnd scan %d \"%s\"", width, ep);
- printf ("%s\n", response);
+ printf ("%s", response);
free (ep);
}
@@ -430,7 +434,7 @@
static int
-retr_action (char *rsp, int flag)
+retr_action (const char *rsp, int len)
{
static FILE *fp;
@@ -438,7 +442,7 @@
static int msgnum;
static char *cp;
- if (flag == OK) {
+ if (retr_action_flag == OK) {
if (!(mp = folder_realloc (mp, mp->lowoff, msgnum = mp->hghmsg + 1)))
adios (NULL, "unable to allocate folder storage");
@@ -471,7 +475,7 @@
return;
}
- fprintf (fp, "%s\n", rsp);
+ fprintf (fp, "%s", rsp);
}
diff -bru /usr/ports/mail/nmh/work/nmh-1.0.4/uip/popsbr.c ./uip/popsbr.c
--- /usr/ports/mail/nmh/work/nmh-1.0.4/uip/popsbr.c Fri Feb 4 01:32:12 2000
+++ ./uip/popsbr.c Thu Jan 31 14:19:06 2002
@@ -63,10 +63,10 @@
#if defined(NNTP) || !defined(MPOP)
/* otherwise they are not static functions */
static int command(const char *, ...);
-static int multiline(void);
+static int multiline(int more);
#endif
-static int traverse (int (*)(), const char *, ...);
+static int traverse (int (*)(const char *, int), const char *, ...);
static int vcommand(const char *, va_list);
static int getline (char *, int, FILE *);
static int putline (char *, FILE *);
@@ -122,6 +122,7 @@
{
int fd1, fd2;
char buffer[BUFSIZ];
+ int linelen;
#ifdef APOP
int apop;
@@ -170,10 +171,10 @@
SIGNAL (SIGPIPE, SIG_IGN);
- switch (getline (response, sizeof response, input)) {
- case OK:
+ linelen = getline (response, sizeof response, input);
+ if (linelen > 0) { /* some bytes */
if (poprint)
- fprintf (stderr, "<--- %s\n", response);
+ fprintf (stderr, "<--- %s", response);
#ifndef NNTP
if (*response == '+') {
# ifndef KPOP
@@ -183,8 +184,7 @@
if (cp && command ("APOP %s", cp) != NOTOK)
return OK;
- }
- else
+ } else
# endif /* APOP */
if (command ("USER %s", user) != NOTOK
&& command ("%s %s", rpop ? "RPOP" : (pophack++, "PASS"),
@@ -206,17 +206,13 @@
command ("QUIT");
strncpy (response, buffer, sizeof(response));
/* and fall */
+ }
- case NOTOK:
- case DONE:
if (poprint)
fprintf (stderr, "%s\n", response);
fclose (input);
fclose (output);
return NOTOK;
- }
-
- return NOTOK; /* NOTREACHED */
}
#ifdef NNTP
@@ -303,7 +299,7 @@
#ifdef NNTP
int
-pop_exists (int (*action)())
+pop_exists (int (*action)(const char *, int))
{
#ifdef XMSGS /* hacked into NNTP 1.5 */
if (traverse (action, "XMSGS %d-%d", (targ_t) xtnd_first, (targ_t) xtnd_last) ==
OK)
@@ -326,6 +322,7 @@
#endif
{
int i;
+ int linelen;
#ifndef BPOP
int *ids = NULL;
#endif
@@ -356,14 +353,15 @@
if (command ("LIST") == NOTOK)
return NOTOK;
- for (i = 0; i < *nmsgs; i++)
- switch (multiline ()) {
- case NOTOK:
+ for (i = 0; i < *nmsgs; i++) {
+ /* LIST lines must be smaller than our buffer */
+ switch(multiline (0)) {
+ case NOTOK: /* EOF or I/O error */
return NOTOK;
- case DONE:
+ case 0: /* end of multi-line */
*nmsgs = ++i;
return OK;
- case OK:
+ default:
*msgs = *bytes = 0;
if (ids) {
*ids = 0;
@@ -374,13 +372,14 @@
sscanf (response, "%d %d", msgs++, bytes++);
break;
}
+ }
for (;;)
- switch (multiline ()) {
- case NOTOK:
+ switch (multiline (0)) {
+ case NOTOK: /* EOF or I/O error */
return NOTOK;
- case DONE:
+ case 0: /* end of multi-line */
return OK;
- case OK:
+ default: /* a multi-line line */
break;
}
#else /* NNTP */
@@ -390,7 +389,7 @@
int
-pop_retr (int msgno, int (*action)())
+pop_retr (int msgno, int (*action)(const char *, int))
{
#ifndef NNTP
return traverse (action, "RETR %d", (targ_t) msgno);
@@ -401,11 +400,13 @@
static int
-traverse (int (*action)(), const char *fmt, ...)
+traverse (int (*action)(const char *, int), const char *fmt, ...)
{
int result;
va_list ap;
char buffer[sizeof(response)];
+ int linelen;
+ int more = 0;
va_start(ap, fmt);
result = vcommand (fmt, ap);
@@ -415,19 +416,23 @@
return NOTOK;
strncpy (buffer, response, sizeof(buffer));
- for (;;)
- switch (multiline ()) {
- case NOTOK:
+ for (;;) {
+ linelen = multiline(more);
+ switch(linelen) {
+ case NOTOK: /* EOF or I/O error. */
return NOTOK;
- case DONE:
+ case 0: /* end of multi-line */
strncpy (response, buffer, sizeof(response));
return OK;
- case OK:
- (*action) (response);
+ default: /* some bytes */
+ /* does this include the end of a line? */
+ more = (response[linelen-1] != '\n');
+ (*action) (response, linelen);
break;
}
+ }
}
@@ -462,7 +467,7 @@
int
-pop_top (int msgno, int lines, int (*action)())
+pop_top (int msgno, int lines, int (*action)(const char *, int))
{
#ifndef NNTP
return traverse (action, "TOP %d %d", (targ_t) msgno, (targ_t) lines);
@@ -474,7 +479,7 @@
#ifdef BPOP
int
-pop_xtnd (int (*action)(), char *fmt, ...)
+pop_xtnd (int (*action)(const char *, int), char *fmt, ...)
{
int result;
va_list ap;
@@ -521,7 +526,7 @@
xtnd_first = atoi (ap[2]);
xtnd_last = atoi (ap[3]);
- (*action) (buffer);
+ (*action) (buffer, strlen(buffer));
return OK;
} else { /* XTND "BBOARDS" */
@@ -577,6 +582,7 @@
vcommand (const char *fmt, va_list ap)
{
char *cp, buffer[BUFSIZ];
+ int linelen;
vsnprintf (buffer, sizeof(buffer), fmt, ap);
if (poprint) {
@@ -595,81 +601,113 @@
if (putline (buffer, output) == NOTOK)
return NOTOK;
- switch (getline (response, sizeof response, input)) {
- case OK:
+ linelen = getline (response, sizeof response, input);
+ if (linelen > 0) {
if (poprint)
- fprintf (stderr, "<--- %s\n", response);
+ fprintf (stderr, "<--- %s", response);
#ifndef NNTP
return (*response == '+' ? OK : NOTOK);
#else /* NNTP */
return (*response < CHAR_ERR ? OK : NOTOK);
#endif /* NNTP */
-
- case NOTOK:
- case DONE:
+ }
if (poprint)
- fprintf (stderr, "%s\n", response);
+ fprintf (stderr, "%s", response);
return NOTOK;
- }
-
- return NOTOK; /* NOTREACHED */
}
+/* multiline(more)
+ *
+ * Handle multi-line responses (RFC1939, section 3). A line which is
+ * a lone '.' ends the response. Any other line which begins with '.'
+ * is "byte-stuffed" by prepending another '.'.
+ *
+ * If the 'more' argument is non-zero, we are getting more of a
+ * part-read line, so the '.' handling does not apply.
+ *
+ * On I/O error or EOF, return NOTOK.
+ * If we have reached the end of the response, return 0.
+ * Otherwise, return the number of bytes.
+ */
+
#if defined(MPOP) && !defined(NNTP)
int
-multiline (void)
+multiline (int more)
#else
static int
-multiline (void)
+multiline (int more)
#endif
{
- char buffer[BUFSIZ + TRMLEN];
-
- if (getline (buffer, sizeof buffer, input) != OK)
+ char buffer[BUFSIZ];
+ int linelen = getline (buffer, sizeof buffer, input);
+ if (linelen <= 0)
return NOTOK;
#ifdef DEBUG
if (poprint)
- fprintf (stderr, "<--- %s\n", response);
+ fprintf (stderr, "<--- %s", buffer);
#endif DEBUG
- if (strncmp (buffer, TRM, TRMLEN) == 0) {
- if (buffer[TRMLEN] == 0)
- return DONE;
- else
- strncpy (response, buffer + TRMLEN, sizeof(response));
+ if ((!more) &&
+ (strncmp (buffer, TRM, TRMLEN) == 0)) { /* leading '.' */
+ if (buffer[TRMLEN] == '\n') { /* lone '.': end of multi-line */
+ return 0;
+ } else {
+ strncpy (response, buffer+TRMLEN, sizeof(response));
+ return (linelen - TRMLEN);
+ }
}
- else
strncpy (response, buffer, sizeof(response));
-
- return OK;
+ return linelen;
}
+/* getline(buf, bufsize, file)
+ * get a CRLF terminated line, and return it with \n in place of CRLF.
+ * If we don't reach a terminating CRLF, this will return as much as
+ * it can. NUL-terminate the result.
+ *
+ * Return value is the number of data bytes, including any \n, not
+ * including the NUL.
+ *
+ * Return NOTOK if we get a connection error.
+ * Return 0 if there are no bytes (EOF).
+ *
+ * SMTP says that all lines are terminated with CRLF and CR and LF do
+ * not appear bare. So we can just discard all CRs. This avoids the problem
+ * of a CRLF being split across buffers. */
static int
getline (char *s, int n, FILE *iop)
{
int c;
char *p;
+ char *limit = s+n-1;
p = s;
- while (--n > 0 && (c = fgetc (iop)) != EOF)
- if ((*p++ = c) == '\n')
+ for (;;) {
+ if (p == limit)
+ break;
+ while ((c = fgetc(iop)) == '\r') /* drop CRs */
+ ;
+ if (c == EOF)
break;
+ *p++ = c;
+ if (c == '\n')
+ break;
+ }
+
if (ferror (iop) && c != EOF) {
- strncpy (response, "error on connection", sizeof(response));
+ strncpy (response, "error on connection\n", sizeof(response));
return NOTOK;
}
if (c == EOF && p == s) {
- strncpy (response, "connection closed by foreign host", sizeof(response));
- return DONE;
+ strncpy (response,
+ "connection closed by foreign host\n", sizeof(response));
}
- *p = 0;
- if (*--p == '\n')
- *p = 0;
- if (*--p == '\r')
+
+ /* add terminating NUL */
*p = 0;
- return OK;
+ return p-s;
}
