access_log directive: if= bug for buffered logs

[Anton Yuzhaninov]

In nginx 1.7.0 the if= parameter was added to the access_log directive for conditional logging. This condition fails to work for buffered logs.

Example nginx config:

# ?x=[01] or no x
map $arg_x $x1 {
  default 1;
  2       0;
}
# ?x=2 only
map $arg_x $x2 {
  default 0;
  2       1;
}

server {
  ...
  access_log /var/log/nginx/access_log;
  ...
  location = /a.gif {
    empty_gif;
    access_log /var/log/nginx/x1_log http_log buffer=16k if=$x1;
    access_log /var/log/nginx/x2_log http_log buffer=16k if=$x2;
  }

  location = /b.gif {
    empty_gif;
    access_log /var/log/nginx/x1_log http_log buffer=16k if=$x1;
    access_log /var/log/nginx/x2_log http_log buffer=16k if=$x2;
}

Test load can be created with some benchmark, e. g. http_load with url_file

http://127.0.0.1/a.gif
http://127.0.0.1/a.gif?x=0
http://127.0.0.1/a.gif?x=1
http://127.0.0.1/a.gif?x=2
http://127.0.0.1/b.gif
http://127.0.0.1/b.gif?x=0
http://127.0.0.1/b.gif?x=1
http://127.0.0.1/b.gif?x=2

After test in the x1_log file I can see requests to /b.gif?x=2 and /b.gif, /b.gif?x=0 in the x2_log. This is wrong.

After removing buffer=16k from access_log directive condition works just fine.

[Valentin V. Bartenev]

Thanks for the report.

The problem is caused by duplicate "buffer" option. You don't need to specify it again for the same file, and the configuration below should work fine:

location = /a.gif {
    empty_gif;
    access_log /var/log/nginx/x1_log http_log buffer=16k if=$x1;
    access_log /var/log/nginx/x2_log http_log buffer=16k if=$x2;
}

location = /b.gif {
    empty_gif;
    access_log /var/log/nginx/x1_log http_log if=$x1;
    access_log /var/log/nginx/x2_log http_log if=$x2;
}

But anyway, this is a bug.

[Anton Yuzhaninov]

Yes with buffer=16k only in first location, I can't repeat the problem. But it is handy to use includes e. g.

location = /a.gif {
  ...
  include logs.conf;
}

location = /b.gif {
  ...
  include logs.conf;
}

In this case buffer parameter will be repeated.

[Valentin V. Bartenev]

This patch should fix the problem:

src/http/modules/ngx_http_log_module.c

@@ -1136 +1136 @@
     ngx_int_t                          gzip;
     ngx_uint_t                         i, n;
     ngx_msec_t                         flush;
-    ngx_str_t                         *value, name, s, filter;
+    ngx_str_t                         *value, name, s;
     ngx_http_log_t                    *log;
     ngx_syslog_peer_t                 *peer;
     ngx_http_log_buf_t                *buffer;
@@ -1257 +1257 @@
     size = 0;
     flush = 0;
     gzip = 0;
-    filter.len = 0;
 
     for (i = 3; i < cf->args->nelts; i++) {
 
@@ -1325 +1324 @@
         }
 
         if (ngx_strncmp(value[i].data, "if=", 3) == 0) {
-            filter.len = value[i].len - 3;
-            filter.data = value[i].data + 3;
+            value[i].len -= 3;
+            value[i].data += 3;
+
+            ngx_memzero(&ccv, sizeof(ngx_http_compile_complex_value_t));
+
+            ccv.cf = cf;
+            ccv.value = &value[i];
+            ccv.complex_value = ngx_palloc(cf->pool,
+                                           sizeof(ngx_http_complex_value_t));
+            if (ccv.complex_value == NULL) {
+                return NGX_CONF_ERROR;
+            }
+
+            if (ngx_http_compile_complex_value(&ccv) != NGX_OK) {
+                return NGX_CONF_ERROR;
+            }
+
+            log->filter = ccv.complex_value;
+
             continue;
         }
 
@@ -1405 +1421 @@
         log->file->data = buffer;
     }
 
-    if (filter.len) {
-        log->filter = ngx_palloc(cf->pool, sizeof(ngx_http_complex_value_t));
-        if (log->filter == NULL) {
-            return NGX_CONF_ERROR;
-        }
-
-        ngx_memzero(&ccv, sizeof(ngx_http_compile_complex_value_t));
-
-        ccv.cf = cf;
-        ccv.value = &filter;
-        ccv.complex_value = log->filter;
-
-        if (ngx_http_compile_complex_value(&ccv) != NGX_OK) {
-            return NGX_CONF_ERROR;
-        }
-    }
-
     return NGX_CONF_OK;
 }
 

[Valentin Bartenev <vbart@…>]

In e3016ee8dba396614f28a3644996b8cc6de8f9e3/nginx:

Access log: fixed the "if=" parameter with buffering (ticket #625).

It might not work if there were more than one "access_log" directives
pointed to the same file and duplicate buffer parameters.

[Valentin V. Bartenev]

The fix will be released with nginx 1.7.5.