hsa: make http client do chunked reads/writes to file

When using the save-to arg, the http client saves the body of the
response to file. With the current mechanism it would allocate a buffer
as big as the body (up to the limit), iteratively fill it and dump
everything (along with the headers) into the file. This would limit how
big of a response can be saved due to memory constraints and settings,
as well as not reproduce it accurately (e.g the file would need to be
trimmed from the saved headers.

With the new approach, if the response is too big for the max-body-size
settings, we reduce the buffer size to the fifo size, fill it up and
write it to file. We keep the file pointer and write to it, until we have
the response fully saved. The headers are now being displayed through
the cli, similarly to the verbose mode.

Type: improvement
Change-Id: I6a72749bc9c1175aba7769d83984d1d4a40ee9f0
Signed-off-by: Semir Sionek <[email protected]>

diff --git a/extras/hs-test/http_test.go b/extras/hs-test/http_test.go
index 88cb886..6047270 100644 (file)
--- a/extras/hs-test/http_test.go
+++ b/extras/hs-test/http_test.go
@@ -38,7 +38,7 @@ func init() {
                HttpRequestLineTest, HttpClientGetTimeout, HttpStaticFileHandlerWrkTest, HttpStaticUrlHandlerWrkTest, HttpConnTimeoutTest,
                HttpClientGetRepeatTest, HttpClientPostRepeatTest, HttpIgnoreH2UpgradeTest, HttpInvalidAuthorityFormUriTest, HttpHeaderErrorConnectionDropTest,
                HttpClientInvalidHeaderNameTest, HttpStaticHttp1OnlyTest, HttpTimerSessionDisable, HttpClientBodySizeTest,
-               HttpStaticRedirectTest, HttpClientNoPrintTest)
+               HttpStaticRedirectTest, HttpClientNoPrintTest, HttpClientChunkedDownloadTest)
        RegisterNoTopoSoloTests(HttpStaticPromTest, HttpGetTpsTest, HttpGetTpsInterruptModeTest, PromConcurrentConnectionsTest,
                PromMemLeakTest, HttpClientPostMemLeakTest, HttpInvalidClientRequestMemLeakTest, HttpPostTpsTest, HttpPostTpsInterruptModeTest,
                PromConsecutiveConnectionsTest, HttpGetTpsTlsTest, HttpPostTpsTlsTest)
@@ -339,6 +339,31 @@ func HttpClientTest(s *NoTopoSuite) {
        s.AssertContains(o, "</html>", "</html> not found in the result!")
 }
 
+func HttpClientChunkedDownloadTest(s *NoTopoSuite) {
+       serverAddress := s.HostAddr() + ":" + s.Ports.Http
+       server := ghttp.NewUnstartedServer()
+       l, err := net.Listen("tcp", serverAddress)
+       s.AssertNil(err, fmt.Sprint(err))
+       server.HTTPTestServer.Listener = l
+       response := strings.Repeat("a", 128*1024)
+       server.AppendHandlers(
+               ghttp.CombineHandlers(
+                       s.LogHttpReq(true),
+                       ghttp.VerifyRequest("GET", "/"),
+                       ghttp.RespondWith(http.StatusOK, response, http.Header{"Content-Length": {strconv.Itoa(len(response))}}),
+               ))
+       server.Start()
+       defer server.Close()
+       uri := "http://" + serverAddress
+       vpp := s.Containers.Vpp.VppInstance
+       o := vpp.Vppctl("http client save-to response.txt fifo-size 64k max-body-size 64k uri " + uri)
+
+       s.Log(o)
+       file_contents, err := vpp.Container.Exec(false, "cat /tmp/response.txt")
+       s.AssertNil(err)
+       s.AssertContains(file_contents, response)
+}
+
 func HttpClientBodySizeTest(s *NoTopoSuite) {
        serverAddress := s.HostAddr() + ":" + s.Ports.Http
        server := ghttp.NewUnstartedServer()
@@ -358,7 +383,7 @@ func HttpClientBodySizeTest(s *NoTopoSuite) {
        o := vpp.Vppctl("http client max-body-size 5 verbose uri " + uri)
 
        s.Log(o)
-       s.AssertContains(o, "* message body over limit", "message body size info not found in result!")
+       s.AssertContains(o, "* response body over limit", "response body size info not found in result!")
        s.AssertContains(o, ", read total 38 bytes", "client retrieved invalid amount of bytes!")
 }
 
@@ -544,7 +569,6 @@ func httpClientGet(s *NoTopoSuite, response string, size int, proto string) {
                s.Log(o)
        }
        s.AssertContains(o, "200 OK")
-       s.AssertContains(o, response)
        s.AssertContains(o, "Content-Length: "+strconv.Itoa(size))
 
        file_contents, err := vpp.Container.Exec(false, "cat /tmp/response.txt")
@@ -608,7 +632,6 @@ func httpClientGet6(s *NoTopo6Suite, response string, size int, proto string) {
        o := vpp.Vppctl(cmd)
        s.Log(o)
        s.AssertContains(o, "200 OK")
-       s.AssertContains(o, response)
        s.AssertContains(o, "Content-Length: "+strconv.Itoa(size))
 
        file_contents, err := vpp.Container.Exec(false, "cat /tmp/response.txt")

diff --git a/src/plugins/hs_apps/http_client.c b/src/plugins/hs_apps/http_client.c
index a284da8..6e52100 100644 (file)
--- a/src/plugins/hs_apps/http_client.c
+++ b/src/plugins/hs_apps/http_client.c
@@ -14,7 +14,7 @@
 #define foreach_hc_s_flag                                                     \
   _ (1, IS_CLOSED)                                                            \
   _ (2, PRINTABLE_BODY)                                                       \
-  _ (4, BODY_OVER_LIMIT)
+  _ (4, CHUNKED_BODY)
 
 typedef enum hc_s_flag_
 {
@@ -44,6 +44,7 @@ typedef struct
   u8 *resp_headers;
   u8 *http_response;
   u8 *response_status;
+  FILE *file_ptr;
 } hc_session_t;
 
 typedef struct
@@ -107,6 +108,7 @@ typedef enum
   HC_TRANSPORT_CLOSED,
   HC_REPLY_RECEIVED,
   HC_GENERIC_ERR,
+  HC_FOPEN_FAILED,
   HC_REPEAT_DONE,
 } hc_cli_signal_t;
 
@@ -263,6 +265,17 @@ hc_session_connected_callback (u32 app_index, u32 hc_session_index,
       hc_session->stats.req_per_wrk = hcm->repeat_count;
       hcm->worker_index = s->thread_index;
     }
+  if (hcm->filename)
+    {
+      hc_session->file_ptr =
+       fopen ((char *) format (0, "/tmp/%v", hcm->filename), "w");
+      if (hc_session->file_ptr == NULL)
+       {
+         vlib_process_signal_event_mt (wrk->vlib_main, hcm->cli_node_index,
+                                       HC_FOPEN_FAILED, 0);
+         return -1;
+       }
+    }
 
   if (!wrk->has_common_headers)
     {
@@ -478,10 +491,11 @@ hc_rx_callback (session_t *s)
        {
          goto done;
        }
-      if (msg.data.body_len > hcm->max_body_size)
-       hc_session->session_flags |= HC_S_FLAG_BODY_OVER_LIMIT;
+
+      if (msg.data.body_len > hcm->max_body_size || hcm->filename)
+       hc_session->session_flags |= HC_S_FLAG_CHUNKED_BODY;
       vec_validate (hc_session->http_response,
-                   (hc_session->session_flags & HC_S_FLAG_BODY_OVER_LIMIT ?
+                   (hc_session->session_flags & HC_S_FLAG_CHUNKED_BODY ?
                       hcm->rx_fifo_size - 1 :
                       msg.data.body_len - 1));
       vec_reset_length (hc_session->http_response);
@@ -506,11 +520,22 @@ hc_rx_callback (session_t *s)
     }
 
   ASSERT (rv == n_deq);
-  if (!(hc_session->session_flags & HC_S_FLAG_BODY_OVER_LIMIT))
+  if (!(hc_session->session_flags & HC_S_FLAG_CHUNKED_BODY))
     vec_set_len (hc_session->http_response, curr + n_deq);
   ASSERT (hc_session->to_recv >= rv);
   hc_session->to_recv -= rv;
   hc_session->body_recv += rv;
+  if (hcm->filename)
+    {
+      if (hc_session->file_ptr == NULL)
+       {
+         vlib_process_signal_event_mt (wrk->vlib_main, hcm->cli_node_index,
+                                       HC_FOPEN_FAILED, 0);
+         goto done;
+       }
+      fwrite (hc_session->http_response, sizeof (u8), rv,
+             hc_session->file_ptr);
+    }
 
 done:
   if (hc_session->to_recv == 0)
@@ -734,7 +759,6 @@ hc_get_event (vlib_main_t *vm)
   hc_main_t *hcm = &hc_main;
   uword event_type, *event_data = 0;
   clib_error_t *err = NULL;
-  FILE *file_ptr;
   u64 event_timeout;
   hc_worker_t *wrk;
   hc_session_t *hc_session;
@@ -760,35 +784,31 @@ hc_get_event (vlib_main_t *vm)
     case HC_GENERIC_ERR:
       err = clib_error_return (0, "error: unknown");
       break;
+    case HC_FOPEN_FAILED:
+      err = clib_error_return (0, "* couldn't open file %v", hcm->filename);
+      break;
     case HC_REPLY_RECEIVED:
       if (hcm->filename)
        {
          wrk = hc_worker_get (hcm->worker_index);
          hc_session = hc_session_get (wrk->session_index, wrk->thread_index);
-         file_ptr =
-           fopen ((char *) format (0, "/tmp/%v", hcm->filename), "a");
-         if (file_ptr == NULL)
-           {
-             vlib_cli_output (vm, "* couldn't open file %v", hcm->filename);
-           }
-         else
-           {
-             fprintf (file_ptr, "< %s\n< %s\n< %s",
-                      hc_session->response_status, hc_session->resp_headers,
-                      hc_session->http_response);
-             fclose (file_ptr);
-             vlib_cli_output (vm, "* file saved (/tmp/%v)", hcm->filename);
-           }
+         vlib_cli_output (
+           vm, "< %s\n< %s\n* %u bytes saved to file (/tmp/%s)",
+           hc_session->response_status, hc_session->resp_headers,
+           hc_session->body_recv, hcm->filename);
+         fclose (hc_session->file_ptr);
        }
-      if (hcm->verbose)
+      else if (hcm->verbose)
        {
          wrk = hc_worker_get (hcm->worker_index);
          hc_session = hc_session_get (wrk->session_index, wrk->thread_index);
          vlib_cli_output (vm, "< %v\n< %v\n%v", hc_session->response_status,
                           hc_session->resp_headers);
-         if (hc_session->session_flags & HC_S_FLAG_BODY_OVER_LIMIT)
+         /* if the body was read in chunks and not saved to file - that
+            means we've hit the response body size limit */
+         if (hc_session->session_flags & HC_S_FLAG_CHUNKED_BODY)
            vlib_cli_output (
-             vm, "* message body over limit, read total %llu bytes",
+             vm, "* response body over limit, read total %llu bytes",
              hc_session->body_recv);
          else
            {
@@ -931,10 +951,12 @@ hc_command_fn (vlib_main_t *vm, unformat_input_t *input,
   hcm->private_segment_size = 0;
   hcm->fifo_size = 0;
   hcm->was_transport_closed = false;
+  hcm->verbose = false;
   /* default max - 64MB */
   hcm->max_body_size = 64 << 20;
   hc_stats.request_count = 0;
   hc_stats.elapsed_time = 0;
+  vec_free (hcm->filename);
 
   if (hcm->attached)
     return clib_error_return (0, "failed: already running!");