Hi Steve,

it's been a long time since we hacked on the CURL IMAP support
together :-) I hope you are doing well.

Purely by accident, I found the recent email thread between you and
John Dunn about the problems with fetching some IMAP messages. When I
read your last analysis message [1], I thought there must have been
another, proper way for this to work - I certainly did not remember
any such limitation of the design.
 I've changed my employer in the meantime and I don't use CURL for
IMAP regularly, I don't have any test environment setup and I don't
even have time to try all this stuff out in practice. So I'll give you
my comments just from dry reading the relevant source files in the
repository. Please beware of uneducated guessing and false reasoning.
(Also sorry for not replying to the thread properly, I had not been
subscribed to the list and I could not find the relevant headers
anywhere now.)

[1] http://curl.haxx.se/mail/lib-2013-08/0210.html
____________________

First, let's talk about 'bytecount', 'maxdownload' and the handling of
the received excess data.

From the code in transfer.c, k->maxdownload is apparently supposed to
be the "total number of bytes which should be downloaded".
k->bytecount then goes from 0 (at the start) to k->maxdownload (when
the transfer is finished). So the IMAP code should set
data->req.bytecount to "how many bytes have been already received" and
data->req.maxdownload to the "total size of data to be received".

The first part is OK, 'bytecount' is initialized to 0 at the start of
each request, then possibly incremented in the code which possibly
pulls whatever has been already received from the PP cache.

The second field is set up incorrectly though. If a part of the data
has been already received, the variable 'size' is decremented at
imap.c:1468, so data->req.maxdownload is set to a value less than what
'bytecount' can become, less than the "total number of bytes to
download". So this line "size -= chunk" should IMHO be removed.

(Why not just set 'bytecount' to 0 and 'maxdownload' to the remaining
size? I have the feeling these values are provided to progress bars
and other user feedback stuff, so better set it up so it looks like
everything is being downloaded as usual, as if nothing had been
pre-read into the PP cache buffer etc.)
____________________

OK, that might solve the weird excess messages with negative values,
but how to solve the main problem then? (Which is that extra data is
read when it should stay in the socket till PP code goes to read the
final FETCH response.)

The key seems to be the 3rd parameter to Curl_setup_transfer(). What's
this 'size' for? It's stored into k->size, then used at the beginning
of readwrite_data() in transfer.c. If set to anything else than -1,
the amount of data which will be Curl_read() is trimmed to the maximum
value of 'k->size - k->bytecount'. So if 'k->size' was set right, the
code would never read past the body and the excess data (the final
FETCH response) would stay in the socket buffer. Why doesn't it work
correctly then?

At imap.c:1494, Curl_setup_transfer() is called with size set to
'size', which is the "number of bytes remaining to read". Then, in
transfer.c around line 398, the following thing happens at the end of
the transfer (I'm using values from John's debug log):

  curl_off_t totalleft = k->size - k->bytecount;
      // totalleft = 262 - 1374 = -1112
  if(totalleft < (curl_off_t)bytestoread) {
      // -1112 < BUFSIZE, signed comparison, surely matches
    bytestoread = (size_t)totalleft;
      // totalleft converted to unsigned size_t, so something like
4294966184 or worse
  }

Then Curl_read() is called to receive at most "infinite" bytes, which
it happily fulfils, reading even the response after the body. Heck,
this might be even a security flaw, possibly overwritting memory
behind the available BUFSIZE bytes with data received from the
network!

Seems to me that the solution is, again, to not decrement 'size' at
imap.c:1468, as suggested earlier in this post. That way, the number
of remaining octets would be calculated correctly, no excess data
would ever get received and everyone would be happy. Hopefully :-)

Kind regards,
Jiri
-------------------------------------------------------------------
List admin: http://cool.haxx.se/list/listinfo/curl-library
Etiquette: http://curl.haxx.se/mail/etiquette.html
Received on 2013-09-04