From 621f97671889cf0ea74cb845a7de33f2cebf8bbe Mon Sep 17 00:00:00 2001 From: Arturo Bernal Date: Sat, 14 Nov 2026 07:50:23 +0100 Subject: [PATCH] core: Add a "b" flag to ErrorLogFormat's %a/%A for RFC 5952 bracketed (#1016) IPv6 addresses. %a and %A log an IPv6 address and port as "address:port", e.g. "::1:34567" -- ambiguous, since both the address and the address/port separator use ':'. This makes it hard for downstream log parsers (syslog anonymizers, SIEMs, etc.) to tell where the address ends and the port begins. Add an opt-in "b" flag, "%{b}a" and "%{b}A", that writes the same address and port in the RFC 5952, section 6 bracket notation, "[address]:port", matching the convention already used by other HTTP implementations for the same ambiguity (e.g. Netty's NetUtil.toSocketAddressString(), OkHttp's HttpUrl.Builder.toString()). IPv4 addresses are unaffected either way. The existing "%a"/"%A"/"%{c}a" output is unchanged, so no existing log consumer is affected unless it opts in. PR 65696 (cherry picked from commit 14d0c60bc77e119ed3e6a4b9c9feca8bcc711a52) --- changes-entries/65696.txt | 5 ++ docs/manual/mod/core.xml | 11 +++ server/log.c | 31 ++++++-- .../test_016_errorlogformat_ipv6_bracket.py | 72 +++++++++++++++++++ 4 files changed, 112 insertions(+), 7 deletions(-) create mode 100644 changes-entries/65696.txt create mode 100644 test/modules/core/test_016_errorlogformat_ipv6_bracket.py diff --git a/changes-entries/65696.txt b/changes-entries/65696.txt new file mode 100644 index 00000000000..6a12c4e8280 --- /dev/null +++ b/changes-entries/65696.txt @@ -0,0 +1,5 @@ + *) core: Add a "b" flag to the ErrorLogFormat "%a" and "%A" directives + (as "%{b}a" and "%{b}A") that writes an IPv6 address in brackets, + "[address]:port", per RFC 5952 section 6, instead of the existing + "address:port" that is ambiguous for IPv6. The default "%a"/"%A" + output is unchanged. PR 65696 [Arturo Bernal] diff --git a/docs/manual/mod/core.xml b/docs/manual/mod/core.xml index a7ed5cd4854..935b7faab03 100644 --- a/docs/manual/mod/core.xml +++ b/docs/manual/mod/core.xml @@ -1761,9 +1761,20 @@ ErrorLogFormat "[%t] [%l] [pid %P] %F: %E: [client %a] %M" Underlying peer IP address and port of the connection (see the mod_remoteip module) + %{b}a + Like %a, but an IPv6 address is written in + brackets, as [address]:port, per + RFC + 5952, section 6. The b and c + flags can be combined, as in %{bc}a + %A Local IP-address and port + %{b}A + Like %A, but an IPv6 address is written in + brackets, as with %{b}a + %{name}e Request environment variable name diff --git a/server/log.c b/server/log.c index 438171d113d..dc1a0451be5 100644 --- a/server/log.c +++ b/server/log.c @@ -595,15 +595,31 @@ static int cpystrn(char *buf, const char *arg, int buflen) } +static int log_formatted_address(const char *ip, apr_port_t port, + const char *arg, char *buf, int buflen) +{ + /* RFC 5952, section 6: an IPv6 address combined with a port is + * written as "[address]:port" to keep the port's colon from being + * read as part of the address. Opt-in via the 'b' arg flag, since + * existing ErrorLogFormat consumers may already parse the + * bracket-less "address:port" this directive has always produced. + */ + if (arg && ap_strchr_c(arg, 'b') && ap_strchr_c(ip, ':')) + return apr_snprintf(buf, buflen, "[%s]:%d", ip, port); + return apr_snprintf(buf, buflen, "%s:%d", ip, port); +} + static int log_remote_address(const ap_errorlog_info *info, const char *arg, char *buf, int buflen) { - if (info->r && !(arg && *arg == 'c')) - return apr_snprintf(buf, buflen, "%s:%d", info->r->useragent_ip, - info->r->useragent_addr ? info->r->useragent_addr->port : 0); + if (info->r && !(arg && ap_strchr_c(arg, 'c'))) + return log_formatted_address(info->r->useragent_ip, + info->r->useragent_addr ? info->r->useragent_addr->port : 0, + arg, buf, buflen); else if (info->c) - return apr_snprintf(buf, buflen, "%s:%d", info->c->client_ip, - info->c->client_addr ? info->c->client_addr->port : 0); + return log_formatted_address(info->c->client_ip, + info->c->client_addr ? info->c->client_addr->port : 0, + arg, buf, buflen); else return 0; } @@ -612,8 +628,9 @@ static int log_local_address(const ap_errorlog_info *info, const char *arg, char *buf, int buflen) { if (info->c) - return apr_snprintf(buf, buflen, "%s:%d", info->c->local_ip, - info->c->local_addr->port); + return log_formatted_address(info->c->local_ip, + info->c->local_addr->port, + arg, buf, buflen); else return 0; } diff --git a/test/modules/core/test_016_errorlogformat_ipv6_bracket.py b/test/modules/core/test_016_errorlogformat_ipv6_bracket.py new file mode 100644 index 00000000000..a049f4ea675 --- /dev/null +++ b/test/modules/core/test_016_errorlogformat_ipv6_bracket.py @@ -0,0 +1,72 @@ +"""ErrorLogFormat's %a/%A print an IPv6 address and port as +"address:port" -- e.g. "::1:34567" -- which is ambiguous: a log consumer +cannot tell where the address ends and the port begins, since both use +':' as a separator (BZ 65696). %{b}a/%{b}A opt into the RFC 5952, +section 6 bracket notation instead: "[::1]:34567", matching the +convention other HTTP libraries (e.g. Netty, OkHttp) already use for +the same host:port ambiguity. +""" +import re +import socket + +import pytest + +from pyhttpd.conf import HttpdConf + + +IPV6_PORT = 5221 + + +def _has_ipv6(): + try: + s = socket.socket(socket.AF_INET6, socket.SOCK_STREAM) + s.bind(('::1', 0)) + s.close() + return True + except (socket.error, OSError): + return False + + +@pytest.fixture(autouse=True, scope='class') +def _class_scope(env): + if not _has_ipv6(): + pytest.skip('IPv6 (::1) not available in this environment') + + conf = HttpdConf(env) + conf.add([ + f'Listen [::1]:{IPV6_PORT}', + 'ErrorLogFormat "a=%a ba=%{b}a A=%A bA=%{b}A %M"', + ]) + conf.start_vhost(domains=[f'ipv6.{env.http_tld}'], port=IPV6_PORT, + with_ssl=False) + conf.add(['', 'Require all denied', '']) + conf.end_vhost() + conf.install() + assert env.apache_restart() == 0 + + +class TestErrorLogFormatIPv6Bracket: + + def test_core_016_01_default_format_unchanged(self, env): + """%a/%A keep their existing, unbracketed "address:port" output + -- this must not regress for any consumer already parsing it.""" + start = env.httpd_error_log.current_pos() + r = env.curl_get(f'http://[::1]:{IPV6_PORT}/missing', insecure=True) + assert r.response['status'] == 403 + + env.httpd_error_log.add_ignored_matches([r'.*AH01630.*']) + line = env.httpd_error_log.wait_for( + re.compile(r'.*a=::1:\d+ .*A=::1:\d+ .*'), start, timeout=10) + assert line, 'expected unbracketed a=::1: and A=::1:' + + def test_core_016_02_b_flag_adds_brackets(self, env): + """%{b}a/%{b}A bracket the same IPv6 address per RFC 5952.""" + start = env.httpd_error_log.current_pos() + r = env.curl_get(f'http://[::1]:{IPV6_PORT}/missing', insecure=True) + assert r.response['status'] == 403 + + env.httpd_error_log.add_ignored_matches([r'.*AH01630.*']) + line = env.httpd_error_log.wait_for( + re.compile(r'.*ba=\[::1\]:\d+ .*bA=\[::1\]:\d+ .*'), start, + timeout=10) + assert line, 'expected bracketed ba=[::1]: and bA=[::1]:'