Use standard httpd logging format in error log

[marcstern]

All credits to @arminabf - see #1997

Use the server context, like in all other places to use standard httpd format.

Remark: We previously had a mix; some entries were using the standard httpd format, some not.
Entries using the standard httpd format had the [client] field duplicated.
In this PR, we use standard logging for all lines and remove the duplicated field.
Actual changes:

• entries that were not using the standard format will use it
• entries that were using the standard format won't have the [client] field duplicated anymore

[fzipi]

Is there a way to actually test this change?

[theseion]

Sounds good, but I'm with @fzipi. From the code it's not clear what the output will be. Does the [client] field still contain the same IP address that ModSecurity would have injected in every case?

[marcstern]

Sounds good, but I'm with @fzipi. From the code it's not clear what the output will be. Does the [client] field still contain the same IP address that ModSecurity would have injected in every case?

Yes, same logic: useragent_ip if present, client_ip otherwise

[marcstern]

Code from httpd log.c

if (info->r) {
    len += apr_snprintf(buf + len, buflen - len,
                        info->r->connection->sbh ? "[client %s:%d] " : "[remote %s:%d] ",
                        info->r->useragent_ip,
                        info->r->useragent_addr ? info->r->useragent_addr->port : 0);
}
else if (info->c) {
    len += apr_snprintf(buf + len, buflen - len,
                        info->c->sbh ? "[client %s:%d] " : "[remote %s:%d] ",
                        info->c->client_ip,
                        info->c->client_addr ? info->c->client_addr->port : 0);
}

[theseion]

That's good. But a test that asserts the log format would be even better.

[marcstern]

I added some tests

[theseion]

Thanks, that's great. @airween, isn't there a test suite for ModSecurity where the test could be added?

[airween]

Thanks, that's great. @airween, isn't there a test suite for ModSecurity where the test could be added?

Unfortunately no, there isn't.

We have only this CI:

And as @marcstern wrote, he added a test (but I'm not sure that's what we expect - I'm waiting for Marc's answer).

But as I introduced few weeks ago, there is MRTS, where the aim is to create a full covered test suite. Feel free to review and contribute! 😄

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud