Skip to content

fpm: Implement access log filtering #8214

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

fpm: Implement access log filtering #8214

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 33 additions & 6 deletions sapi/fpm/fpm/fpm_conf.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -557,11 +557,13 @@ static char *fpm_conf_set_array(zval *key, zval *value, void **config, int conve
}

memset(kv, 0, sizeof(*kv));
kv->key = strdup(Z_STRVAL_P(key));
if (key) {
kv->key = strdup(Z_STRVAL_P(key));

if (!kv->key) {
free(kv);
return "fpm_conf_set_array: strdup(key) failed";
if (!kv->key) {
free(kv);
return "fpm_conf_set_array: strdup(key) failed";
}
}

if (convert_to_bool) {
Expand Down Expand Up @@ -663,6 +665,11 @@ int fpm_worker_pool_config_free(struct fpm_worker_pool_config_s *wpc) /* {{{ */
free(wpc->apparmor_hat);
#endif

for (kv = wpc->access_suppress_paths; kv; kv = kv_next) {
kv_next = kv->next;
free(kv->value);
free(kv);
}
for (kv = wpc->php_values; kv; kv = kv_next) {
kv_next = kv->next;
free(kv->key);
Expand Down Expand Up @@ -1497,11 +1504,24 @@ static void fpm_conf_ini_parser_array(zval *name, zval *key, zval *value, void *
char *err = NULL;
void *config;

if (!*Z_STRVAL_P(key)) {
zlog(ZLOG_ERROR, "[%s:%d] Misspelled array ?", ini_filename, ini_lineno);
if (zend_string_equals_literal(Z_STR_P(name), "access.suppress_path")) {
if (!(*Z_STRVAL_P(key) == '\0')) {
zlog(ZLOG_ERROR, "[%s:%d] Keys provided to field 'access.suppress_path' are ignored", ini_filename, ini_lineno);
*error = 1;
}
if (!(*Z_STRVAL_P(value)) || (*Z_STRVAL_P(value) != '/')) {
zlog(ZLOG_ERROR, "[%s:%d] Values provided to field 'access.suppress_path' must begin with '/'", ini_filename, ini_lineno);
*error = 1;
}
if (*error) {
return;
}
} else if (!*Z_STRVAL_P(key)) {
zlog(ZLOG_ERROR, "[%s:%d] You must provide a key for field '%s'", ini_filename, ini_lineno, Z_STRVAL_P(name));
*error = 1;
return;
}

if (!current_wp || !current_wp->config) {
zlog(ZLOG_ERROR, "[%s:%d] Array are not allowed in the global section", ini_filename, ini_lineno);
*error = 1;
Expand Down Expand Up @@ -1533,6 +1553,10 @@ static void fpm_conf_ini_parser_array(zval *name, zval *key, zval *value, void *
config = (char *)current_wp->config + WPO(php_admin_values);
err = fpm_conf_set_array(key, value, &config, 1);

} else if (zend_string_equals_literal(Z_STR_P(name), "access.suppress_path")) {
config = (char *)current_wp->config + WPO(access_suppress_paths);
err = fpm_conf_set_array(NULL, value, &config, 0);

} else {
zlog(ZLOG_ERROR, "[%s:%d] unknown directive '%s'", ini_filename, ini_lineno, Z_STRVAL_P(name));
*error = 1;
Expand Down Expand Up @@ -1735,6 +1759,9 @@ static void fpm_conf_dump(void) /* {{{ */
zlog(ZLOG_NOTICE, "\tping.response = %s", STR2STR(wp->config->ping_response));
zlog(ZLOG_NOTICE, "\taccess.log = %s", STR2STR(wp->config->access_log));
zlog(ZLOG_NOTICE, "\taccess.format = %s", STR2STR(wp->config->access_format));
for (kv = wp->config->access_suppress_paths; kv; kv = kv->next) {
zlog(ZLOG_NOTICE, "\taccess.suppress_path[] = %s", kv->value);
}
zlog(ZLOG_NOTICE, "\tslowlog = %s", STR2STR(wp->config->slowlog));
zlog(ZLOG_NOTICE, "\trequest_slowlog_timeout = %ds", wp->config->request_slowlog_timeout);
zlog(ZLOG_NOTICE, "\trequest_slowlog_trace_depth = %d", wp->config->request_slowlog_trace_depth);
Expand Down
1 change: 1 addition & 0 deletions sapi/fpm/fpm/fpm_conf.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ struct fpm_worker_pool_config_s {
char *ping_response;
char *access_log;
char *access_format;
struct key_value_s *access_suppress_paths;
char *slowlog;
int request_slowlog_timeout;
int request_slowlog_trace_depth;
Expand Down
53 changes: 53 additions & 0 deletions sapi/fpm/fpm/fpm_log.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,9 @@

static char *fpm_log_format = NULL;
static int fpm_log_fd = -1;
static struct key_value_s *fpm_access_suppress_paths = NULL;

static int fpm_access_log_suppress(struct fpm_scoreboard_proc_s *proc);

int fpm_log_open(int reopen) /* {{{ */
{
Expand Down Expand Up @@ -79,6 +82,17 @@ int fpm_log_init_child(struct fpm_worker_pool_s *wp) /* {{{ */
}
}

for (struct key_value_s *kv = wp->config->access_suppress_paths; kv; kv = kv->next) {
struct key_value_s *kvcopy = calloc(1, sizeof(*kvcopy));
if (kvcopy == NULL) {
zlog(ZLOG_ERROR, "unable to allocate memory while opening the access log");
return -1;
}
kvcopy->value = strdup(kv->value);
kvcopy->next = fpm_access_suppress_paths;
fpm_access_suppress_paths = kvcopy;
}

if (fpm_log_fd == -1) {
fpm_log_fd = wp->log_fd;
}
Expand Down Expand Up @@ -136,6 +150,10 @@ int fpm_log_write(char *log_format) /* {{{ */
}
proc = *proc_p;
fpm_scoreboard_proc_release(proc_p);

if (UNEXPECTED(fpm_access_log_suppress(&proc))) {
return -1;
}
}

token = 0;
Expand Down Expand Up @@ -474,3 +492,38 @@ int fpm_log_write(char *log_format) /* {{{ */
return 0;
}
/* }}} */

static int fpm_access_log_suppress(struct fpm_scoreboard_proc_s *proc)
{
// Never suppress when query string is passed
if (proc->query_string[0] != '\0') {
return 0;
}

// Never suppress if request method is not GET or HEAD
if (
strcmp(proc->request_method, "GET") != 0
&& strcmp(proc->request_method, "HEAD") != 0
) {
return 0;
}

// Never suppress when response code does not indicate success
if (SG(sapi_headers).http_response_code < 200 || SG(sapi_headers).http_response_code > 299) {
return 0;
}

// Never suppress when a body has been sent
if (SG(request_info).content_length > 0) {
return 0;
}

// Suppress when request URI is an exact match for one of our entries
for (struct key_value_s *kv = fpm_access_suppress_paths; kv; kv = kv->next) {
if (kv->value && strcmp(kv->value, proc->request_uri) == 0) {
return 1;
}
}

return 0;
}
34 changes: 34 additions & 0 deletions sapi/fpm/tests/config-array-validation-php-value-key.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
--TEST--
FPM: Validates arrays in configuration are correctly set - php_value array must be passed a key
--SKIPIF--
<?php include "skipif.inc"; ?>
--FILE--
<?php

require_once "tester.inc";

$cfg = <<<EOT
[global]
error_log = {{FILE:LOG}}
pid = {{FILE:PID}}
[unconfined]
listen = {{ADDR}}
php_value[] = E_ALL
pm = static
pm.max_children = 5
EOT;

$tester = new FPM\Tester($cfg);
$tester->start(['-tt']);
$tester->expectLogError("\[%s:%d\] You must provide a key for field 'php_value'");

?>
Done
--EXPECT--

Done
--CLEAN--
<?php
require_once "tester.inc";
FPM\Tester::clean();
?>
34 changes: 34 additions & 0 deletions sapi/fpm/tests/config-array-validation-suppress-path-key-2.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
--TEST--
FPM: Validates arrays in configuration are correctly set - access.suppress_path doesn't accept key with forward slash
--SKIPIF--
<?php include "skipif.inc"; ?>
--FILE--
<?php

require_once "tester.inc";

$cfg = <<<EOT
[global]
error_log = {{FILE:LOG}}
pid = {{FILE:PID}}
[unconfined]
listen = {{ADDR}}
access.suppress_path[/] = test
pm = static
pm.max_children = 5
EOT;

$tester = new FPM\Tester($cfg);
$tester->start(['-tt']);
$tester->expectLogError("\[%s:%d\] Keys provided to field 'access.suppress_path' are ignored");

?>
Done
--EXPECT--

Done
--CLEAN--
<?php
require_once "tester.inc";
FPM\Tester::clean();
?>
34 changes: 34 additions & 0 deletions sapi/fpm/tests/config-array-validation-suppress-path-key.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
--TEST--
FPM: Validates arrays in configuration are correctly set - access.suppress_path doesn't allow key
--SKIPIF--
<?php include "skipif.inc"; ?>
--FILE--
<?php

require_once "tester.inc";

$cfg = <<<EOT
[global]
error_log = {{FILE:LOG}}
pid = {{FILE:PID}}
[unconfined]
listen = {{ADDR}}
access.suppress_path[pingpath] = /ping
pm = static
pm.max_children = 5
EOT;

$tester = new FPM\Tester($cfg);
$tester->start(['-tt']);
$tester->expectLogError("\[%s:%d\] Keys provided to field 'access.suppress_path' are ignored");

?>
Done
--EXPECT--

Done
--CLEAN--
<?php
require_once "tester.inc";
FPM\Tester::clean();
?>
34 changes: 34 additions & 0 deletions sapi/fpm/tests/config-array-validation-suppress-path-starts-slash.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
--TEST--
FPM: Validates arrays in configuration are correctly set - access.suppress_path begins with forward slash
--SKIPIF--
<?php include "skipif.inc"; ?>
--FILE--
<?php

require_once "tester.inc";

$cfg = <<<EOT
[global]
error_log = {{FILE:LOG}}
pid = {{FILE:PID}}
[unconfined]
listen = {{ADDR}}
access.suppress_path[] = needs / to start with a slash
pm = static
pm.max_children = 5
EOT;

$tester = new FPM\Tester($cfg);
$tester->start(['-tt']);
$tester->expectLogError("\[%s:%d\] Values provided to field 'access.suppress_path' must begin with '\/'");

?>
Done
--EXPECT--

Done
--CLEAN--
<?php
require_once "tester.inc";
FPM\Tester::clean();
?>
49 changes: 49 additions & 0 deletions sapi/fpm/tests/config-array.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
--TEST--
FPM: Set arrays in configuration
--SKIPIF--
<?php include "skipif.inc"; ?>
--FILE--
<?php

require_once "tester.inc";

$cfg = <<<EOT
[global]
error_log = {{FILE:LOG}}
pid = {{FILE:PID}}
[unconfined]
listen = {{ADDR}}
access.suppress_path[] = /ping
access.suppress_path[] = /health_check.php
pm = static
pm.max_children = 5
php_value[error_reporting] = E_ALL
php_value[date.timezone] = Europe/London
php_admin_value[disable_functions] = eval
php_flag[display_errors] = On
php_admin_flag[log_errors] = 1
EOT;

$tester = new FPM\Tester($cfg);
$tester->start(['-tt']);
$tester->expectLogConfigOptions([
'access.suppress_path[] = /ping',
'access.suppress_path[] = /health_check.php',
'php_value[error_reporting] = 32767',
'php_value[date.timezone] = Europe/London',
'php_value[display_errors] = 1',
'php_admin_value[disable_functions] = eval',
'php_admin_value[log_errors] = 1',
]);


?>
Done
--EXPECT--

Done
--CLEAN--
<?php
require_once "tester.inc";
FPM\Tester::clean();
?>
Loading