If you use --iconv to sync between two machines and a file with a name that needs conversion is to be deleted, the rsync protocol may get out of sync and yield a protocol error if the converted filename differs in length form the unconverted name. This is because mplex_write in io.c will record and send the message length *before* the message has been converted using iconvbuf. If the converted message differs in length, the reading rsync process will get confused and interpret part of the message data as tag & length. Here is an example: [21:45:27]lenux:~/Mirror$ rsync -a --progress --del -n --iconv=utf-8,utf-8-mac Lennart/Auto/ sva:Mirror/Lennart/Auto/ sending incremental file list Audi Cabrio 92 (CUB931)/D\#303\#244ck/ deleting Audi Cabrio 92 (CUB931)/D\#303\#244ck/www.stro.nu.url.URL\#001 unexpected tag -7 [sender] rsync error: error in rsync protocol data stream (code 12) at io.c(1135) [sender=3.0.0pre5] (Note the garbage #\001 at the end of the filename.) To fix this, mplex_write needs to convert the message data before sending its length. Here is a possible rewrite: /* Write an message to a multiplexed stream. If this fails, rsync exits. */ static void mplex_write(int fd, enum msgcode code, const char *buf, size_t len, int convert) { char buffer[BIGPATHBUFLEN]; /* Oversized for use by iconv code. */ size_t n = len; #ifdef ICONV_OPTION if (convert && ic_send == (iconv_t)-1) #endif convert = 0; #ifdef ICONV_OPTION /* We need to convert buf before doing anything else so that we * can include the (converted) byte length in the message header. */ if (convert) { xbuf outbuf, inbuf; INIT_XBUF(outbuf, buffer + 4, 0, sizeof(buffer) - 4); INIT_XBUF(inbuf, (char*)buf, len, -1); iconvbufs(ic_send, &inbuf, &outbuf, ICB_INCLUDE_BAD | ICB_INCLUDE_INCOMPLETE); if (inbuf.len > 0) { rprintf(FERROR, "conversion buffer overflow in mplex_write; " "either enlarge 'buffer' or rewrite code to allocate " "buffer dynamically"); } n = len = outbuf.len; } else #endif if (n > sizeof(buffer) - 4) n = 0; else memcpy(buffer + 4, buf, n); SIVAL(buffer, 0, ((MPLEX_BASE + (int)code)<<24) + len); defer_forwarding_keep = 1; /* defer_forwarding_messages++ on return */ writefd_unbuffered(fd, buffer, n+4); defer_forwarding_keep = 0; len -= n; buf += n; if (len) writefd_unbuffered(fd, buf, len); if (!--defer_forwarding_messages && !no_flush) msg_flush(); } Context diff based on 3.0.0pre5 follows: *** io.c~ Sat Nov 3 00:20:05 2007 --- io.c Sun Nov 11 21:59:57 2007 *************** *** 468,485 **** char buffer[BIGPATHBUFLEN]; /* Oversized for use by iconv code. */ size_t n = len; - SIVAL(buffer, 0, ((MPLEX_BASE + (int)code)<<24) + len); - #ifdef ICONV_OPTION if (convert && ic_send == (iconv_t)-1) #endif convert = 0; ! if (convert || n > 1024 - 4) /* BIGPATHBUFLEN can handle 1024 bytes */ n = 0; else memcpy(buffer + 4, buf, n); defer_forwarding_keep = 1; /* defer_forwarding_messages++ on return */ writefd_unbuffered(fd, buffer, n+4); defer_forwarding_keep = 0; --- 468,508 ---- char buffer[BIGPATHBUFLEN]; /* Oversized for use by iconv code. */ size_t n = len; #ifdef ICONV_OPTION if (convert && ic_send == (iconv_t)-1) #endif convert = 0; ! #ifdef ICONV_OPTION ! /* We need to convert buf before doing anything else so that we ! * can include the (converted) byte length in the message header. ! */ ! if (convert) { ! xbuf outbuf, inbuf; ! ! INIT_XBUF(outbuf, buffer + 4, 0, sizeof(buffer) - 4); ! INIT_XBUF(inbuf, (char*)buf, len, -1); ! ! iconvbufs(ic_send, &inbuf, &outbuf, ! ICB_INCLUDE_BAD | ICB_INCLUDE_INCOMPLETE); ! ! if (inbuf.len > 0) { ! rprintf(FERROR, "conversion buffer overflow in mplex_write; " ! "either enlarge 'buffer' or rewrite code to allocate " ! "buffer dynamically"); ! } ! ! n = len = outbuf.len; ! } else ! #endif ! if (n > sizeof(buffer) - 4) n = 0; else memcpy(buffer + 4, buf, n); + SIVAL(buffer, 0, ((MPLEX_BASE + (int)code)<<24) + len); + + defer_forwarding_keep = 1; /* defer_forwarding_messages++ on return */ writefd_unbuffered(fd, buffer, n+4); defer_forwarding_keep = 0; *************** *** 487,506 **** len -= n; buf += n; - #ifdef ICONV_OPTION - if (convert) { - xbuf outbuf, inbuf; - - INIT_CONST_XBUF(outbuf, buffer); - INIT_XBUF(inbuf, (char*)buf, len, -1); - - do { - iconvbufs(ic_send, &inbuf, &outbuf, - ICB_INCLUDE_BAD | ICB_INCLUDE_INCOMPLETE); - writefd_unbuffered(fd, outbuf.buf, outbuf.len); - } while (inbuf.len); - } else - #endif if (len) writefd_unbuffered(fd, buf, len);
Created attachment 2963 [details] Diffs to fix bug
Sorry, I meant to add this too: There is also an uninitialized variable in readfd_buffered that can cause garbage to be read independently of the bug in mplex_write. Under case MSG_DELETED, inbuf.pos is never initialized which can -- and will -- cause the call to iconvbufs further down to process random data. The fix is to initialize inbuf with a proper call to INIT_XBUF before proceeding to do the conversion. Diff below and attached. --- 510,515 ---- *************** *** 1060,1066 **** int pos = 0; INIT_CONST_XBUF(outbuf, line); ! inbuf.buf = ibuf; while (msg_bytes) { inbuf.len = msg_bytes > sizeof ibuf --- 1069,1075 ---- int pos = 0; INIT_CONST_XBUF(outbuf, line); ! INIT_XBUF(inbuf, ibuf, 0, -1); while (msg_bytes) { inbuf.len = msg_bytes > sizeof ibuf
Thank you very much for the problem report and the patch. Both problems are now fixed in the git version.