http: revisit http_perhapsrewind()

- use facilities provided by client readers better
- work also for non-uploading requests like GET/HEAD
- update documentation

Closes #13117

lib/cf-h1-proxy.c

@@ -237,6 +237,8 @@ static CURLcode start_CONNECT(struct Curl_cfilter *cf,
   http_minor = (cf->conn->http_proxy.proxytype == CURLPROXY_HTTP_1_0) ? 0 : 1;
 
   result = Curl_h1_req_write_head(req, http_minor, &ts->request_data);
+  if(!result)
+    result = Curl_creader_set_null(data);
 
 out:
   if(result)
@@ -749,6 +751,10 @@ static CURLcode start_CONNECT(struct Curl_cfilter *cf,
   if(result)
     goto error;
 
+  result = Curl_creader_set_null(data);
+  if(result)
+    goto error;
+
   sendtask = hyper_clientconn_send(client, req);
   if(!sendtask) {
     failf(data, "hyper_clientconn_send");

lib/cf-h2-proxy.c

@@ -38,6 +38,7 @@
 #include "http2.h"
 #include "http_proxy.h"
 #include "multiif.h"
+#include "sendf.h"
 #include "cf-h2-proxy.h"
 
 /* The last 3 #include files should be in this order */
@@ -954,6 +955,9 @@ static CURLcode submit_CONNECT(struct Curl_cfilter *cf,
   struct httpreq *req = NULL;
 
   result = Curl_http_proxy_create_CONNECT(&req, cf, data, 2);
+  if(result)
+    goto out;
+  result = Curl_creader_set_null(data);
   if(result)
     goto out;
 
@@ -1125,7 +1129,12 @@ static CURLcode cf_h2_proxy_connect(struct Curl_cfilter *cf,
 
 out:
   *done = (result == CURLE_OK) && (ts->state == H2_TUNNEL_ESTABLISHED);
-  cf->connected = *done;
+  if(*done) {
+    cf->connected = TRUE;
+    /* The real request will follow the CONNECT, reset request partially */
+    Curl_req_soft_reset(&data->req, data);
+    Curl_client_reset(data);
+  }
   CF_DATA_RESTORE(cf, save);
   return result;
 }

lib/http.c

@@ -405,123 +405,88 @@ static bool pickoneauth(struct auth *pick, unsigned long mask)
 /*
  * http_perhapsrewind()
  *
- * If we are doing POST or PUT {
- *   If we have more data to send {
- *     If we are doing NTLM {
- *       Keep sending since we must not disconnect
- *     }
- *     else {
- *       If there is more than just a little data left to send, close
- *       the current connection by force.
- *     }
- *   }
- *   If we have sent any data {
- *     If we don't have track of all the data {
- *       call app to tell it to rewind
- *     }
- *     else {
- *       rewind internally so that the operation can restart fine
- *     }
- *   }
- * }
+ * The current request needs to be done again - maybe due to a follow
+ * or authentication negotiation. Check if:
+ * 1) a rewind of the data sent to the server is necessary
+ * 2) the current transfer should continue or be stopped early
  */
 static CURLcode http_perhapsrewind(struct Curl_easy *data,
                                    struct connectdata *conn)
 {
-  struct HTTP *http = data->req.p.http;
-  curl_off_t bytessent;
+  curl_off_t bytessent = data->req.writebytecount;
   curl_off_t expectsend = Curl_creader_total_length(data);
+  curl_off_t upload_remain = (expectsend >= 0)? (expectsend - bytessent) : -1;
+  bool little_upload_remains = (upload_remain >= 0 && upload_remain < 2000);
+  bool needs_rewind = Curl_creader_needs_rewind(data);
+  /* By default, we'd like to abort the transfer when little or
+   * unknown amount remains. But this may be overridden by authentications
+   * further below! */
+  bool abort_upload = (!data->req.upload_done && !little_upload_remains);
+  const char *ongoing_auth = NULL;
+
+  /* We need a rewind before uploading client read data again. The
+   * checks below just influence of the upload is to be continued
+   * or aborted early.
+   * This depends on how much remains to be sent and in what state
+   * the authentication is. Some auth schemes such as NTLM do not work
+   * for a new connection. */
+  if(needs_rewind) {
+    infof(data, "Need to rewind upload for next request");
+    Curl_creader_set_rewind(data, TRUE);
+  }
 
-  if(!http)
-    /* If this is still NULL, we have not reach very far and we can safely
-       skip this rewinding stuff */
-    return CURLE_OK;
-
-  if(!expectsend)
-    /* not sending any body */
+  if(conn->bits.close)
+    /* If we already decided to close this connection, we cannot veto. */
     return CURLE_OK;
 
-  if(!conn->bits.protoconnstart)
-    /* HTTP CONNECT in progress: there is no body */
-    expectsend = 0;
-
-  bytessent = data->req.writebytecount;
-  Curl_creader_set_rewind(data, FALSE);
-
-  if((expectsend == -1) || (expectsend > bytessent)) {
+  if(abort_upload) {
+    /* We'd like to abort the upload - but should we? */
 #if defined(USE_NTLM)
-    /* There is still data left to send */
     if((data->state.authproxy.picked == CURLAUTH_NTLM) ||
        (data->state.authhost.picked == CURLAUTH_NTLM) ||
        (data->state.authproxy.picked == CURLAUTH_NTLM_WB) ||
        (data->state.authhost.picked == CURLAUTH_NTLM_WB)) {
-      if(((expectsend - bytessent) < 2000) ||
-         (conn->http_ntlm_state != NTLMSTATE_NONE) ||
+      ongoing_auth = "NTML";
+      if((conn->http_ntlm_state != NTLMSTATE_NONE) ||
          (conn->proxy_ntlm_state != NTLMSTATE_NONE)) {
-        /* The NTLM-negotiation has started *OR* there is just a little (<2K)
-           data left to send, keep on sending. */
-
-        /* rewind data when completely done sending! */
-        if(!data->req.authneg && (conn->writesockfd != CURL_SOCKET_BAD)) {
-          Curl_creader_set_rewind(data, TRUE);
-          infof(data, "Rewind stream before next send");
-        }
-
-        return CURLE_OK;
+        /* The NTLM-negotiation has started, keep on sending.
+         * Need to do further work on same connection */
+        abort_upload = FALSE;
       }
-
-      if(conn->bits.close)
-        /* this is already marked to get closed */
-        return CURLE_OK;
-
-      infof(data, "NTLM send, close instead of sending %"
-            CURL_FORMAT_CURL_OFF_T " bytes",
-            (curl_off_t)(expectsend - bytessent));
     }
 #endif
 #if defined(USE_SPNEGO)
     /* There is still data left to send */
     if((data->state.authproxy.picked == CURLAUTH_NEGOTIATE) ||
        (data->state.authhost.picked == CURLAUTH_NEGOTIATE)) {
-      if(((expectsend - bytessent) < 2000) ||
-         (conn->http_negotiate_state != GSS_AUTHNONE) ||
+      ongoing_auth = "NEGOTIATE";
+      if((conn->http_negotiate_state != GSS_AUTHNONE) ||
          (conn->proxy_negotiate_state != GSS_AUTHNONE)) {
-        /* The NEGOTIATE-negotiation has started *OR*
-        there is just a little (<2K) data left to send, keep on sending. */
-
-        /* rewind data when completely done sending! */
-        if(!data->req.authneg && (conn->writesockfd != CURL_SOCKET_BAD)) {
-          Curl_creader_set_rewind(data, TRUE);
-          infof(data, "Rewind stream before next send");
-        }
-
-        return CURLE_OK;
+        /* The NEGOTIATE-negotiation has started, keep on sending.
+         * Need to do further work on same connection */
+        abort_upload = FALSE;
       }
-
-      if(conn->bits.close)
-        /* this is already marked to get closed */
-        return CURLE_OK;
-
-      infof(data, "NEGOTIATE send, close instead of sending %"
-        CURL_FORMAT_CURL_OFF_T " bytes",
-        (curl_off_t)(expectsend - bytessent));
     }
 #endif
+  }
 
-    /* This is not NEGOTIATE/NTLM or many bytes left to send: close */
+  if(abort_upload) {
+    if(upload_remain >= 0)
+      infof(data, "%s%sclose instead of sending %"
+        CURL_FORMAT_CURL_OFF_T " more bytes",
+        ongoing_auth? ongoing_auth : "",
+        ongoing_auth? " send, " : "",
+        upload_remain);
+    else
+      infof(data, "%s%sclose instead of sending unknown amount "
+        "of more bytes",
+        ongoing_auth? ongoing_auth : "",
+        ongoing_auth? " send, " : "");
+    /* We decided to abort the ongoing transfer */
     streamclose(conn, "Mid-auth HTTP and much data left to send");
+    /* FIXME: questionable manipulation here, can we do this differently? */
     data->req.size = 0; /* don't download any more than 0 bytes */
-
-    /* There still is data left to send, but this connection is marked for
-       closure so we can safely do the rewind right now */
-  }
-
-  if(Curl_creader_needs_rewind(data)) {
-    /* mark for rewind since if we already sent something */
-    Curl_creader_set_rewind(data, TRUE);
-    infof(data, "Please rewind output before next send");
   }
-
   return CURLE_OK;
 }
 
@@ -575,13 +540,10 @@ CURLcode Curl_http_auth_act(struct Curl_easy *data)
 #endif
 
   if(pickhost || pickproxy) {
-    if((data->state.httpreq != HTTPREQ_GET) &&
-       (data->state.httpreq != HTTPREQ_HEAD) &&
-       !Curl_creader_will_rewind(data)) {
-      result = http_perhapsrewind(data, conn);
-      if(result)
-        return result;
-    }
+    result = http_perhapsrewind(data, conn);
+    if(result)
+      return result;
+
     /* In case this is GSS auth, the newurl field is already allocated so
        we must make sure to free it before allocating a new one. As figured
        out in bug #2284386 */