handle partial write()'s without data loss

CHANGES

@@ -57,6 +57,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.
+
 	docu mentions option so-bindtodev but correct name is so-bindtodevice. 
 	Thanks to Jim Zimmerman for reporting.
 

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

@@ -41,6 +41,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

@@ -10868,6 +10868,50 @@ 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))
+
 
 # socat up to 1.7.2.0 and  2.0.0-b4 had a bug in xioscan_readline() that could
 # be exploited 

xio-proxy.c

@@ -371,10 +371,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->wfd, request, strlen(request));
-   } while (sresult < 0 && errno == EINTR);
-   if (sresult < 0) {
+   if (writefull(xfd->wfd, request, strlen(request)) < 0) {
       Msg4(level, "write(%d, %p, "F_Zu"): %s",
 	   xfd->wfd, request, strlen(request), strerror(errno));
       if (Close(xfd->wfd) < 0) {
@@ -404,10 +401,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->wfd, header, strlen(header));
-      } while (sresult < 0 && errno == EINTR);
-      if (sresult < 0) {
+      if (writefull(xfd->wfd, header, strlen(header)) < 0) {
 	 Msg4(level, "write(%d, %p, "F_Zu"): %s",
 	      xfd->wfd, header, strlen(header), strerror(errno));
 	 if (Close(xfd->wfd/*!*/) < 0) {
@@ -420,10 +414,14 @@ int _xioopen_proxy_connect(struct single *xfd,
    }
 
    Info("sending \"\\r\\n\"");
-   do {
-      sresult = Write(xfd->wfd, "\r\n", 2);
-   } while (sresult < 0 && errno == EINTR);
-   /*! */
+   if (writefull(xfd->wfd, "\r\n", 2) < 0) {
+      Msg2(level, "write(%d, %p, "F_Zu"): %s",
+	   xfd->wfd, strerror(errno));
+      if (Close(xfd->wfd) < 0) {
+	 Info2("close(%d): %s", xfd->wfd, strerror(errno));
+      }
+      return STAT_RETRYLATER;
+   }
 
    /* request is kept for later error messages */
    *strstr(request, " HTTP") = '\0';

xio-readline.c

@@ -185,9 +185,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->rfd, "\r", 1);
-	 } while (writt < 0 && errno == EINTR);
+	 writt = writefull(pipe->rfd, "\r", 1);
 	 if (writt < 0) {
 	    Warn2("write(%d, \"\\r\", 1): %s",
 		   pipe->rfd, strerror(errno));

xio-socks.c

@@ -377,10 +377,7 @@ int _xioopen_socks4_connect(struct single *xfd,
       }
    }
 #endif /* WITH_MSGLEVEL <= E_DEBUG */
-   do {
-      result = Write(wfd, sockhead, headlen);
-   } while (result < 0 && errno == EINTR);
-   if (result < 0) {
+   if (writefull(wfd, sockhead, headlen) < 0) {
       Msg4(level, "write(%d, %p, "F_Zu"): %s",
 	   wfd, sockhead, headlen, strerror(errno));
       if (Close(wfd) < 0) {

xiolockfile.c

@@ -1,5 +1,5 @@
 /* source: xiolockfile.c */
-/* Copyright Gerhard Rieger 2005-2006 */
+/* Copyright Gerhard Rieger 2005-2012 */
 /* 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-2012 */
 /* Published under the GNU General Public License V.2, see file COPYING */
 
 /* this is the source of the extended write function */
@@ -53,9 +53,7 @@ ssize_t xiowrite(xiofile_t *file, const void *buff, size_t bytes) {
    switch (pipe->dtype & XIODATA_WRITEMASK) {
 
    case XIOWRITE_STREAM:
-      do {
-	 writt = Write(fd, buff, bytes);
-      } while (writt < 0 && errno == EINTR);
+      writt = writefull(pipe->wfd, buff, bytes);
       if (writt < 0) {
 	 _errno = errno;
 	 switch (_errno) {
@@ -74,10 +72,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
@@ -125,9 +119,7 @@ ssize_t xiowrite(xiofile_t *file, const void *buff, size_t bytes) {
 
    case XIOWRITE_PIPE:
    case XIOWRITE_2PIPE:
-      do {
-	 writt = Write(fd, buff, bytes);
-      } while (writt < 0 && errno == EINTR);
+      writt = Write(pipe->wfd, buff, bytes);
       _errno = errno;
       if (writt < 0) {
 	 Error4("write(%d, %p, "F_Zu"): %s",
@@ -135,10 +127,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_TEST