DO NOT REPLY [Bug 5075] New: Syncing with --iconv may yield protocol error

[samba-bugs at samba.org]

https://bugzilla.samba.org/show_bug.cgi?id=5075

           Summary: Syncing with --iconv may yield protocol error
           Product: rsync
           Version: 3.0.0
          Platform: All
        OS/Version: All
            Status: NEW
          Severity: major
          Priority: P3
         Component: core
        AssignedTo: wayned at samba.org
        ReportedBy: lennart.samba at lovstrand.com
         QAContact: rsync-qa at samba.org


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);


-- 
Configure bugmail: https://bugzilla.samba.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug, or are watching the QA contact.