pidgin/pidgin

Fix hang when completing a file transfer over XMPP
release-2.x.y
23 months ago, Belgin Știrbu
1d02278206d2
Parents ccd35a6add9c
Children 55aa35e81e71
Fix hang when completing a file transfer over XMPP

The way purple_xfer_drain_socket was implemented,
if the peer never close()d the connection, we would
wait forever for it to be closed remotely, causing
Pidgin to hang.

Testing Done:
Tested IRC and XMPP on Windows and Linux. Transferred big and small files.

Reviewed at https://reviews.imfreedom.org/r/1466/
2 files changed, 46 insertions(+), 21 deletions(-)
  • +13 -21
    libpurple/ft.c
  • +33 -0
    libpurple/protocols/irc/dcc_send.c
    • --- a/libpurple/ft.c Mon May 23 20:30:57 2022 -0500
    +++ b/libpurple/ft.c Wed May 25 23:51:10 2022 -0500
    @@ -1107,11 +1107,12 @@
    r = write(xfer->fd, buffer, s);
    if (r < 0 && errno == EAGAIN)
    r = 0;
    +
    + if ((purple_xfer_get_bytes_sent(xfer) + r) >= purple_xfer_get_size(xfer) &&
    + !purple_xfer_is_completed(xfer)) {
    + purple_xfer_set_completed(xfer, TRUE);
    + }
    }
    - if (r >= 0 && (purple_xfer_get_bytes_sent(xfer)+r) >= purple_xfer_get_size(xfer) &&
    - !purple_xfer_is_completed(xfer))
    - purple_xfer_set_completed(xfer, TRUE);
    -
    return r;
    }
    @@ -1222,6 +1223,7 @@
    if (xfer->type == PURPLE_XFER_RECEIVE) {
    r = purple_xfer_read(xfer, &buffer);
    +
    if (r > 0) {
    size_t wc;
    if (ui_ops && ui_ops->ui_write)
    @@ -1236,9 +1238,13 @@
    return;
    }
    - if ((purple_xfer_get_size(xfer) > 0) &&
    - ((purple_xfer_get_bytes_sent(xfer)+r) >= purple_xfer_get_size(xfer)))
    - purple_xfer_set_completed(xfer, TRUE);
    + if(xfer->ops.read == NULL) {
    + if ((purple_xfer_get_size(xfer) > 0) &&
    + ((purple_xfer_get_bytes_sent(xfer) + r) >= purple_xfer_get_size(xfer))) {
    + purple_xfer_set_completed(xfer, TRUE);
    + }
    + }
    +
    } else if(r < 0) {
    purple_xfer_cancel_remote(xfer);
    g_free(buffer);
    @@ -1545,19 +1551,6 @@
    begin_transfer(xfer, cond);
    }
    -static void
    -purple_xfer_drain_socket(int sock)
    -{
    - int ret;
    - char buffer[64];
    -
    - do {
    - ret = read(sock, buffer, sizeof(buffer));
    - } while(ret > 0 ||
    - (ret == -1 &&
    - (errno == EAGAIN || errno == EWOULDBLOCK)));
    -}
    -
    void
    purple_xfer_end(PurpleXfer *xfer)
    {
    @@ -1579,7 +1572,6 @@
    }
    if (xfer->fd != -1) {
    - purple_xfer_drain_socket(xfer->fd);
    close(xfer->fd);
    }
    --- a/libpurple/protocols/irc/dcc_send.c Mon May 23 20:30:57 2022 -0500
    +++ b/libpurple/protocols/irc/dcc_send.c Wed May 25 23:51:10 2022 -0500
    @@ -62,6 +62,38 @@
    }
    }
    +static gssize irc_dccsend_recv_read(guchar **buffer, PurpleXfer *xfer) {
    + gssize s, r;
    +
    + if (purple_xfer_get_size(xfer) == 0) {
    + s = xfer->current_buffer_size;
    + } else {
    + s = MIN(purple_xfer_get_bytes_remaining(xfer), xfer->current_buffer_size);
    + }
    +
    + *buffer = g_malloc0(s);
    +
    + r = read(xfer->fd, *buffer, s);
    + if (r < 0 && errno == EAGAIN) {
    + /* read() would block if the socket was nonblocking, so retry */
    + r = 0;
    + } else if (r < 0) {
    + /* read() has errored out for some other reason, so we fail the xfer */
    + r = -1;
    + } else if (r == 0) {
    + /* read() signals that there is no more data to get from the socket */
    + if(purple_xfer_get_bytes_sent(xfer) >= purple_xfer_get_size(xfer)) {
    + /* We got all the data, xfer successful */
    + purple_xfer_set_completed(xfer, TRUE);
    + } else {
    + /* We expect more data, fail the xfer */
    + r = -1;
    + }
    + }
    +
    + return r;
    +}
    +
    static void irc_dccsend_recv_init(PurpleXfer *xfer) {
    struct irc_xfer_rx_data *xd = xfer->data;
    @@ -134,6 +166,7 @@
    purple_xfer_set_init_fnc(xfer, irc_dccsend_recv_init);
    purple_xfer_set_ack_fnc(xfer, irc_dccsend_recv_ack);
    + purple_xfer_set_read_fnc(xfer, irc_dccsend_recv_read);
    purple_xfer_set_end_fnc(xfer, irc_dccsend_recv_destroy);
    purple_xfer_set_request_denied_fnc(xfer, irc_dccsend_recv_destroy);
    purple_xfer_set_cancel_recv_fnc(xfer, irc_dccsend_recv_destroy);