handle partial write()'s without data loss

CHANGES

@@ -30,6 +30,14 @@ corrections:
 	process crash when socat prints info about an unnamed unix domain
 	socket
 
+	Michal Soltys reported the following problem and provided an initial
+	patch: when socat was interrupted, e.g. by SIGSTOP, and resumed during
+	data transfer only parts of the data might have been written.
+
+	Option o-nonblock in combination with large transfer block sizes
+	may result in partial writes and/or EAGAIN errors that were not handled
+	properly but resulted in data loss or process termination.
+
 ####################### V 1.7.1.3:
 
 security:

sysutils.c

@@ -16,6 +16,40 @@
 #include "utils.h"
 #include "sysutils.h"
 
+/* Substitute for Write():
+   Try to write all bytes before returning; this handles EINTR,
+   EAGAIN/EWOULDBLOCK, and partial write situations. The drawback is that this
+   function might block even with O_NONBLOCK option.
+   Returns <0 on unhandled error, errno valid
+   Will only return <0 or bytes
+*/
+ssize_t writefull(int fd, const void *buff, size_t bytes) {
+   size_t writt = 0;
+   ssize_t chk;
+   while (1) {
+      chk = Write(fd, (const char *)buff + writt, bytes - writt);
+      if (chk < 0) {
+	 switch (errno) {
+	 case EINTR:
+	 case EAGAIN:
+#if EAGAIN != EWOULDBLOCK
+	 case EWOULDBLOCK:
+#endif
+	    Warn4("write(%d, %p, "F_Zu"): %s", fd, (const char *)buff+writt, bytes-writt, strerror(errno));
+	    Sleep(1); continue;
+	 default: return -1;
+	 }
+      } else if (writt+chk < bytes) {
+	 Warn4("write(%d, %p, "F_Zu"): only wrote "F_Zu" bytes, trying to continue ",
+	       fd, (const char *)buff+writt, bytes-writt, chk);
+	 writt += chk;
+      } else {
+	 writt = bytes;
+	 break;
+      }
+   }
+   return writt;
+}
 
 #if WITH_UNIX
 void socket_un_init(struct sockaddr_un *sa) {

sysutils.h

@@ -1,5 +1,5 @@
 /* source: sysutils.h */
-/* Copyright Gerhard Rieger 2001-2010 */
+/* Copyright Gerhard Rieger 2001-2011 */
 /* Published under the GNU General Public License V.2, see file COPYING */
 
 #ifndef __sysutils_h_included
@@ -40,6 +40,8 @@ struct xiorange {
 } ;
 #endif /* _WITH_SOCKET */
 
+extern ssize_t writefull(int fd, const void *buff, size_t bytes);
+
 #if _WITH_SOCKET
 extern socklen_t socket_init(int af, union sockaddr_union *sa);
 #endif

test.sh

@@ -10439,6 +10439,51 @@ esac
 N=$((N+1))
 
 
+# incomplete writes were reported but led to data loss
+NAME=INCOMPLETE_WRITE
+case "$TESTS" in
+*%functions%*|*%bugs%*|*%$NAME%*)
+TEST="$NAME: check if incomplete writes are handled properly"
+# write to a nonblocking fd a block that is too large for atomic write
+# and check if all data arrives
+if ! eval $NUMCOND; then :; else
+tf="$td/test$N.stdout"
+te="$td/test$N.stderr"
+tp="$td/test$N.pipe"
+tw="$td/test$N.wc-c"
+# this is the size we write() in one call; data is never stored on disk, so
+# make it large enough to exceed any atomic write size; but higher number might
+# take much time
+bytes=100000	# for Linux 2.6.? this must be >65536
+CMD0="$SOCAT $opts -u PIPE:$tp STDOUT"
+CMD1="$SOCAT $opts -u -b $bytes OPEN:/dev/zero,readbytes=$bytes FILE:$tp,o-nonblock"
+printf "test $F_n $TEST... " $N
+$CMD0 2>"${te}0" |wc -c >"$tw" &
+pid=$!
+waitfile "$tp"
+$CMD1 2>"${te}1" >"${tf}1"
+rc1=$?
+wait
+if [ $rc1 -ne 0 ]; then
+    $PRINTF "$NO_RESULT\n"
+    numCANT=$((numCANT+1))
+elif [ ! -e "$tw" ]; then 
+    $PRINTF "$NO_RESULT\n"
+    numCANT=$((numCANT+1))
+elif [ "$bytes" -eq $(cat "$tw") ]; then
+    $PRINTF "$OK\n"
+    numOK=$((numOK+1))
+else
+    $PRINTF "$FAILED\n"
+    echo "transferred only $(cat $tw) of $bytes bytes" >&2
+    numFAIL=$((numFAIL+1))
+fi
+fi # NUMCOND
+ ;;
+esac
+N=$((N+1))
+
+
 ###############################################################################
 # here come tests that might affect your systems integrity. Put normal tests
 # before this paragraph.

xio-proxy.c

@@ -1,5 +1,5 @@
 /* source: xio-proxy.c */
-/* Copyright Gerhard Rieger 2002-2008 */
+/* Copyright Gerhard Rieger 2002-2011 */
 /* Published under the GNU General Public License V.2, see file COPYING */
 
 /* this file contains the source for opening addresses of HTTP proxy CONNECT
@@ -293,10 +293,7 @@ int _xioopen_proxy_connect(struct single *xfd,
    * xiosanitize(request, strlen(request), textbuff) = '\0';
    Info1("sending \"%s\"", textbuff);
    /* write errors are assumed to always be hard errors, no retry */
-   do {
-      sresult = Write(xfd->fd, request, strlen(request));
-   } while (sresult < 0 && errno == EINTR);
-   if (sresult < 0) {
+   if (writefull(xfd->fd, request, strlen(request)) < 0) {
       Msg4(level, "write(%d, %p, "F_Zu"): %s",
 	   xfd->fd, request, strlen(request), strerror(errno));
       if (Close(xfd->fd) < 0) {
@@ -326,10 +323,7 @@ int _xioopen_proxy_connect(struct single *xfd,
       *next = '\0';
       Info1("sending \"%s\\r\\n\"", header);
       *next++ = '\r';  *next++ = '\n'; *next++ = '\0';
-      do {
-	 sresult = Write(xfd->fd, header, strlen(header));
-      } while (sresult < 0 && errno == EINTR);
-      if (sresult < 0) {
+      if (writefull(xfd->fd, header, strlen(header)) < 0) {
 	 Msg4(level, "write(%d, %p, "F_Zu"): %s",
 	      xfd->fd, header, strlen(header), strerror(errno));
 	 if (Close(xfd->fd) < 0) {
@@ -342,10 +336,14 @@ int _xioopen_proxy_connect(struct single *xfd,
    }
 
    Info("sending \"\\r\\n\"");
-   do {
-      sresult = Write(xfd->fd, "\r\n", 2);
-   } while (sresult < 0 && errno == EINTR);
-   /*! */
+   if (writefull(xfd->fd, "\r\n", 2) < 0) {
+      Msg2(level, "write(%d, %p, "F_Zu"): %s",
+	   xfd->fd, strerror(errno));
+      if (Close(xfd->fd) < 0) {
+	 Info2("close(%d): %s", xfd->fd, strerror(errno));
+      }
+      return STAT_RETRYLATER;
+   }
 
    /* request is kept for later error messages */
    *strstr(request, " HTTP") = '\0';

xio-readline.c

@@ -1,5 +1,5 @@
 /* source: xio-readline.c */
-/* Copyright Gerhard Rieger 2002-2009 */
+/* Copyright Gerhard Rieger 2002-2011 */
 /* Published under the GNU General Public License V.2, see file COPYING */
 
 /* this file contains the source for opening the readline address */
@@ -177,9 +177,7 @@ ssize_t xioread_readline(struct single *pipe, void *buff, size_t bufsiz) {
 	 /* we must carriage return, because readline will first print the
 	    prompt */
 	 ssize_t writt;
-	 do {
-	    writt = Write(pipe->fd, "\r", 1);
-	 } while (writt < 0 && errno == EINTR);
+	 writt = writefull(pipe->fd, "\r", 1);
 	 if (writt < 0) {
 	    Warn2("write(%d, \"\\r\", 1): %s",
 		   pipe->fd, strerror(errno));

xio-socks.c

@@ -1,5 +1,5 @@
 /* source: xio-socks.c */
-/* Copyright Gerhard Rieger 2001-2009 */
+/* Copyright Gerhard Rieger 2001-2011 */
 /* Published under the GNU General Public License V.2, see file COPYING */
 
 /* this file contains the source for opening addresses of socks4 type */
@@ -328,10 +328,7 @@ int _xioopen_socks4_connect(struct single *xfd,
       }
    }
 #endif /* WITH_MSGLEVEL <= E_DEBUG */
-   do {
-      result = Write(xfd->fd, sockhead, headlen);
-   } while (result < 0 && errno == EINTR);
-   if (result < 0) {
+   if (writefull(xfd->fd, sockhead, headlen) < 0) {
       Msg4(level, "write(%d, %p, "F_Zu"): %s",
 	   xfd->fd, sockhead, headlen, strerror(errno));
       if (Close(xfd->fd) < 0) {

xiolockfile.c

@@ -1,5 +1,5 @@
 /* source: xiolockfile.c */
-/* Copyright Gerhard Rieger 2005-2006 */
+/* Copyright Gerhard Rieger 2005-2011 */
 /* Published under the GNU General Public License V.2, see file COPYING */
 
 /* this file contains socats explicit locking mechanisms */
@@ -52,7 +52,10 @@ int xiogetlock(const char *lockfile) {
   
    pid = Getpid();
    bytes = sprintf(pidbuf, F_pid, pid);
-   Write(fd, pidbuf, bytes);
+   if (writefull(fd, pidbuf, bytes) < 0) {
+      Error4("write(%d, %p, "F_Zu"): %s", fd, pidbuf, bytes, strerror(errno));
+      return -1;
+   }
    Close(fd);
 
    /* Chmod(lockfile, 0600); */

xiowrite.c

@@ -1,5 +1,5 @@
 /* source: xiowrite.c */
-/* Copyright Gerhard Rieger 2001-2008 */
+/* Copyright Gerhard Rieger 2001-2011 */
 /* Published under the GNU General Public License V.2, see file COPYING */
 
 /* this is the source of the extended write function */
@@ -49,9 +49,7 @@ ssize_t xiowrite(xiofile_t *file, const void *buff, size_t bytes) {
    switch (pipe->dtype & XIODATA_WRITEMASK) {
 
    case XIOWRITE_STREAM:
-      do {
-	 writt = Write(pipe->fd, buff, bytes);
-      } while (writt < 0 && errno == EINTR);
+      writt = writefull(pipe->fd, buff, bytes);
       if (writt < 0) {
 	 _errno = errno;
 	 switch (_errno) {
@@ -70,10 +68,6 @@ ssize_t xiowrite(xiofile_t *file, const void *buff, size_t bytes) {
 	 errno = _errno;
 	 return -1;
       }
-      if ((size_t)writt < bytes) {
-	 Warn2("write() only wrote "F_Zu" of "F_Zu" bytes",
-	       writt, bytes);
-      }
       break;
 
 #if _WITH_SOCKET
@@ -120,9 +114,7 @@ ssize_t xiowrite(xiofile_t *file, const void *buff, size_t bytes) {
 #endif /* _WITH_SOCKET */
 
    case XIOWRITE_PIPE:
-      do {
-	 writt = Write(pipe->para.bipipe.fdout, buff, bytes);
-      } while (writt < 0 && errno == EINTR);
+      writt = Write(pipe->para.bipipe.fdout, buff, bytes);
       _errno = errno;
       if (writt < 0) {
 	 Error4("write(%d, %p, "F_Zu"): %s",
@@ -130,16 +122,10 @@ ssize_t xiowrite(xiofile_t *file, const void *buff, size_t bytes) {
 	 errno = _errno;
 	 return -1;
       }
-      if ((size_t)writt < bytes) {
-	 Warn2("write() only wrote "F_Zu" of "F_Zu" bytes",
-	       writt, bytes);
-      }
       break;
 
    case XIOWRITE_2PIPE:
-      do {
-	 writt = Write(pipe->para.exec.fdout, buff, bytes);
-      } while (writt < 0 && errno == EINTR);
+      writt = Write(pipe->para.exec.fdout, buff, bytes);
       _errno = errno;
       if (writt < 0) {
 	 Error4("write(%d, %p, "F_Zu"): %s",
@@ -147,10 +133,6 @@ ssize_t xiowrite(xiofile_t *file, const void *buff, size_t bytes) {
 	 errno = _errno;
 	 return -1;
       }
-      if ((size_t)writt < bytes) {
-	 Warn2("write() only processed "F_Zu" of "F_Zu" bytes",
-	       writt, bytes);
-      }
       break;
 
 #if WITH_OPENSSL