Thus said Ken Hornstein on Wed, 16 Nov 2022 09:19:11 -0500:
> I think the correct solution is when nmh is doing a RETR inside of
> POP we should use netsec_read() and perform the necessary CR-LF
> translations on the returned buffer. The only complicated thing I see
> is dealing with the case where a CR is at the end of one buffer and
> the LF is at the start of the next buffer; I think that is
> straightforward.
I have a working prototype that I'm testing that uses netsec_read(). I
had to expand the function declaration for netsec_read() because it
didn't return the number of bytes that the caller received. Not sure if
that was intentional and I'm just misunderstanding netsec_read().
I pretty much just stuffed all the logic into traverse() for now just to
simplify the flow, though, I imagine it's still more complicated than I
should like. I tested it with 5 emails that I have received that were
known to cause problems, and have been receiving other "normal" emails
since I put it into production.
The patch is currently against 1.7.1 since that is what I run on my
system, so I'll rework it for master when I get more free time. I've
attached the patch so others can review for correctness, though, as I
said, it's for 1.7.1 so it won't apply cleanly to master yet.
Thanks,
Andy
Index: h/netsec.h
==================================================================
--- h/netsec.h
+++ h/netsec.h
@@ -164,12 +164,12 @@
* errstr - Error size
*
* Returns number of bytes read, or -1 on error.
*/
-ssize_t netsec_read(netsec_context *ns_context, void *buffer, size_t size,
- char **errstr);
+ssize_t netsec_read(netsec_context *ns_context, char *buffer, size_t size,
+ size_t *retlen, char **errstr);
/*
* Write data to the network; if encryption is being performed, we will
* do it. Data may be buffered; use netsec_flush() to flush any outstanding
* data to the network.
Index: sbr/netsec.c
==================================================================
--- sbr/netsec.c
+++ sbr/netsec.c
@@ -372,14 +372,13 @@
* Read data from the network. Basically, return anything in our buffer,
* otherwise fill from the network.
*/
ssize_t
-netsec_read(netsec_context *nsc, void *buffer, size_t size, char **errstr)
+netsec_read(netsec_context *nsc, char *buffer, size_t size,
+ size_t *retlen, char **errstr)
{
- int retlen;
-
/*
* If our buffer is empty, then we should fill it now
*/
if (nsc->ns_inbuflen == 0) {
@@ -390,15 +389,15 @@
/*
* netsec_fillread only returns if the buffer is full, so we can
* assume here that this has something in it.
*/
- retlen = min(size, nsc->ns_inbuflen);
+ *retlen = min(size, nsc->ns_inbuflen);
- memcpy(buffer, nsc->ns_inptr, retlen);
+ memcpy(buffer, nsc->ns_inptr, *retlen);
- if (retlen == (int) nsc->ns_inbuflen) {
+ if (*retlen == nsc->ns_inbuflen) {
/*
* We've emptied our buffer, so reset everything.
*/
nsc->ns_inptr = nsc->ns_inbuffer;
nsc->ns_inbuflen = 0;
Index: uip/inc.c
==================================================================
--- uip/inc.c
+++ uip/inc.c
@@ -934,9 +934,9 @@
}
static int
pop_action (char *s)
{
- 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. */
}
Index: uip/popsbr.c
==================================================================
--- uip/popsbr.c
+++ uip/popsbr.c
@@ -489,12 +489,17 @@
static int
traverse (int (*action)(char *), const char *fmt, ...)
{
int result, snoopstate;
+ size_t len, unused, inoffset, slen, count;
va_list ap;
char buffer[sizeof(response)];
+ size_t buflen;
+ char *errstr, *ptr, *sptr, *tptr;
+ boolean checkline = true;
+ boolean needdata = true;
va_start(ap, fmt);
result = vcommand (fmt, ap);
va_end(ap);
@@ -503,25 +508,94 @@
strncpy (buffer, response, sizeof(buffer));
if ((snoopstate = netsec_get_snoop(nsc)))
netsec_set_snoop(nsc, 0);
- for (;;)
- switch (multiline ()) {
- case NOTOK:
+ unused = sizeof(buffer);
+ inoffset = 0;
+ buflen = 0;
+ for (;;) {
+ if (needdata) {
+ result = netsec_read(nsc, buffer + inoffset, unused, &len, &errstr);
+ if (result != OK) {
+ strncpy(response, errstr, sizeof(response));
+ response[sizeof(response) - 1] = '\0';
+ free(errstr);
netsec_set_snoop(nsc, snoopstate);
return NOTOK;
-
- case DONE:
- strncpy (response, buffer, sizeof(response));
- netsec_set_snoop(nsc, snoopstate);
+ }
+ tptr = buffer;
+ buflen += len;
+ needdata = false;
+ }
+ if (checkline) {
+ if (buflen >= LEN(TRM) + 2 &&
+ strncmp(TRM "\r\n", tptr, LEN(TRM) + 2) == 0)
return OK;
-
- case OK:
+ if (buflen >= LEN(TRM TRM) &&
+ strncmp(TRM TRM, tptr, LEN(TRM TRM)) == 0) {
+ tptr += LEN(TRM);
+ buflen -= LEN(TRM);
+ }
+ }
+ ptr = tptr;
+ checkline = false;
+ count = 0;
+ while (count < buflen) {
+ count++;
+ if (*ptr++ == '\n') {
+ sptr = tptr;
+ if (count > 1 && *(ptr - 2) == '\r') {
+ *--ptr = '\0';
+ checkline = true;
+ }
+ *--ptr = '\n'; ptr++;
+ slen = ptr - tptr;
+ strncpy(response, sptr, slen);
+ response[slen] = '\0';
+ buflen -= count;
+ tptr += count;
(*action) (response);
break;
+ }
+ }
+ if (checkline) {
+ if (buflen > 0) {
+ if (buflen < LEN(TRM) + 2 || buflen < LEN(TRM TRM)) {
+ needdata = true;
+ inoffset = buflen;
+ unused = sizeof(buffer) - buflen;
+ memmove(buffer, tptr, buflen);
+ }
+ } else {
+ needdata = true;
+ inoffset = 0;
+ unused = sizeof(buffer);
+ }
+ continue;
+ }
+ if (count == buflen) {
+ sptr = tptr;
+ if (buflen > 1 && tptr[buflen - 1] == '\r') {
+ ptr--;
+ }
+ slen = ptr - tptr;
+ strncpy(response, sptr, slen);
+ response[slen] = '\0';
+ buflen -= slen;
+ (*action) (response);
+ if (buflen > 0) {
+ inoffset = buflen;
+ unused = sizeof(buffer) - buflen;
+ memmove(buffer, buffer + slen, buflen);
+ } else {
+ inoffset = 0;
+ unused = sizeof(buffer);
+ }
+ needdata = true;
}
+ }
}
int
pop_dele (int msgno)
