infof: clearly indicate truncation

The internal buffer in infof() is limited to 2048 bytes of payload plus
an additional byte for NULL termination. Servers with very long error
messages can however cause truncation of the string, which currently
isn't very clear, and leads to badly formatted output.

This appends a "...\n" (or just "..." in case the format didn't with a
newline char) marker to the end of the string to clearly show
that it has been truncated.

Also include a unittest covering infof() to try and catch any bugs
introduced in this quite important function.

Closes #3216
Reviewed-by: Daniel Stenberg <daniel@haxx.se>
Reviewed-by: Marcel Raad <Marcel.Raad@teamviewer.com>

diff --git a/lib/sendf.c b/lib/sendf.c
index d3c10b36907be8ca3aa89040e31bafe79ee88529..77eacdf5fa00dedd73e1995132c1fd60a0ff820d 100644 (file)
--- a/lib/sendf.c
+++ b/lib/sendf.c
@@ -237,7 +237,18 @@ void Curl_infof(struct Curl_easy *data, const char *fmt, ...)
     size_t len;
     char print_buffer[2048 + 1];
     va_start(ap, fmt);
-    vsnprintf(print_buffer, sizeof(print_buffer), fmt, ap);
+    len = vsnprintf(print_buffer, sizeof(print_buffer), fmt, ap);
+    /*
+     * Indicate truncation of the input by replacing the last 3 characters
+     * with "...", and transfer the newline over in case the format had one.
+     */
+    if(len >= sizeof(print_buffer)) {
+      len = strlen(fmt);
+      if(fmt[--len] == '\n')
+        snprintf(print_buffer + (sizeof(print_buffer) - 5), 5, "...\n");
+      else
+        snprintf(print_buffer + (sizeof(print_buffer) - 4), 4, "...");
+    }
     va_end(ap);
     len = strlen(print_buffer);
     Curl_debug(data, CURLINFO_TEXT, print_buffer, len);

diff --git a/tests/data/Makefile.inc b/tests/data/Makefile.inc
index 82aeb88bef9b0c579e202309e3ac9cc9f36ab0dd..4611d3cb89eefa0f196b1585945b388a71ab45e8 100644 (file)
--- a/tests/data/Makefile.inc
+++ b/tests/data/Makefile.inc
@@ -184,7 +184,7 @@ test1590 \
 test1600 test1601 test1602 test1603 test1604 test1605 test1606 test1607 \
 test1608 test1609 test1620 \
 \
-test1650 test1651 \
+test1650 test1651 test1652 \
 \
 test1700 test1701 test1702 \
 \

diff --git a/tests/data/test1652 b/tests/data/test1652
new file mode 100644 (file)
index 0000000..c411690
--- /dev/null
+++ b/tests/data/test1652
@@ -0,0 +1,23 @@
+<testcase>
+<info>
+<keywords>
+unittest
+infof
+</keywords>
+</info>
+
+<client>
+<server>
+none
+</server>
+<features>
+unittest
+</features>
+<name>
+infof
+</name>
+<tool>
+unit1652
+</tool>
+</client>
+</testcase>

diff --git a/tests/unit/Makefile.inc b/tests/unit/Makefile.inc
index 95d6cb44b0c0538963ceb2367f14a6d67f42da77..2ba6b5ee525f0a7486fb5650337f7c7c0eedc9f0 100644 (file)
--- a/tests/unit/Makefile.inc
+++ b/tests/unit/Makefile.inc
@@ -11,7 +11,7 @@ UNITPROGS = unit1300 unit1301 unit1302 unit1303 unit1304 unit1305 unit1307 \
  unit1399 \
  unit1600 unit1601 unit1602 unit1603 unit1604 unit1605 unit1606 unit1607 \
  unit1608 unit1609 unit1620 \
- unit1650 unit1651
+ unit1650 unit1651 unit1652
 
 unit1300_SOURCES = unit1300.c $(UNITFILES)
 unit1300_CPPFLAGS = $(AM_CPPFLAGS)
@@ -105,3 +105,6 @@ unit1650_CPPFLAGS = $(AM_CPPFLAGS)
 
 unit1651_SOURCES = unit1651.c $(UNITFILES)
 unit1651_CPPFLAGS = $(AM_CPPFLAGS)
+
+unit1652_SOURCES = unit1652.c $(UNITFILES)
+unit1652_CPPFLAGS = $(AM_CPPFLAGS)

diff --git a/tests/unit/unit1652.c b/tests/unit/unit1652.c
new file mode 100644 (file)
index 0000000..923c870
--- /dev/null
+++ b/tests/unit/unit1652.c
@@ -0,0 +1,133 @@
+/***************************************************************************
+ *                                  _   _ ____  _
+ *  Project                     ___| | | |  _ \| |
+ *                             / __| | | | |_) | |
+ *                            | (__| |_| |  _ <| |___
+ *                             \___|\___/|_| \_\_____|
+ *
+ * Copyright (C) 1998 - 2018, Daniel Stenberg, <daniel@haxx.se>, et al.
+ *
+ * This software is licensed as described in the file COPYING, which
+ * you should have received as part of this distribution. The terms
+ * are also available at https://curl.haxx.se/docs/copyright.html.
+ *
+ * You may opt to use, copy, modify, merge, publish, distribute and/or sell
+ * copies of the Software, and permit persons to whom the Software is
+ * furnished to do so, under the terms of the COPYING file.
+ *
+ * This software is distributed on an "AS IS" basis, WITHOUT WARRANTY OF ANY
+ * KIND, either express or implied.
+ *
+ ***************************************************************************/
+#include "curlcheck.h"
+
+#include "urldata.h"
+#include "sendf.h"
+
+/*
+ * This test hardcodes the knowledge of the buffer size which is internal to
+ * Curl_infof(). If that buffer is changed in size, this tests needs to be
+ * updated to still be valid.
+ */
+
+static struct Curl_easy *data;
+
+static char input[4096];
+static char result[4096];
+
+int debugf_cb(CURL *handle, curl_infotype type, char *buf, size_t size,
+              void *userptr);
+
+/*
+ * This debugf callback is simply dumping the string into the static buffer
+ * for the unit test to inspect. Since we know that we're only dealing with
+ * text we can afford the luxury of skipping the type check here.
+ */
+int
+debugf_cb(CURL *handle, curl_infotype type, char *buf, size_t size,
+                void *userptr)
+{
+  (void)handle;
+  (void)type;
+  (void)userptr;
+
+  memset(result, '\0', sizeof(result));
+  memcpy(result, buf, size);
+  return 0;
+}
+
+static CURLcode
+unit_setup(void)
+{
+  int res = 0;
+
+  global_init(CURL_GLOBAL_ALL);
+  data = curl_easy_init();
+  if(!data)
+    return CURLE_OUT_OF_MEMORY;
+  curl_easy_setopt(data, CURLOPT_DEBUGFUNCTION, debugf_cb);
+  curl_easy_setopt(data, CURLOPT_VERBOSE, 1L);
+  return CURLE_OK;
+}
+
+static void
+unit_stop(void)
+{
+  curl_easy_cleanup(data);
+  curl_global_cleanup();
+}
+
+UNITTEST_START
+
+/* Injecting a simple short string via a format */
+snprintf(input, sizeof(input), "Simple Test");
+Curl_infof(data, "%s", input);
+fail_unless(strcmp(result, input) == 0, "Simple string test");
+
+/* Injecting a few different variables with a format */
+Curl_infof(data, "%s %u testing %.1f\n", input, 42, 43.0123324);
+fail_unless(strcmp(result, "Simple Test 42 testing 43.0\n") == 0,
+            "Format string");
+
+/* Variations of empty strings */
+Curl_infof(data, "");
+fail_unless(strlen(result) == 0, "Empty string");
+Curl_infof(data, "%s", NULL);
+fail_unless(strcmp(result, "(nil)") == 0, "Passing NULL as string");
+
+/* A string just long enough to not be truncated */
+memset(input, '\0', sizeof(input));
+memset(input, 'A', 2048);
+Curl_infof(data, "%s", input);
+fail_unless(strlen(result) == 2048, "No truncation of infof input");
+fail_unless(strcmp(result, input) == 0, "No truncation of infof input");
+fail_unless(result[sizeof(result) - 1] == '\0',
+            "No truncation of infof input");
+
+/* Just over the limit for truncation without newline */
+memset(input + 2047, 'A', 4);
+Curl_infof(data, "%s", input);
+fail_unless(strlen(result) == 2048, "Truncation of infof input 1");
+fail_unless(result[sizeof(result) - 1] == '\0', "Truncation of infof input 1");
+fail_unless(strncmp(result + 2045, "...", 3) == 0,
+            "Truncation of infof input 1");
+
+/* Just over the limit for truncation with newline */
+memset(input + 2047, 'A', 4);
+memset(input + 2047 + 4, '\n', 1);
+Curl_infof(data, "%s\n", input);
+fail_unless(strlen(result) == 2048, "Truncation of infof input 2");
+fail_unless(result[sizeof(result) - 1] == '\0', "Truncation of infof input 2");
+fail_unless(strncmp(result + 2044, "...", 3) == 0,
+            "Truncation of infof input 2");
+
+/* Way over the limit for truncation with newline */
+memset(input, '\0', sizeof(input));
+memset(input, 'A', sizeof(input) - 1);
+Curl_infof(data, "%s\n", input);
+fail_unless(strlen(result) == 2048, "Truncation of infof input 3");
+fail_unless(result[sizeof(result) - 1] == '\0', "Truncation of infof input 3");
+fail_unless(strncmp(result + 2044, "...", 3) == 0,
+            "Truncation of infof input 3");
+
+UNITTEST_STOP