Re: qpsmtpd-async: premature end of data problem

[Radu Greab]

Matt Sergeant wrote:
> 
> >> I can't help feeling there's a better way to do it - I hate applying
> >> two
> >> regexps every time a DATA packet comes in... I'll have a think on
> >> it.

One option could be to keep this regexp

  $data =~ s/\r\n\.\r\n(.*)\z/\r\n/ms

and, if it is not matched, then check if the packet ends with CRLF. If
true, then push back to Danga::Client those CRLF, for the next
invocation of the callback. Please check the attached patch for what I'm
thinking at. It needs the Danga::Client change and testing.

> I'm thinking more on the lines of pushing back reads if they're not full 
> lines.

Just thinking aloud: could that be used by malevolent people? Like they
send valid data lines, but they take care to never end a packet with
CRLF. If this looks plausible, then Danga::Client needs to protect
against that attack?


Thanks,
Radu Greab


=== lib/Qpsmtpd/PollServer.pm
==================================================================
--- lib/Qpsmtpd/PollServer.pm   (revision 149)
+++ lib/Qpsmtpd/PollServer.pm   (local)
@@ -16,7 +16,6 @@
     start_time
     cmd_timeout
     conn
-    prev_crlf
     _auth
     _auth_mechanism
     _auth_state
@@ -213,7 +212,6 @@
     $self->{header_lines} = '';
     $self->{data_size} = 0;
     $self->{in_header} = 1;
-    $self->{prev_crlf} = 0;
     $self->{max_size} = ($self->config('databytes'))[0] || 0;
     
     $self->log(LOGDEBUG, "max_size: $self->{max_size} / size: 
$self->{data_size}");
@@ -231,14 +229,15 @@
 
     my $done = 0;
     my $remainder;
-    if ($data =~ s/\r\n\.\r\n(.*)\z/\r\n/ms
-        ||
-        ($self->{prev_crlf} && $data =~ s/^\.\r\n(.*)\z//ms)
-       ) 
-    {
+    if ($data =~ s/\r\n\.\r\n(.*)\z/\r\n/ms) {
         $remainder = $1;
         $done = 1;
     }
+    elsif (substr($data, -2, 2) eq "\r\n") {
+        substr($data, -2, 2) = "";
+        # push back CRLF to Danga::Client
+        # ...
+    }
 
     # add a transaction->blocked check back here when we have line by line 
plugin access...
     unless (($self->{max_size} and $self->{data_size} > $self->{max_size})) {
@@ -278,7 +277,6 @@
 
         $self->transaction->body_write(\$data);
         $self->{data_size} += length $data;
-        $self->{prev_crlf} = $data =~ /\r\n\z/;
     }