HTTP client: Fix GET request handling when rctx is reused (keep-alive)

This also updates the documentation of OSSL_HTTP_REQ_CTX_set1_req().

Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/15697)

2 files changed, 27 insertions, 24 deletions

diff --git a/crypto/http/http_client.c b/crypto/http/http_client.c
index 79fe9ccd41..0906818451 100644
--- a/crypto/http/http_client.c
+++ b/crypto/http/http_client.c
@@ -200,9 +200,13 @@ int OSSL_HTTP_REQ_CTX_set_request_line(OSSL_HTTP_REQ_CTX *rctx, int method_POST,
         path = "/";
     if (path[0] != '/' && BIO_printf(rctx->mem, "/") <= 0)
         return 0;
-
+    /*
+     * Add (the rest of) the path and the HTTP version,
+     * which is fixed to 1.0 for straightforward implementation of keep-alive
+     */
     if (BIO_printf(rctx->mem, "%s "HTTP_1_0"\r\n", path) <= 0)
         return 0;
+
     rctx->resp_len = 0;
     rctx->state = OHS_ADD_HEADERS;
     return 1;
@@ -275,6 +279,8 @@ static int set1_content(OSSL_HTTP_REQ_CTX *rctx,
             && !OSSL_HTTP_REQ_CTX_add1_header(rctx, "Connection", "keep-alive"))
         return 0;
 
+    BIO_free(rctx->req);
+    rctx->req = NULL;
     if (req == NULL)
         return 1;
     if (!rctx->method_POST) {
@@ -287,11 +293,9 @@ static int set1_content(OSSL_HTTP_REQ_CTX *rctx,
         return 0;
 
     /* streaming BIO may not support querying size */
-    if ((req_len = BIO_ctrl(req, BIO_CTRL_INFO, 0, NULL)) <= 0
-        || BIO_printf(rctx->mem, "Content-Length: %ld\r\n", req_len) > 0) {
-        if (!BIO_up_ref(req))
-            return 0;
-        BIO_free(rctx->req);
+    if (((req_len = BIO_ctrl(req, BIO_CTRL_INFO, 0, NULL)) <= 0
+         || BIO_printf(rctx->mem, "Content-Length: %ld\r\n", req_len) > 0)
+        && BIO_up_ref(req)) {
         rctx->req = req;
         return 1;
     }
@@ -301,16 +305,12 @@ static int set1_content(OSSL_HTTP_REQ_CTX *rctx,
 int OSSL_HTTP_REQ_CTX_set1_req(OSSL_HTTP_REQ_CTX *rctx, const char *content_type,
                                const ASN1_ITEM *it, const ASN1_VALUE *req)
 {
-    BIO *mem;
-    int res;
-
-    if (rctx == NULL || it == NULL || req == NULL) {
-        ERR_raise(ERR_LIB_HTTP, ERR_R_PASSED_NULL_PARAMETER);
-        return 0;
-    }
+    BIO *mem = NULL;
+    int res = 1;
 
-    res = (mem = ASN1_item_i2d_mem_bio(it, req)) != NULL
-        && set1_content(rctx, content_type, mem);
+    if (req != NULL)
+        res = (mem = ASN1_item_i2d_mem_bio(it, req)) != NULL;
+    res = res && set1_content(rctx, content_type, mem);
     BIO_free(mem);
     return res;
 }
diff --git a/doc/man3/OSSL_HTTP_REQ_CTX.pod b/doc/man3/OSSL_HTTP_REQ_CTX.pod
index ec358d265f..c1cf9ad87b 100644
--- a/doc/man3/OSSL_HTTP_REQ_CTX.pod
+++ b/doc/man3/OSSL_HTTP_REQ_CTX.pod
@@ -111,12 +111,16 @@ If the value is 1 or 2 then a persistent connection is requested.
 If the value is 2 then a persistent connection is required,
 i.e., an error occurs in case the server does not grant it.
 
-OSSL_HTTP_REQ_CTX_set1_req() is to be used if and only if the I<method_POST>
-parameter in the OSSL_HTTP_REQ_CTX_set_request_line() call was 1
-and an ASN.1-encoded request should be sent, which does not support streaming.
-It finalizes the HTTP request context by adding the DER encoding of I<req>,
-using the ASN.1 template I<it> to do the encoding.
+OSSL_HTTP_REQ_CTX_set1_req() finalizes the HTTP request context.
+It is needed if the I<method_POST> parameter in the
+OSSL_HTTP_REQ_CTX_set_request_line() call was 1
+and an ASN.1-encoded request should be sent.
+It must also be used when requesting "keep-alive",
+even if a GET request is going to be sent, in which case I<req> must be NULL.
+Unless I<req> is NULL, the function adds the DER encoding of I<req> using
+the ASN.1 template I<it> to do the encoding (which does not support streaming).
 The HTTP header C<Content-Length> is filled out with the length of the request.
+I<content_type> must be NULL if I<req> is NULL.
 If I<content_type> isn't NULL,
 the HTTP header C<Content-Type> is also added with the given string value.
 All of this ends up in the internal memory B<BIO>.
@@ -188,7 +192,7 @@ Then, the HTTP request must be prepared with request data:
 
 =item 1.
 
-Calling OSSL_HTTP_REQ_CTX_set_request_line().  This must be done exactly once.
+Calling OSSL_HTTP_REQ_CTX_set_request_line().
 
 =item 2.
 
@@ -197,9 +201,8 @@ This is optional and may be done multiple times with different names.
 
 =item 3.
 
-Add C<POST> data with OSSL_HTTP_REQ_CTX_set1_req().  This may only be done if
-I<method_POST> was 1 in the OSSL_HTTP_REQ_CTX_set_request_line() call,
-and must be done exactly once in that case.
+Finalize the request using OSSL_HTTP_REQ_CTX_set1_req().
+This may be omitted if the GET method is used and "keep-alive" is not requested.
 
 =back