http_proxy: Fix proxy CONNECT hang on pending data

- Check for pending data before waiting on the socket.

Bug: https://github.com/curl/curl/issues/1156
Reported-by: Adam Langley

lib/connect.c

@@ -1404,3 +1404,16 @@ void Curl_conncontrol(struct connectdata *conn,
                                    should assign this bit */
   }
 }
+
+/* Data received can be cached at various levels, so check them all here. */
+bool Curl_conn_data_pending(struct connectdata *conn, int sockindex)
+{
+  int readable;
+
+  if(Curl_ssl_data_pending(conn, sockindex) ||
+     Curl_recv_has_postponed_data(conn, sockindex))
+    return true;
+
+  readable = SOCKET_READABLE(conn->sock[sockindex], 0);
+  return (readable > 0 && (readable & CURL_CSELECT_IN));
+}

lib/connect.h

@@ -142,4 +142,6 @@ void Curl_conncontrol(struct connectdata *conn,
 #define connkeep(x,y) Curl_conncontrol(x, CONNCTRL_KEEP)
 #endif
 
+bool Curl_conn_data_pending(struct connectdata *conn, int sockindex);
+
 #endif /* HEADER_CURL_CONNECT_H */

lib/ftp.c

@@ -740,7 +740,7 @@ CURLcode Curl_GetFTPResponse(ssize_t *nreadp, /* return number of bytes read */
        * wait for more data anyway.
        */
     }
-    else if(!Curl_ssl_data_pending(conn, FIRSTSOCKET)) {
+    else if(!Curl_conn_data_pending(conn, FIRSTSOCKET)) {
       switch(SOCKET_READABLE(sockfd, interval_ms)) {
       case -1: /* select() error, stop reading */
         failf(data, "FTP response aborted due to select/poll error: %d",

lib/http_proxy.c

@@ -285,7 +285,7 @@ CURLcode Curl_proxyCONNECT(struct connectdata *conn,
     }
 
     if(!blocking) {
-      if(0 == SOCKET_READABLE(tunnelsocket, 0))
+      if(!Curl_conn_data_pending(conn, sockindex))
         /* return so we'll be called again polling-style */
         return CURLE_OK;
       else {
@@ -304,13 +304,22 @@ CURLcode Curl_proxyCONNECT(struct connectdata *conn,
       char *ptr;
       char *line_start;
 
-      ptr=data->state.buffer;
+      ptr = data->state.buffer;
       line_start = ptr;
 
-      nread=0;
-      perline=0;
+      nread = 0;
+      perline = 0;
 
-      while((nread<BUFSIZE) && (keepon && !error)) {
+      while(nread < BUFSIZE && keepon && !error) {
+        int writetype;
+
+        if(Curl_pgrsUpdate(conn))
+          return CURLE_ABORTED_BY_CALLBACK;
+
+        if(ptr >= &data->state.buffer[BUFSIZE]) {
+          failf(data, "CONNECT response too large!");
+          return CURLE_RECV_ERROR;
+        }
 
         check = Curl_timeleft(data, NULL, TRUE);
         if(check <= 0) {
@@ -319,26 +328,22 @@ CURLcode Curl_proxyCONNECT(struct connectdata *conn,
           break;
         }
 
-        /* loop every second at least, less if the timeout is near */
-        switch(SOCKET_READABLE(tunnelsocket, check<1000L?check:1000)) {
-        case -1: /* select() error, stop reading */
+        /* Read one byte at a time to avoid a race condition. Wait at most one
+           second before looping to ensure continuous pgrsUpdates. */
+        result = Curl_read(conn, tunnelsocket, ptr, 1, &gotbytes);
+        if(result == CURLE_AGAIN) {
+          if(SOCKET_READABLE(tunnelsocket, check<1000L?check:1000) == -1) {
             error = SELECT_ERROR;
             failf(data, "Proxy CONNECT aborted due to select/poll error");
             break;
-        case 0: /* timeout */
-          break;
-        default:
-          if(ptr >= &data->state.buffer[BUFSIZE]) {
-            failf(data, "CONNECT response too large!");
-            return CURLE_RECV_ERROR;
           }
-          result = Curl_read(conn, tunnelsocket, ptr, 1, &gotbytes);
-          if(result==CURLE_AGAIN)
-            continue; /* go loop yourself */
-          else if(result)
+          continue;
+        }
+        else if(result) {
           keepon = FALSE;
+          break;
+        }
         else if(gotbytes <= 0) {
-            keepon = FALSE;
           if(data->set.proxyauth && data->state.authproxy.avail) {
             /* proxy auth was requested and there was proxy auth available,
                then deem this as "mere" proxy disconnect */
@@ -349,8 +354,10 @@ CURLcode Curl_proxyCONNECT(struct connectdata *conn,
             error = SELECT_ERROR;
             failf(data, "Proxy CONNECT aborted");
           }
+          keepon = FALSE;
+          break;
         }
-          else {
+
         /* We got a byte of data */
         nread++;
 
@@ -358,12 +365,12 @@ CURLcode Curl_proxyCONNECT(struct connectdata *conn,
           /* This means we are currently ignoring a response-body */
 
           nread = 0; /* make next read start over in the read buffer */
-              ptr=data->state.buffer;
+          ptr = data->state.buffer;
           if(cl) {
             /* A Content-Length based body: simply count down the counter
                and make sure to break out of the loop when we're done! */
             cl--;
-                if(cl<=0) {
+            if(cl <= 0) {
               keepon = FALSE;
               break;
             }
@@ -372,7 +379,7 @@ CURLcode Curl_proxyCONNECT(struct connectdata *conn,
             /* chunked-encoded body, so we need to do the chunked dance
                properly to know when the end of the body is reached */
             CHUNKcode r;
-                ssize_t tookcareof=0;
+            ssize_t tookcareof = 0;
 
             /* now parse the chunked piece of data so that we can
                properly tell when the stream ends */
@@ -384,15 +391,17 @@ CURLcode Curl_proxyCONNECT(struct connectdata *conn,
               /* we did the full CONNECT treatment, go COMPLETE */
               conn->tunnel_state[sockindex] = TUNNEL_COMPLETE;
             }
-                else
-                  infof(data, "Read %zd bytes of chunk, continue\n",
-                        tookcareof);
           }
+          continue;
         }
-            else {
+
         perline++; /* amount of bytes in this line so far */
-              if(*ptr == 0x0a) {
-                int writetype;
+
+        /* if this is not the end of a header line then continue */
+        if(*ptr != 0x0a) {
+          ptr++;
+          continue;
+        }
 
         /* convert from the network encoding */
         result = Curl_convert_from_network(data, line_start, perline);
@@ -410,8 +419,7 @@ CURLcode Curl_proxyCONNECT(struct connectdata *conn,
         if(data->set.include_header)
           writetype |= CLIENTWRITE_BODY;
 
-                result = Curl_client_write(conn, writetype, line_start,
-                                           perline);
+        result = Curl_client_write(conn, writetype, line_start, perline);
 
         data->info.header_size += (long)perline;
         data->req.headerbytecount += (long)perline;
@@ -428,7 +436,7 @@ CURLcode Curl_proxyCONNECT(struct connectdata *conn,
           /* end of response-headers from the proxy */
           nread = 0; /* make next read start over in the read
                         buffer */
-                  ptr=data->state.buffer;
+          ptr = data->state.buffer;
           if((407 == k->httpcode) && !data->state.authproblem) {
             /* If we get a 407 response code with content length
                when we have no auth problem, we must ignore the
@@ -441,6 +449,9 @@ CURLcode Curl_proxyCONNECT(struct connectdata *conn,
             }
             else if(chunked_encoding) {
               CHUNKcode r;
+
+              infof(data, "Ignore chunked response-body\n");
+
               /* We set ignorebody true here since the chunked
                  decoder function will acknowledge that. Pay
                  attention so that this is cleared again when this
@@ -455,8 +466,7 @@ CURLcode Curl_proxyCONNECT(struct connectdata *conn,
 
               /* now parse the chunked piece of data so that we can
                  properly tell when the stream ends */
-                      r = Curl_httpchunk_read(conn, line_start+1, 1,
-                                              &gotbytes);
+              r = Curl_httpchunk_read(conn, line_start + 1, 1, &gotbytes);
               if(r == CHUNKE_STOP) {
                 /* we're done reading chunks! */
                 infof(data, "chunk reading DONE\n");
@@ -465,25 +475,22 @@ CURLcode Curl_proxyCONNECT(struct connectdata *conn,
                    COMPLETE */
                 conn->tunnel_state[sockindex] = TUNNEL_COMPLETE;
               }
-                      else
-                        infof(data, "Read %zd bytes of chunk, continue\n",
-                              gotbytes);
             }
             else {
               /* without content-length or chunked encoding, we
                  can't keep the connection alive since the close is
                  the end signal so we bail out at once instead */
-                      keepon=FALSE;
+              keepon = FALSE;
             }
           }
           else
             keepon = FALSE;
           /* we did the full CONNECT treatment, go to COMPLETE */
           conn->tunnel_state[sockindex] = TUNNEL_COMPLETE;
-                  break; /* breaks out of for-loop, not switch() */
+          continue;
         }
 
-                line_start[perline]=0; /* zero terminate the buffer */
+        line_start[perline] = 0; /* zero terminate the buffer */
         if((checkprefix("WWW-Authenticate:", line_start) &&
             (401 == k->httpcode)) ||
            (checkprefix("Proxy-authenticate:", line_start) &&
@@ -514,8 +521,7 @@ CURLcode Curl_proxyCONNECT(struct connectdata *conn,
           cl = curlx_strtoofft(line_start +
                                strlen("Content-Length:"), NULL, 10);
         }
-                else if(Curl_compareheader(line_start,
-                                           "Connection:", "close"))
+        else if(Curl_compareheader(line_start, "Connection:", "close"))
           closeConnection = TRUE;
         else if(Curl_compareheader(line_start,
                                    "Transfer-Encoding:",
@@ -524,8 +530,7 @@ CURLcode Curl_proxyCONNECT(struct connectdata *conn,
             /* A server MUST NOT send any Transfer-Encoding or
                Content-Length header fields in a 2xx (Successful)
                response to CONNECT. (RFC 7231 section 4.3.6) */
-                    failf(data, "Transfer-Encoding: in %03d response",
-                          k->httpcode);
+            failf(data, "Transfer-Encoding: in %03d response", k->httpcode);
             return CURLE_RECV_ERROR;
           }
           infof(data, "CONNECT responded chunked\n");
@@ -533,8 +538,7 @@ CURLcode Curl_proxyCONNECT(struct connectdata *conn,
           /* init our chunky engine */
           Curl_httpchunk_init(conn);
         }
-                else if(Curl_compareheader(line_start,
-                                           "Proxy-Connection:", "close"))
+        else if(Curl_compareheader(line_start, "Proxy-Connection:", "close"))
           closeConnection = TRUE;
         else if(2 == sscanf(line_start, "HTTP/1.%d %d",
                             &subversion,
@@ -543,19 +547,13 @@ CURLcode Curl_proxyCONNECT(struct connectdata *conn,
           data->info.httpproxycode = k->httpcode;
         }
 
-                perline=0; /* line starts over here */
+        perline = 0; /* line starts over here */
         ptr = data->state.buffer;
         line_start = ptr;
-              }
-              else /* not end of header line */
-                ptr++;
-            }
-          }
-          break;
-        } /* switch */
+      } /* while there's buffer left and loop is requested */
+
       if(Curl_pgrsUpdate(conn))
         return CURLE_ABORTED_BY_CALLBACK;
-      } /* while there's buffer left and loop is requested */
 
       if(error)
         return CURLE_RECV_ERROR;
@@ -570,8 +568,7 @@ CURLcode Curl_proxyCONNECT(struct connectdata *conn,
         if(conn->bits.close)
           /* the connection has been marked for closure, most likely in the
              Curl_http_auth_act() function and thus we can kill it at once
-             below
-          */
+             below */
           closeConnection = TRUE;
       }
 

lib/sendf.c

@@ -122,6 +122,13 @@ static size_t convert_lineends(struct Curl_easy *data,
 #endif /* CURL_DO_LINEEND_CONV */
 
 #ifdef USE_RECV_BEFORE_SEND_WORKAROUND
+bool Curl_recv_has_postponed_data(struct connectdata *conn, int sockindex)
+{
+  struct postponed_data * const psnd = &(conn->postponed[sockindex]);
+  return psnd->buffer && psnd->allocated_size &&
+         psnd->recv_size > psnd->recv_processed;
+}
+
 static void pre_receive_plain(struct connectdata *conn, int num)
 {
   const curl_socket_t sockfd = conn->sock[num];
@@ -201,6 +208,10 @@ static ssize_t get_pre_recved(struct connectdata *conn, int num, char *buf,
 }
 #else  /* ! USE_RECV_BEFORE_SEND_WORKAROUND */
 /* Use "do-nothing" macros instead of functions when workaround not used */
+bool Curl_recv_has_postponed_data(struct connectdata *conn, int sockindex)
+{
+  return false;
+}
 #define pre_receive_plain(c,n) do {} WHILE_FALSE
 #define get_pre_recved(c,n,b,l) 0
 #endif /* ! USE_RECV_BEFORE_SEND_WORKAROUND */

lib/sendf.h

@@ -56,6 +56,8 @@ CURLcode Curl_client_chop_write(struct connectdata *conn, int type, char *ptr,
 CURLcode Curl_client_write(struct connectdata *conn, int type, char *ptr,
                            size_t len) WARN_UNUSED_RESULT;
 
+bool Curl_recv_has_postponed_data(struct connectdata *conn, int sockindex);
+
 /* internal read-function, does plain socket only */
 CURLcode Curl_read_plain(curl_socket_t sockfd,
                          char *buf,