Hello!

On Mon, Aug 07, 2017 at 10:36:19AM +0000, Eran Kornblau wrote:

> Hi all,
> 
> The attached patch adds support for log buffering when using variables in the 
> access log file name.
> 
> The use case is this - we use nginx to receive analytics beacons and write 
> them to the access log.
> We'd like to have a log file per hour that contains the logs of the specific 
> hour. If we use some external 
> script to perform log rotate, we cannot avoid log lines slipping between 
> adjacent files.
> It's important for us to have the log lines partitioning accurate, in case we 
> need to reindex a specific 
> hour to the database.
> Scripted logs seem perfect for this, but we don't want to give up on log 
> compression, since the 
> number of events can be high.
> 
> The patch relies on the open file cache to keep the context (buffer + flush 
> event) of each file.
> In order to do that, I added the ability to register callbacks in open file 
> cache for these events:
> 1. init - new cached file object created
> 2. flush - the file handle of a cached file is closed
> 3. free - the cached file is destroyed
> The context is allocated right after the ngx_cached_open_file_t struct, this 
> was done in order to avoid
> increasing the size of the ngx_cached_open_file_t struct (compared to the 
> alternative of adding some
> opaque pointer on this struct).
> Therefore, there are no memory implications for "regular" (log = 0) open file 
> caches, the overhead is
> only an extra 'if' on the creation / deletion of the cached file object (no 
> impact at all on cache hit,
> which is probably the most performance sensitive) 

Just a quick note: for me, the whole feature looks questionable, 
and the implementation is far from being in a commitable state.

> Btw, on a similar subject - can anyone explain the purpose of checking the 
> existence of the root dir
> in scripted logs? Only explanation I could think of is to provide some 
> security protection in case $uri
> is used in the script, but that sounds like a very specific use case to me... 
> Anyway, if that is indeed the case, maybe it makes sense to add conf 
> directive to enable/disable 
> this behavior?

The basic purpose of this restriction is to protect configurations 
like

    server {
        access_log /spool/vhost/logs/$host;
        ...
    }

from creating arbitrary log files, thus allowing DoS attacks via 
inode exhaustion.  Accordingly, the documentation 
(http://nginx.org/r/access_log) recommends to specify both root 
and access_log if you want to use variables in logging:

: during each log write the existence of the request’s root 
: directory is checked, and if it does not exist the log is not 
: created. It is thus a good idea to specify both root and 
: access_log on the same level:
:
: server {
:     root       /spool/vhost/data/$host;
:     access_log /spool/vhost/logs/$host;
:     ...

-- 
Maxim Dounin
http://nginx.org/
_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel

Reply via email to