Fix file transfers failing at 99% over IRC

Review Request #1385 — Created April 11, 2022 and submitted

Information

pidgin/pidgin
release-2.x.y

Reviewers

Fix file transfers failing at 99% over IRC

File transfers over IRC sometimes failed because the sender close()d the socket before reading everything from it, resulting in a TCP RST packet being sent, which was interpreted by the receiver as a problem with the connection, sometimes dropping the last few bytes of the file as a result.
This patch reads everything from the socket before closing, thus avoiding the RST packet issue.

Tested on Windows, Linux, Windows-to-Linux and vice-versa IRC file transfers.

Summary ID
Fix file transfers failing at 99% over IRC
52daf9171bd2ecd0feaecbc8f4bb520f5b607a58
Description From Last Updated

It looks like we're just throwing away the data here? Isn't this the last remaining bytes of the file that …

grimgrim
grim
  1. 
      
  2. libpurple/ft.c (Diff revision 1)
     
     
    Show all issues

    It looks like we're just throwing away the data here? Isn't this the last remaining bytes of the file that should be written to disk?

    1. No, when that line in the code is reached, the transfer is already complete. The scenario this patch fixes is the following (at least this is what I believe happens by reading lots of Wireshark logs):

      1. Sender sends last few bytes in the form of a few TCP data packets, then immediately closes the connection without draining the read buffer of the TCP socket, this makes Linux send a TCP RST packet to the receiver
      2. Receiver's kernel buffers data from the last few TCP data packets, but before Pidgin does a read() in purple_xfer_read to get this data, the kernel sees the aforementioned RST packet
      3. When Pidgin actually does the read() in purple_xfer_read, because the kernel saw the RST packet, the read syscall returns -1 (or 0, I forgot), so Pidgin discards the last few data packets

      This patch makes it so the kernel doesn't send the RST packet at all.

    2. Gotcha, thank you very much for the explanation!

    3. IIRC, for IRC DCC file transfers, the receiver sends back the bytes received for every chunk it receives. That is what's in the read buffer. However, IRC does not decide that the file transfer is complete; libpurple does, when sent size >= file size. This means IRC cannot correctly set the complete flag because libpurple overrides it. The most "correct" fix is that IRC should set complete and not libpurple, but I suspect that having libpurple not do the send size >= file size check will just break something else.

  3. 
      
grim
  1. Ship It!
  2. Awesome work again, thank you so much!!

  3. 
      
grim
Review request changed
Status:
Completed