http_client.c: fix OSSL_HTTP_REQ_CTX_nbio() to return content on non-fatal HTTP statu...

Reviewed-by: Dmitry Belyavskiy <beldmit@gmail.com>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/25541)

diff --git a/crypto/http/http_client.c b/crypto/http/http_client.c
index e23160fe3f6cd6b0daa6a92b3d396eaa6e93b4b1..95dabc8ec54d5cdef511b093b60612eea03fe6f3 100644 (file)
--- a/crypto/http/http_client.c
+++ b/crypto/http/http_client.c
@@ -35,6 +35,7 @@
 #define HTTP_STATUS_CODE_OK                200
 #define HTTP_STATUS_CODE_MOVED_PERMANENTLY 301
 #define HTTP_STATUS_CODE_FOUND             302
+#define HTTP_STATUS_CODES_NONFATAL_ERROR   400
 
 /* Stateful HTTP request code, supporting blocking and non-blocking I/O */
 
@@ -431,7 +432,7 @@ static OSSL_HTTP_REQ_CTX *http_req_ctx_new(int free_wbio, BIO *wbio, BIO *rbio,
 
 static int parse_http_line1(char *line, int *found_keep_alive)
 {
-    int i, retcode, err;
+    int i, retcode;
     char *code, *reason, *end;
 
     if (!CHECK_AND_SKIP_PREFIX(line, HTTP_PREFIX_VERSION))
@@ -487,14 +488,11 @@ static int parse_http_line1(char *line, int *found_keep_alive)
     case HTTP_STATUS_CODE_FOUND:
         return retcode;
     default:
-        err = HTTP_R_RECEIVED_ERROR;
-        if (retcode < 400)
-            err = HTTP_R_STATUS_CODE_UNSUPPORTED;
-        if (*reason == '\0')
-            ERR_raise_data(ERR_LIB_HTTP, err, "code=%s", code);
-        else
-            ERR_raise_data(ERR_LIB_HTTP, err, "code=%s, reason=%s", code,
-                           reason);
+        if (retcode < HTTP_STATUS_CODES_NONFATAL_ERROR) {
+            ERR_raise_data(ERR_LIB_HTTP, HTTP_R_STATUS_CODE_UNSUPPORTED, "code=%s", code);
+            if (*reason != '\0')
+                ERR_add_error_data(2, ", reason=", reason);
+        } /* must return content normally if status >= 400 */
         return retcode;
     }
 
@@ -752,7 +750,8 @@ int OSSL_HTTP_REQ_CTX_nbio(OSSL_HTTP_REQ_CTX *rctx)
 
         /* First line in response header */
         if (rctx->state == OHS_FIRSTLINE) {
-            switch (parse_http_line1(buf, &found_keep_alive)) {
+            i = parse_http_line1(buf, &found_keep_alive);
+            switch (i) {
             case HTTP_STATUS_CODE_OK:
                 rctx->state = OHS_HEADERS;
                 goto next_line;
@@ -766,8 +765,10 @@ int OSSL_HTTP_REQ_CTX_nbio(OSSL_HTTP_REQ_CTX *rctx)
                 /* redirection is not supported/recommended for POST */
                 /* fall through */
             default:
-                rctx->state = OHS_HEADERS_ERROR;
-                goto next_line; /* continue parsing and reporting header */
+                /* must return content if status >= 400 */
+                rctx->state = i < HTTP_STATUS_CODES_NONFATAL_ERROR
+                    ? OHS_HEADERS_ERROR : OHS_HEADERS;
+                goto next_line; /* continue parsing, also on HTTP error */
             }
         }
         key = buf;
@@ -864,6 +865,8 @@ int OSSL_HTTP_REQ_CTX_nbio(OSSL_HTTP_REQ_CTX *rctx)
                             got_text ? "text" : "ASN.1");
                 goto next_line;
             }
+            /* discard response content when trace not enabled */
+            (void)BIO_reset(rctx->rbio);
             return 0;
         }
 

diff --git a/test/http_test.c b/test/http_test.c
index eca2a9292214441c800106f19a407675c022ece5..67fed9a05ca1a84c372dcb5829bb4f21595ee7de 100644 (file)
--- a/test/http_test.c
+++ b/test/http_test.c
@@ -15,6 +15,10 @@
 
 #include "testutil.h"
 
+#define HTTP_STATUS_CODE_OK                200
+#define HTTP_STATUS_CODES_FATAL_ERROR      399
+#define HTTP_STATUS_CODES_NONFATAL_ERROR   400
+
 static const ASN1_ITEM *x509_it = NULL;
 static X509 *x509 = NULL;
 #define RPATH "/path"
@@ -32,6 +36,8 @@ typedef struct {
  * For POST, copy request headers+body from mem BIO |in| as response to |out|.
  * For GET, redirect to RPATH unless already there, else use |content_type| and
  * respond with |txt| if not NULL, else with |rsp| of ASN1 type |it|.
+ * Take the status code suggsted by the client via special prefix of the path.
+ * On fatal status, respond with empty content.
  * Response hdr has HTTP version 1.|version| and |keep_alive| (unless implicit).
  */
 static int mock_http_server(BIO *in, BIO *out, char version, int keep_alive,
@@ -40,15 +46,27 @@ static int mock_http_server(BIO *in, BIO *out, char version, int keep_alive,
 {
     const char *req, *path;
     long count = BIO_get_mem_data(in, (unsigned char **)&req);
-    const char *hdr = (char *)req;
+    const char *hdr = (char *)req, *suggested_status;
+    char status[4] = "200";
     int len;
     int is_get = count >= 4 && CHECK_AND_SKIP_PREFIX(hdr, "GET ");
 
-    /* first line should contain "(GET|POST) <path> HTTP/1.x" */
+    /* first line should contain "(GET|POST) (/<suggested status>)?/<path> HTTP/1.x" */
     if (!is_get
             && !(TEST_true(count >= 5 && CHECK_AND_SKIP_PREFIX(hdr, "POST "))))
         return 0;
 
+    /* get any status code string to be returned suggested by test client */
+    if (*hdr == '/') {
+        suggested_status = ++hdr;
+        while (*hdr >= '0' && *hdr <= '9')
+            hdr++;
+        if (hdr == suggested_status + sizeof(status) - 1)
+            strncpy(status, suggested_status, sizeof(status) - 1);
+        else
+            hdr = suggested_status - 1;
+    }
+
     path = hdr;
     hdr = strchr(hdr, ' ');
     if (hdr == NULL)
@@ -74,12 +92,19 @@ static int mock_http_server(BIO *in, BIO *out, char version, int keep_alive,
                           "Location: %s\r\n\r\n",
                           version, RPATH) > 0; /* same server */
     }
-    if (BIO_printf(out, "HTTP/1.%c 200 OK\r\n", version) <= 0)
+    if (BIO_printf(out, "HTTP/1.%c %s %s\r\n", version, status,
+                   /* mock some reason string: */
+                   strcmp(status, "200") == 0 ? "OK" :
+                   strcmp(status, "400") >= 0 ? "error" : "fatal") <= 0)
         return 0;
     if ((version == '0') == keep_alive) /* otherwise, default */
         if (BIO_printf(out, "Connection: %s\r\n",
                        version == '0' ? "keep-alive" : "close") <= 0)
             return 0;
+
+    if (strcmp(status, "399") == 0) /* HTTP_STATUS_CODES_FATAL_ERROR */
+        return BIO_puts(out, "\r\n") == 2; /* empty content */
+
     if (is_get) { /* construct new header and body */
         if (txt != NULL)
             len = strlen(txt);
@@ -122,7 +147,7 @@ static long http_bio_cb_ex(BIO *bio, int oper, const char *argp, size_t len,
 #define DOCTYPE_HTML "<!DOCTYPE html>\n"
 
 /* do_get > 1 used for testing redirection */
-static int test_http_method(int do_get, int do_txt)
+static int test_http_method(int do_get, int do_txt, int suggested_status)
 {
     BIO *wbio = BIO_new(BIO_s_mem());
     BIO *rbio = BIO_new(BIO_s_mem());
@@ -134,7 +159,7 @@ static int test_http_method(int do_get, int do_txt)
     int res = 0;
     int real_server = do_txt && 0; /* remove "&& 0" for using real server */
 
-    snprintf(path, sizeof(path), "%s",
+    snprintf(path, sizeof(path), "/%d%s", suggested_status,
              do_get > 1 ? "/will-be-redirected" : RPATH);
     if (do_txt) {
         content_type = "text/plain";
@@ -176,6 +201,10 @@ static int test_http_method(int do_get, int do_txt)
                              req, content_type, !do_txt /* expect_asn1 */,
                              OSSL_HTTP_DEFAULT_MAX_RESP_LEN, 0 /* timeout */,
                              0 /* keep_alive */);
+    if (!TEST_int_eq(suggested_status == HTTP_STATUS_CODES_FATAL_ERROR, rsp == NULL))
+        goto err;
+    if (suggested_status == HTTP_STATUS_CODES_FATAL_ERROR)
+        res = 1;
     if (rsp != NULL) {
         if (do_get && real_server) {
             char rtext[sizeof(DOCTYPE_HTML)];
@@ -362,32 +391,52 @@ static int test_http_url_invalid_path(void)
 
 static int test_http_get_txt(void)
 {
-    return test_http_method(1 /* GET */, 1);
+    return test_http_method(1 /* GET */, 1, HTTP_STATUS_CODE_OK);
 }
 
 static int test_http_get_txt_redirected(void)
 {
-    return test_http_method(2 /* GET with redirection */, 1);
+    return test_http_method(2 /* GET with redirection */, 1, HTTP_STATUS_CODE_OK);
+}
+
+static int test_http_get_txt_fatal_status(void)
+{
+    return test_http_method(1 /* GET */, 1, HTTP_STATUS_CODES_FATAL_ERROR);
+}
+
+static int test_http_get_txt_error_status(void)
+{
+    return test_http_method(1 /* GET */, 1, HTTP_STATUS_CODES_NONFATAL_ERROR);
 }
 
 static int test_http_post_txt(void)
 {
-    return test_http_method(0 /* POST */, 1);
+    return test_http_method(0 /* POST */, 1, HTTP_STATUS_CODE_OK);
 }
 
 static int test_http_get_x509(void)
 {
-    return test_http_method(1 /* GET */, 0);
+    return test_http_method(1 /* GET */, 0, HTTP_STATUS_CODE_OK);
 }
 
 static int test_http_get_x509_redirected(void)
 {
-    return test_http_method(2 /* GET with redirection */, 0);
+    return test_http_method(2 /* GET with redirection */, 0, HTTP_STATUS_CODE_OK);
 }
 
 static int test_http_post_x509(void)
 {
-    return test_http_method(0 /* POST */, 0);
+    return test_http_method(0 /* POST */, 0, HTTP_STATUS_CODE_OK);
+}
+
+static int test_http_post_x509_fatal_status(void)
+{
+    return test_http_method(0 /* POST */, 0, HTTP_STATUS_CODES_FATAL_ERROR);
+}
+
+static int test_http_post_x509_error_status(void)
+{
+    return test_http_method(0 /* POST */, 0, HTTP_STATUS_CODES_NONFATAL_ERROR);
 }
 
 static int test_http_keep_alive_0_no_no(void)
@@ -520,10 +569,15 @@ int setup_tests(void)
 
     ADD_TEST(test_http_get_txt);
     ADD_TEST(test_http_get_txt_redirected);
+    ADD_TEST(test_http_get_txt_fatal_status);
+    ADD_TEST(test_http_get_txt_error_status);
     ADD_TEST(test_http_post_txt);
     ADD_TEST(test_http_get_x509);
     ADD_TEST(test_http_get_x509_redirected);
     ADD_TEST(test_http_post_x509);
+    ADD_TEST(test_http_post_x509_fatal_status);
+    ADD_TEST(test_http_post_x509_error_status);
+
     ADD_TEST(test_http_keep_alive_0_no_no);
     ADD_TEST(test_http_keep_alive_1_no_no);
     ADD_TEST(test_http_keep_alive_0_prefer_yes);