True, I was led astray by the comment that seemed to indicate it was all about headers. if the intention of the comment is correct, then it should probably if ( $in_header and ... ), whether or not the new regex is added. That block would also need to be moved to after the block that sets $in_header = 0 if we've reached the end, to avoid catching the blank line that separates headers and newlines.
At any rate, Chris has a point -- I was actually thinking the other day that code like that original deferral would be better of sitting in a plugin, so I could muck around with it in a more straightforward manner in order to, say, log what we did to our own database, or change it to a rejection, or whatever :) Anyway, this is all kind of minutia compared to the original patch, which I feel I should remind everyone has to do with accepting legitimate mail from Lotus software without inadvertently stripping the headers :) -Jared > That line of code doesn't look at the headers though, just at the final > dot at the end-of-data. > >> ------------------------------------------------------------------------ >> >> Jared Johnson <mailto:jjohn...@efolder.net> >> August 16, 2011 3:00 PM >> >> >> There's already a special case for something similar to this: >> >> # Reject messages that have either bare LF or CR. rjkaes noticed a >> # lot of spam that is malformed in the header. >> >> ($_ eq ".\n" or $_ eq ".\r") >> and $self->respond(421, "See >> http://smtpd.develooper.com/barelf.html";) >> and return $self->disconnect; >> >> you could just make it ( /\r\r\n$/ or $_ eq ".\n" or $_ eq ".\r" ) >> maybe? >> and update the link to point to a URL that explains both? >> >> -Jared >> >> >> >> ------------------------------------------------------------------------ >> >> Matt Sergeant <mailto:m...@sergeant.org> >> August 16, 2011 11:28 AM >> >> >> Yup there's a lot of this going around right now. Just to be explicit >> though, the header lines end in \r\r\n. Worth rejecting the bloody >> lot, frankly :) >> >> ------------------------------------------------------------------------ >> >> Chris Lewis <mailto:cle...@nortel.com> >> August 15, 2011 4:21 PM >> >> >> >> >> As a FYI, I've been seeing bot-emitted spam that appears to have extra >> \r at the end of _all_ header lines, and the qpsmtpd parser seems to >> be treating all of it as part of the _body_. IOW: except for the >> received line inserted by qpsmtpd, qpsmtpd doesn't see _any_ headers. >> >> This implementation is backrev (0.80 I think), and as it's only spam >> from one particular bot, we don't care about that particular wierdness >> enough to investigate further. But it's worth being aware of. >> >> ------------------------------------------------------------------------ >> >> Jared Johnson <mailto:jjohn...@efolder.net> >> August 15, 2011 3:39 PM >> >> >> Hi, >> >> We got a bug report from someone using IBM's Lotus suite (I think for >> both >> their MUA and MTA). Their users would often send messages where all the >> content was in the subject and they didn't bother sending any message >> content. I'm not sure if it's due to an apparently uncommon behavior for >> their particular MUA or their MTA, but every one of these messages was >> coming through with data in a form that looked like this: >> >> "Subject: howdy\r\n.\r\n" >> >> Rather than including the blank line that one might expect to follow >> headers, since it's required in the event that a message body is >> present: >> >> "Subject: howdy\r\n\r\n.\r\n" >> >> The customer reported these messages were having their subjects >> stripped; >> additional testing indicted all existing headers were being stripped. It >> looks like this is because the loop that processes message data in >> Qpsmtpd::SMTP::data_respond() and creates a Mail::Header object which is >> later used to write out the header in delivery, only works if a blank >> line >> exists after the header, e.g. the second form above. The following is >> all >> I could find in RFC 5322 that elaborated on this blank line, which >> obviously must exist if a message body is included: >> >> "A message consists of header fields (collectively called "the header >> section of the message") followed, optionally, by a body. The header >> section is a sequence of lines of characters with special syntax as >> defined in this specification. The body is simply a sequence of >> characters that follows the header section and is separated from the >> header section by an empty line (i.e., a line with nothing preceding the >> CRLF)." >> >> I read this as implicitly allowing the exclusion of this blank line if >> there is no message body: the specification for the blank line is only >> mentioned in the description of the body, which is itself described as >> optional. Considering we haven't run into this bug in years of usage, I >> assume it's unconventional to exclude the blank line, but it looks like >> it >> is legitimate syntax. >> >> At any rate this was effecting multiple legitimate end users so we put >> together the attached patch, which pulls the header building into its >> own >> sub which is then called inside the loop if we reach the blank line >> indicating the header section is complete; otherwise, it's called >> outside >> of the loop if we have no more message data, indicating the header >> section >> is complete. Sorry I'm not putting this on a github fork, I still don't >> have my git stuff together, I may never get around to it but I thought >> you >> guys might find this useful. >> >> -Jared >
