On Thu, 2014-06-19 14:44:30 -0400, Daniel Richard G. wrote: > Well, sSMTP is my dumb forwarder of choice, and as the original > reporter appears to have moved on, and sSMTP has been dropped from > testing due to this bug, and no one else seems to care, it looks like > I'll have to step in. > > Attached is a patch against the original 2.64 source that should address > this bug, and hopefully not break anything. An overview of my changes: > > * Added code to standarise() to drop the trailing 'r' if the line > originally ended with "rn". > > * Added a check to header_parse() that effectively converts an "rn" in > the input into 'n'. > > * Added a conditional so that header_parse() doesn't pass the empty > string to header_save()---a behavior I observed in testing, at the end > of a header block with "rn" line endings. > > * Simplified the last if(in_header) conditional in header_parse(), > because it erroneously assumes that if in_header == True, then c could > have some value other than EOF. (See the condition on the previous > "while" loop, and the lack of any other way to exit said loop.) > > header_parse() will now properly grab a header if fed a message > without a body (i.e. no "nn" ending the header block), although this > code will still drop a header if there is no newline at the end. > > > Christoph, thank you for your excellent analysis, and the test cases. I > made use of them, and with my changes sSMTP appears to do the right > thing. I hope we can still count you as a Debian user, four years on! > > Aníbal, please test this patch and let me know if there are any issues. > If everything looks good, I have two requests: > > 1. Convert all instances of (char)NULL in the source to '0'. The former > is silly, and if you add -Wall -Wextra to CFLAGS, GCC complains about > it voluminously. (I followed that convention in my patch, but only so > that this change could be applied across the board.) > > 2. Integrate all/most of debian/patches/* into the original upstream > source, and bump the version to >= 2.65. It's about time.
Thank you!
I'll review/test/merge your patch this weekend. Hopefully, there will be
a 2.65 release in the next fwew days. :-)
> --- ssmtp-2.64/ssmtp.c.orig 2009-11-23 04:55:11.000000000 -0500
> +++ ssmtp-2.64/ssmtp.c 2014-06-19 01:21:01.907825184 -0400
> @@ -365,6 +365,12 @@
> if((p = strchr(str, '\n'))) {
> *p = (char)NULL;
> *linestart = True;
> +
> + /* If the line ended in "\r\n", then drop the '\r' too */
> + sl = strlen(str);
> + if(sl >= 1 && str[sl - 1] == '\r') {
> + str[sl - 1] = (char)NULL;
> + }
> }
> return(leadingdot);
> }
> @@ -758,6 +764,14 @@
> }
> len++;
>
> + if(l == '\r' && c == '\n') {
> + /* Properly handle input that already has "\r\n"
> + line endings; see https://bugs.debian.org/584162 */
> + l = (len >= 2 ? *(q - 2) : '\n');
> + q--;
> + len--;
> + }
> +
> if(l == '\n') {
> switch(c) {
> case ' ':
> @@ -780,7 +794,9 @@
> if((q = strrchr(p, '\n'))) {
> *q = (char)NULL;
> }
> - header_save(p);
> + if(len > 0) {
> + header_save(p);
> + }
>
> q = p;
> len = 0;
> @@ -790,35 +806,12 @@
>
> l = c;
> }
> - if(in_header) {
> - if(l == '\n') {
> - switch(c) {
> - case ' ':
> - case '\t':
> - /* Must insert '\r' before
> '\n's embedded in header
> - fields otherwise qmail won't
> accept our mail
> - because a bare '\n' violates
> some RFC */
> -
> - *(q - 1) = '\r'; /*
> Replace previous \n with \r */
> - *q++ = '\n'; /*
> Insert \n */
> - len++;
> -
> - break;
> -
> - case '\n':
> - in_header = False;
> -
> - default:
> - *q = (char)NULL;
> - if((q = strrchr(p, '\n'))) {
> - *q = (char)NULL;
> - }
> - header_save(p);
> -
> - q = p;
> - len = 0;
> - }
> + if(in_header && l == '\n') {
> + /* Got EOF while reading the header */
> + if((q = strrchr(p, '\n'))) {
> + *q = (char)NULL;
> }
> + header_save(p);
> }
> (void)free(p);
> }
signature.asc
Description: Digital signature
