Fix hangups on binary blob reads from webcit stable

diff --git a/citadel/file_ops.c b/citadel/file_ops.c
index ada35251401c8489c25c0e5e8fef1af54fc73217..413e32da1aa6f62f6fc41fc6f0d4b3916c6f0466 100644 (file)
--- a/citadel/file_ops.c
+++ b/citadel/file_ops.c
@@ -639,9 +639,9 @@ void cmd_read(char *cmdbuf)
 {
        long start_pos;
        size_t bytes;
-       size_t actual_bytes;
-       char *buf = NULL;
+       char buf[SIZ];
 
+       /* The client will transmit its requested offset and byte count */
        start_pos = extract_long(cmdbuf, 0);
        bytes = extract_int(cmdbuf, 1);
 
@@ -651,24 +651,24 @@ void cmd_read(char *cmdbuf)
                return;
        }
 
-       if (bytes > 100000) bytes = 100000;
-       buf = malloc(bytes + 1);
+       /* If necessary, reduce the byte count to the size of our buffer */
+       if (bytes > sizeof(buf)) {
+               bytes = sizeof(buf);
+       }
 
        fseek(CC->download_fp, start_pos, 0);
-
-       actual_bytes = fread(buf, 1, bytes, CC->download_fp);
-       if (actual_bytes > 0) {
-               cprintf("%d %d\n", BINARY_FOLLOWS, (int)actual_bytes);
+       bytes = fread(buf, 1, bytes, CC->download_fp);
+       if (bytes > 0) {
+               /* Tell the client the actual byte count and transmit it */
+               cprintf("%d %d\n", BINARY_FOLLOWS, (int)bytes);
                client_write(buf, bytes);
        }
        else {
                cprintf("%d %s\n", ERROR, strerror(errno));
        }
-       free(buf);
 }
 
 
-
 /*
  * write to the upload file
  */

diff --git a/webcit/tcp_sockets.c b/webcit/tcp_sockets.c
index c488b43158f032742e9df58eac54a87ce46ba361..e783feb7fca0ca5f9643b581a92bee5d1ad0c381 100644 (file)
--- a/webcit/tcp_sockets.c
+++ b/webcit/tcp_sockets.c
@@ -1,7 +1,5 @@
 /*
- * $Id$
- *
- * Copyright (c) 1987-2010 by the citadel.org team
+ * Copyright (c) 1987-2011 by the citadel.org team
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
@@ -28,7 +26,6 @@
 #include "webserver.h"
 
 extern int DisableGzip;
-long MaxRead = -1; /* should we do READ scattered or all at once? */
 
 /*
  * register the timeout
@@ -313,115 +310,45 @@ void serv_printf(const char *format,...)
 
 
 
-/**
- * Read binary data from server into memory using a series of
- * server READ commands.
- * \return the read content as StrBuf
+/*
+ * Read binary data from server into memory using a series of server READ commands.
+ * returns the read content as StrBuf
  */
 int serv_read_binary(StrBuf *Ret, size_t total_len, StrBuf *Buf) 
 {
        wcsession *WCC = WC;
-       size_t bytes = 0;
-       size_t thisblock = 0;
-       
-       if (Ret == NULL)
-           return -1;
+       size_t bytes_read = 0;
+       size_t this_block = 0;
+       int rc;
 
-       if (MaxRead == -1)
-       {
-               serv_printf("READ %d|"SIZE_T_FMT, 0, total_len);
-               if (StrBuf_ServGetln(Buf) > 0)
-               {
-                       long YetRead;
-                       const char *ErrStr;
-                       const char *pch;
-                       int rc;
-
-                       if (GetServerStatus(Buf, NULL) == 6)
-                       {
-                           StrBufCutLeft(Buf, 4);
-                           thisblock = StrTol(Buf);
-                           if (WCC->serv_sock==-1) {
-                                   FlushStrBuf(Ret); 
-                                   return -1; 
-                           }
-
-                           if (WCC->ReadPos != NULL) {
-                                   pch = ChrPtr(WCC->ReadBuf);
-
-                                   YetRead = WCC->ReadPos - pch;
-                                   if (YetRead > 0)
-                                   {
-                                           long StillThere;
-                                           
-                                           StillThere = StrLength(WCC->ReadBuf) - 
-                                                   YetRead;
-                                           
-                                           StrBufPlain(Ret, 
-                                                       WCC->ReadPos,
-                                                       StillThere);
-                                           total_len -= StillThere;
-                                   }
-                                   FlushStrBuf(WCC->ReadBuf);
-                                   WCC->ReadPos = NULL;
-                           } 
-                           if (total_len > 0)
-                           {
-                                   rc = StrBufReadBLOB(Ret, 
-                                                       &WCC->serv_sock, 
-                                                       1, 
-                                                       total_len,
-                                                       &ErrStr);
-                                   if (rc < 0)
-                                   {
-                                           lprintf(1, "Server connection broken: %s\n",
-                                                   (ErrStr)?ErrStr:"");
-                                           wc_backtrace();
-                                           WCC->serv_sock = (-1);
-                                           WCC->connected = 0;
-                                           WCC->logged_in = 0;
-                                           return rc;
-                                   }
-                                   else
-                                           return StrLength(Ret);
-                           }
-                           else 
-                                   return StrLength(Ret);
-                       }
-               }
-               else
-                       return -1;
+       if (Ret == NULL) {
+               return -1;
        }
-       else while ((WCC->serv_sock!=-1) &&
-              (bytes < total_len)) {
-               thisblock = MaxRead;
-               if ((total_len - bytes) < thisblock) {
-                       thisblock = total_len - bytes;
-                       if (thisblock == 0) {
-                               FlushStrBuf(Ret); 
-                               return -1; 
-                       }
+
+       while (bytes_read < total_len) {
+
+               if (WCC->serv_sock==-1) {
+                       FlushStrBuf(Ret); 
+                       return -1; 
                }
-               serv_printf("READ %d|%d", (int)bytes, (int)thisblock);
-               if (StrBuf_ServGetln(Buf) > 0)
-               {
-                       if (GetServerStatus(Buf, NULL) == 6)
-                       {
-                           StrBufCutLeft(Buf, 4);
-                           thisblock = StrTol(Buf);
-                           if (WCC->serv_sock==-1) {
-                                   FlushStrBuf(Ret); 
-                                   return -1; 
-                           }
-                           StrBuf_ServGetBLOBBuffered(Ret, thisblock);
-                           bytes += thisblock;
-                   }
-                   else {
-                           lprintf(3, "Error: %s\n", ChrPtr(Buf) + 4);
-                           return -1;
-                   }
+
+               serv_printf("READ %d|%d", bytes_read, total_len-bytes_read);
+               if ( (StrBuf_ServGetln(Buf) > 0) && (GetServerStatus(Buf, NULL) == 6) ) {
+                       StrBufCutLeft(Buf, 4);
+                       this_block = StrTol(Buf);
+                       rc = StrBuf_ServGetBLOBBuffered(Ret, this_block);
+                       if (rc < 0) {
+                               lprintf(1, "Server connection broken during download\n");
+                               wc_backtrace();
+                               WCC->serv_sock = (-1);
+                               WCC->connected = 0;
+                               WCC->logged_in = 0;
+                               return rc;
+                       }
+                       bytes_read += rc;
                }
        }
+
        return StrLength(Ret);
 }
 

diff --git a/webcit/webserver.c b/webcit/webserver.c
index ae0c3dae56ad5ae45ec28136f6967719ab5ee80e..3546878dbcbeeb572aaad8d8b70a9afa3ef777fd 100644 (file)
--- a/webcit/webserver.c
+++ b/webcit/webserver.c
@@ -22,7 +22,6 @@ int vsnprintf(char *buf, size_t max, const char *fmt, va_list argp);
 extern int msock;                      /* master listening socket */
 extern int verbosity;          /* Logging level */
 extern char static_icon_dir[PATH_MAX];          /* where should we find our mime icons */
-extern long MaxRead; 
 int is_https = 0;              /* Nonzero if I am an HTTPS service */
 int follow_xff = 0;            /* Follow X-Forwarded-For: header */
 int home_specified = 0;                /* did the user specify a homedir? */
@@ -114,9 +113,9 @@ int main(int argc, char **argv)
 
        /* Parse command line */
 #ifdef HAVE_OPENSSL
-       while ((a = getopt(argc, argv, "R:u:h:i:p:t:T:B:x:dD:G:cfsS:Z")) != EOF)
+       while ((a = getopt(argc, argv, "u:h:i:p:t:T:B:x:dD:G:cfsS:Z")) != EOF)
 #else
-       while ((a = getopt(argc, argv, "R:u:h:i:p:t:T:B:x:dD:G:cfZ")) != EOF)
+       while ((a = getopt(argc, argv, "u:h:i:p:t:T:B:x:dD:G:cfZ")) != EOF)
 #endif
                switch (a) {
                case 'u':
@@ -205,9 +204,6 @@ int main(int argc, char **argv)
                        I18nDump = NewStrBufPlain(HKEY("int templatestrings(void)\n{\n"));
                        I18nDumpFile = optarg;
                        break;
-               case 'R':
-                       MaxRead = atol(optarg);
-                       break;
                default:
                        fprintf(stderr, "usage: webcit "
                                "[-i ip_addr] [-p http_port] "