| Subject: | message_string() mismatched size handling too strict and verbose |
| Date: | Tue, 03 Feb 2009 02:33:41 -0500 |
| To: | bug-Mail-IMAPClient [...] rt.cpan.org |
| From: | Phil Lobbes <phil [...] perkpartners.com> |
Using Mail::IMAPClient v3.13 (and earlier versions) against an IMAP
server where RFC822.SIZE does not match the size of LITERAL size via
BODY.PEEK some problems can arise.
Here's data from a case I ran into:
Sending: 19 UID FETCH 4 (RFC822.SIZE)
Sent 30 bytes
Read: * 4 FETCH (UID 4 RFC822.SIZE 75078)
...
Sending: 20 UID FETCH 4 BODY.PEEK[]
Sent 28 bytes
LITERAL: received literal in line * 4 FETCH (UID 4 BODY[] of length
75076; attempting to retrieve from the 2863 bytes in...
This generates these (duplicate) ERROR's:
ERROR: message_string() expected 75078 bytes but received 75076
ERROR: message_string() expected 75078 bytes but received 75076
And these warnings:
* Use of uninitialized value in substitution (s///) at
.../lib/Mail/IMAPClient.pm line 2338.
* Use of uninitialized value in length at
.../lib/Mail/IMAPClient.pm line 2340.
* Use of uninitialized value in concatenation (.) or string at
.../lib/Mail/IMAPClient.pm line 2345.
- These are the lines in question from append_string():
2337 my $count = $self->Count($self->Count+1);
2338 $text =~ s/\r?\n/\r\n/g;
2339
2340 my $command = "$count APPEND $folder " . ($flags ? "$flags " : "") .
2341 ($date ? "$date " : "") . "{" . length($text) . "}\r\n";
2342
2343 $self->_record($count,[$self->_next_index, "INPUT", $command]);
2344
2345 my $bytes = $self->_send_line($command.$text."\r\n");
2346 unless(defined $bytes)
Beyond the undefined warnings, I believe we need to decide what is the
correct behavior. Currently we have this...
sub message_string
{ my ($self, $msg) = @_;
my $expected_size = $self->size($msg);
[ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Note: this is RFC822.SIZE! ]
...
$self->fetch($msg, $cmd)
or return undef;
my $string = $self->_transaction_literals;
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
unless($self->Ignoresizeerrors)
{ # Should this return undef if length != expected?
# now, attempts are made to salvage parts of the message.
if( length($string) != $expected_size )
....
I believe our current behavior of message_string is potentially overly
strict. RFC2683 Section 3.4.5 "RFC822.SIZE" seems to address this
issue. There's a bit to it, but I believe the most relevant statement
is:
" ... a client should use the size that's returned in the literal when
you fetch the data."
Since our code uses _transaction_literals which should (seems to)
properly handle the literal size. This extra check of RFC822.SIZE vs
the length of the literal string should not be necessary, is not
recommended per the RFC, and perhaps more importantly should not be the
default behavior.
To make matters potentially worse, if the RFC822.SIZE is smaller than
the actual message size then the code will truncate a message. This
would probably cause valid data to be lost.
If desired, I'll submit a patch that makes Ignoresizeerrors default to
TRUE, but we the option could still be turned off to be more strict if
desired.
Here is a patch that:
- only records the size mismatch error once
- does NOT truncate LITERALS longer than RFC822.SIZE
- enforces strictness of RFC822.SIZE vs LITERAL length check, so any
mismatch in size (larger or smaller) causes an error and returns undef
- eliminates undefined warnings in append_string on empty $text
Phil
--- IMAPClient.pm.OLD 2009-02-03 02:27:00.706750000 -0500
+++ IMAPClient.pm 2009-02-03 02:28:37.534875000 -0500
@@ -564,20 +564,13 @@
my $string = $self->_transaction_literals;
+ # Size check may be overly strict, see RFC2683 3.4.5 "RFC822.SIZE"
unless($self->Ignoresizeerrors)
{ # Should this return undef if length != expected?
- # now, attempts are made to salvage parts of the message.
+ # do not truncate the message if more data than RFC822.SIZE returned
if( length($string) != $expected_size )
{ $self->LastError("message_string() "
. "expected $expected_size bytes but received ".length($string));
- }
-
- $string = substr $string, 0, $expected_size
- if length($string) > $expected_size;
-
- if( length($string) < $expected_size )
- { $self->LastError("message_string() expected ".
- "$expected_size bytes but received ".length($string));
return undef;
}
}
@@ -2318,6 +2311,9 @@
my $folder = $self->Massage(shift);
my ($text, $flags, $date) = @_;
+ # an empty message will likely fail but we will try anyway
+ $text = "" unless(defined $text);
+
if(defined $flags)
{ $flags =~ s/^\s+//g;
$flags =~ s/\s+$//g;