Blob Blame History Raw
From f421e599d3507f22d3d06b2dab070811e7e4f41c Mon Sep 17 00:00:00 2001
From: "Richard W.M. Jones" <rjones@redhat.com>
Date: Sat, 28 Mar 2020 18:22:42 +0000
Subject: [PATCH 16/19] curl: Fix -D curl.verbose=1 option.

It didn't work previously for various reasons:

 - Passed int instead of long to curl_easy_setopt.

 - No CURLOPT_DEBUGFUNCTION callback was supplied, so messages were
   sent to stderr, which meant they were never logged for the majority
   of use cases.

This also removes extra debugging in the regular header callback.
This is no longer needed as the now-working -D curl.verbose=1 option
will log the headers.

Fixes commit 2ba11ee8f154ad1c84e10b43479b265fca2e996b.

(cherry picked from commit 6791c69bddf76577b65fa3ddfde652c0594ce340)
---
 plugins/curl/Makefile.am |  2 ++
 plugins/curl/curl.c      | 73 ++++++++++++++++++++++++++++++----------
 tests/test-curl.c        |  3 +-
 3 files changed, 60 insertions(+), 18 deletions(-)

diff --git a/plugins/curl/Makefile.am b/plugins/curl/Makefile.am
index 6595eb95..024ddb6d 100644
--- a/plugins/curl/Makefile.am
+++ b/plugins/curl/Makefile.am
@@ -44,6 +44,7 @@ nbdkit_curl_plugin_la_SOURCES = \
 
 nbdkit_curl_plugin_la_CPPFLAGS = \
 	-I$(top_srcdir)/include \
+	-I$(top_srcdir)/common/utils \
 	$(NULL)
 nbdkit_curl_plugin_la_CFLAGS = \
 	$(WARNINGS_CFLAGS) \
@@ -51,6 +52,7 @@ nbdkit_curl_plugin_la_CFLAGS = \
 	$(NULL)
 nbdkit_curl_plugin_la_LIBADD = \
 	$(CURL_LIBS) \
+	$(top_builddir)/common/utils/libutils.la \
 	$(NULL)
 nbdkit_curl_plugin_la_LDFLAGS = \
 	-module -avoid-version -shared \
diff --git a/plugins/curl/curl.c b/plugins/curl/curl.c
index 8b341ae0..b1693dc0 100644
--- a/plugins/curl/curl.c
+++ b/plugins/curl/curl.c
@@ -56,6 +56,8 @@
 
 #include <nbdkit-plugin.h>
 
+#include "cleanup.h"
+
 static const char *url = NULL;
 static const char *user = NULL;
 static char *password = NULL;
@@ -283,6 +285,8 @@ struct curl_handle {
                   curl_easy_strerror ((r)), (h)->errbuf);       \
   } while (0)
 
+static int debug_cb (CURL *handle, curl_infotype type,
+                     const char *data, size_t size, void *);
 static size_t header_cb (void *ptr, size_t size, size_t nmemb, void *opaque);
 static size_t write_cb (char *ptr, size_t size, size_t nmemb, void *opaque);
 static size_t read_cb (void *ptr, size_t size, size_t nmemb, void *opaque);
@@ -311,11 +315,13 @@ curl_open (int readonly)
     goto err;
   }
 
-  /* Note this writes the output to stderr directly.  We should
-   * consider using CURLOPT_DEBUGFUNCTION so we can handle it with
-   * nbdkit_debug.
-   */
-  curl_easy_setopt (h->c, CURLOPT_VERBOSE, curl_debug_verbose);
+  if (curl_debug_verbose) {
+    /* NB: Constants must be explicitly long because the parameter is
+     * varargs.
+     */
+    curl_easy_setopt (h->c, CURLOPT_VERBOSE, 1L);
+    curl_easy_setopt (h->c, CURLOPT_DEBUGFUNCTION, debug_cb);
+  }
 
   curl_easy_setopt (h->c, CURLOPT_ERRORBUFFER, h->errbuf);
 
@@ -441,12 +447,56 @@ curl_open (int readonly)
   return NULL;
 }
 
+/* When using CURLOPT_VERBOSE, this callback is used to redirect
+ * messages to nbdkit_debug (instead of stderr).
+ */
+static int
+debug_cb (CURL *handle, curl_infotype type,
+          const char *data, size_t size, void *opaque)
+{
+  size_t origsize = size;
+  CLEANUP_FREE char *str;
+
+  /* The data parameter passed is NOT \0-terminated, but also it may
+   * have \n or \r\n line endings.  The only sane way to deal with
+   * this is to copy the string.  (The data strings may also be
+   * multi-line, but we don't deal with that here).
+   */
+  str = malloc (size + 1);
+  if (str == NULL)
+    goto out;
+  memcpy (str, data, size);
+  str[size] = '\0';
+
+  while (size > 0 && (str[size-1] == '\n' || str[size-1] == '\r')) {
+    str[size-1] = '\0';
+    size--;
+  }
+
+  switch (type) {
+  case CURLINFO_TEXT:
+    nbdkit_debug ("%s", str);
+    break;
+  case CURLINFO_HEADER_IN:
+    nbdkit_debug ("S: %s", str);
+    break;
+  case CURLINFO_HEADER_OUT:
+    nbdkit_debug ("C: %s", str);
+    break;
+  default:
+    /* Assume everything else is binary data that we cannot print. */
+    nbdkit_debug ("<data with size=%zu>", origsize);
+  }
+
+ out:
+  return 0;
+}
+
 static size_t
 header_cb (void *ptr, size_t size, size_t nmemb, void *opaque)
 {
   struct curl_handle *h = opaque;
   size_t realsize = size * nmemb;
-  size_t len;
   const char *accept_line = "Accept-Ranges: bytes";
   const char *line = ptr;
 
@@ -454,17 +504,6 @@ header_cb (void *ptr, size_t size, size_t nmemb, void *opaque)
       strncmp (line, accept_line, strlen (accept_line)) == 0)
     h->accept_range = true;
 
-  /* Useful to print the server headers when debugging.  However we
-   * must strip off trailing \r?\n from each line.
-   */
-  len = realsize;
-  if (len > 0 && line[len-1] == '\n')
-    len--;
-  if (len > 0 && line[len-1] == '\r')
-    len--;
-  if (len > 0)
-    nbdkit_debug ("S: %.*s", (int) len, line);
-
   return realsize;
 }
 
diff --git a/tests/test-curl.c b/tests/test-curl.c
index 2b7e3beb..165edb35 100644
--- a/tests/test-curl.c
+++ b/tests/test-curl.c
@@ -74,9 +74,10 @@ main (int argc, char *argv[])
     exit (EXIT_FAILURE);
   }
   if (test_start_nbdkit ("curl",
+                         "-D", "curl.verbose=1",
+                         "http://localhost/disk",
                          "cookie=foo=bar; baz=1;",
                          usp_param, /* unix-socket-path=... */
-                         "http://localhost/disk",
                          NULL) == -1)
     exit (EXIT_FAILURE);
 
-- 
2.18.2