<!DOCTYPE html PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN">
<html>
<head>
<meta http-equiv="Content-Type" content="text/html;charset=ISO-8859-1">
<title></title>
</head>
<body>
Matthew Reimer wrote:<br>
<blockquote type="cite" cite="midlists.dovecot.3FA2E1C3.30207@vpop.net">
<meta content="text/html;" http-equiv="Content-Type">
<title></title>
There is a bug in partial IMAP fetches where the following happens:<br>
<br>
<table border="1" cellspacing="2" cellpadding="2">
<tbody>
<tr>
<td valign="top">fetch ... <0.10240><br>
</td>
<td valign="top">correctly returns bytes 0-10239<br>
</td>
</tr>
<tr>
<td valign="top">fetch ... <10240.10240><br>
</td>
<td valign="top"><font color="#ff0000">incorrectly returns
bytes
20479...</font><br>
</td>
</tr>
<tr>
<td valign="top">fetch ... <10240.10240> (a 2nd time)<br>
</td>
<td valign="top">correctly returns bytes 10240-20479<br>
</td>
</tr>
</tbody>
</table>
<br>
While the second fetch fails, the third fetch works correctly. I
tracked this down to a problem with partial_cache. There seems to be a
bug in lib-mail/message-send.c:39 (CVS):<br>
<br>
ret = o_stream_send_istream(output, input) > 0;<br>
<br>
This causes message_send() to return 0 or 1 (note the "> 0"), not
the number of bytes sent. This in turn causes partial.pos.virtual_size
to get a value of 1 rather than 10240 in fetch_body(), and this in turn
results in the wrong portion of the message being returned for the
subsequent fetch.<br>
<br>
But then when I remove the "> 0" from the above line of code, I get
off-by-one errors:<br>
<br>
<table cellpadding="2" cellspacing="2" border="1">
<tbody>
<tr>
<td valign="top">fetch ... <0.10240><br>
</td>
<td valign="top">correctly returns bytes 0-10240<br>
</td>
</tr>
<tr>
<td valign="top">fetch ... <10240.10240><br>
</td>
<td valign="top"><font color="#ff0000">incorrectly returns
bytes
10241-20480</font><br>
</td>
</tr>
<tr>
<td valign="top">fetch ... <10240.10240> (a 2nd time)<br>
</td>
<td valign="top">correctly returns bytes 10240-20479<br>
</td>
</tr>
</tbody>
</table>
<br>
I'm guessing there is a bug in the interaction with
message_skip_virtual(), but I haven't been able to find the problem yet.<br>
<br>
Any ideas? I've attached the offending message.<br>
<br>
Matt<br>
</blockquote>
I think I've found the bug. The attached patch fixes the problem for
me. The first problem is that the wrong value is being returned by
message_send() because ret is getting set to the wrong value (see above
or patch). The second problem is that in the special case where
physical_size == virtual_size (i.e., sendfile can be used)
i_stream_skip(input, virtual_skip) is being called on the input stream.
This is wrong because virtual_skip is supposed to indicate that \r was
the last character seen and therefore \n needs to be sent before any of
the input stream; but in the sendfile case this is not necessary.<br>
<br>
I think a little refactoring of seek_partial(), message_send() and
message_skip_virtual() might be helpful to clarify the code. The
virtual_skip argument to message_send() is an off_t but is only used to
indicate that \r was the last character read, so it would probably be
more clear to add another argument cr_skipped (like
message_skip_virtual()). Better might be to put all the partial cache
stuff in message-send.c.<br>
<br>
Matt<br>
</body>
</html>