BUG/MINOR: log: prevent double spaces emission in sess_build_logline()

Christian reported in GH #2556 that since 3.0-dev double spaces may be
found in log messages on some cases where it was not the case before.

As we were able to easily reproduce, a quick bisect led us to c6a7138
("MINOR: log: simplify last_isspace in sess_build_logline()"). While
it is true that all switch cases set the last_isspace variable to 0,
there was a subtelty for some fields such as '%hr', '%hrl', '%hs' or
'%hsl' and I overlooked it. Indeed, for '%hr', last_isspace was only set
to 0 if data was emitted, else the assignment didn't occur.

But with c6a7138, last_isspace is always set to 0 as long as the current
node type is not a separator. Because of that, if no data is emitted for
the current node value, and a space was already emitted prior to the
current node, then an extra space could be emitted after the node,
resulting in two spaces being emitted.

Note that while c6a7138 introduces a slight behavior regression regarding
last_isspace logic with the specific fields mentionned above, this
behavior could already be triggered with a failing or empty logformat
node sample expression. Consider this logformat expression:

  log-format "%{-M}o | %[str()] |"

str() will not print anything, and since we disabled mandatory option with
'-M', nothing gets printed for the node sample expression. As a result, we
have the following output:

  "|  |"

Instead of (when mandatory option is enabled):

  "| - |"

Thus in order to stick to the historical behavior, systematically set
last_isspace to 0 for EXPR nodes, and only set last_isspace to 0 when
data was written for TAG nodes. This way, '%hr', '%hrl', '%hs' or
'%hsl' should behave as before.

No backport needed.

diff --git a/src/log.c b/src/log.c
index f6ab921c2020bb747f61ee44a5a8da0169dc90c9..14c03cc18d3bf66f8b0d5d2cbb3f64cf928d9d03 100644 (file)
--- a/src/log.c
+++ b/src/log.c
@@ -3797,6 +3797,7 @@ int sess_build_logline(struct session *sess, struct stream *s, char *dst, size_t
                        if (ret == NULL)
                                goto out;
                        tmplog = ret;
+                       last_isspace = 0; /* consider that data was written */
                        goto next_fmt;
                }
 
@@ -4756,8 +4757,6 @@ int sess_build_logline(struct session *sess, struct stream *s, char *dst, size_t
 
                }
  next_fmt:
-               last_isspace = 0;
-
                if (value_beg == tmplog) {
                        /* handle the case where no data was generated for the value after
                         * the key was already announced
@@ -4782,6 +4781,13 @@ int sess_build_logline(struct session *sess, struct stream *s, char *dst, size_t
                 * to end it
                 */
                LOG_VARTEXT_END();
+               if (tmplog != value_beg) {
+                       /* data was actually generated for the current dynamic
+                        * node, reset the space hint so that a new space may
+                        * now be emitted when relevant.
+                        */
+                       last_isspace = 0;
+               }
        }
 
        /* back to global ctx (some encoding types may need to output