Re: qpsmtpd-async: premature end of data problem

[Radu Greab]

Matt Sergeant wrote:
> 
> I'm thinking more on the lines of pushing back reads if they're not full 
> lines.
> 
> Probably needs a bit of support for that in lib/Danga/Client.pm

Attached is an attempt to do that. The reason for this is that I've seen
another case of end of data marker coming in two different packets and
the previous fix does not work: one packet ends with CRLF. , the
following packet starts with CRLF. A test script is attached too.

This new attempt reverts all previous changes to Qpsmtpd::PollServer and
changes Danga::Client to send to the callback full lines: either the
entire packet if it ends with CRLF or the part up to the last CRLF,
inclusive. What is left, a possible partial line, is buffered and
processed when the next packet comes in.


Thanks,
Radu Greab


=== lib/Qpsmtpd/PollServer.pm
==================================================================
--- lib/Qpsmtpd/PollServer.pm   (revision 154)
+++ lib/Qpsmtpd/PollServer.pm   (revision 155)
@@ -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,18 +229,13 @@
 
     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(.*)\z//ms) {
         $remainder = $1;
         $done = 1;
     }
 
     # 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})) {
-        $self->{prev_crlf} = $data =~ /\r\n\z/;
         $data =~ s/\r\n/\n/mg;
         $data =~ s/^\.\./\./mg;
         
=== lib/Danga/Client.pm
==================================================================
--- lib/Danga/Client.pm (revision 154)
+++ lib/Danga/Client.pm (revision 155)
@@ -54,6 +54,26 @@
     $self->{callback} = $callback;
 }
 
+sub process_chunk {
+    my Danga::Client $self = shift;
+    my $callback = shift;
+
+    my $last_crlf = rindex($self->{line}, "\r\n");
+
+    if ($last_crlf != -1) {
+        if ($last_crlf + 2 == length($self->{line})) {
+            my $data = $self->{line};
+            $self->{line} = '';
+            $callback->($data);
+        }
+        else {
+            my $data = substr($self->{line}, 0, $last_crlf);
+            $self->{line} = substr($self->{line}, $last_crlf + 1);
+            $callback->($data);
+        }
+    }
+}
+
 sub get_chunks {
     my Danga::Client $self = shift;
     my ($bytes, $callback) = @_;
@@ -61,8 +81,7 @@
         die "get_bytes/get_chunks currently in progress!";
     }
     $self->{read_bytes} = $bytes;
-    $callback->($self->{line}) if length($self->{line});
-    $self->{line} = '';
+    $self->process_chunk($callback) if length($self->{line});
     $self->{callback} = $callback;
     $self->{get_chunks} = 1;
 }
@@ -84,7 +103,8 @@
         if ($self->{get_chunks}) {
             my $bref = $self->read($self->{read_bytes});
             return $self->close($!) unless defined $bref;
-            $self->{callback}->($$bref) if length($$bref);
+            $self->{line} .= $$bref;
+            $self->process_chunk($self->{callback}) if length($self->{line});
             return;
         }
         if ($self->{read_bytes} > 0) {

#!/usr/bin/perl -w

use strict;
use Net::SMTP;

my $smtp = Net::SMTP->new('localhost:25', Debug => 1);
$smtp->mail('');
$smtp->to('[EMAIL PROTECTED]');
$smtp->data;

$smtp->rawdatasend("Subject: test\r\n\r\nOne chunk\r\n.");

sleep 1;  # let qpsmtpd get the previous data as a packet

$smtp->rawdatasend("\r\n");

print "server responds: ", $smtp->response, "\n";

$smtp->rawdatasend("QUIT\r\n");