[checkmk-commits] 7385 FIX check_http: Fix port mixup in URL-mode

Module: check_mk Branch: master Commit: 0c171e0e780f5a9ab15711f2d1531d2dc7fecb0e URL: http://git.mathias-kettner.de/git/?p=check_mk.git;a=commit;h=0c171e0e780f5a… Author: Moritz Kiemer <mo(a)mathias-kettner.de> Date: Mon Apr 1 09:38:08 2019 +0200 7385 FIX check_http: Fix port mixup in URL-mode The check_http check in URL mode mixed up the servers port and the proxy port if a proxy was used. If users were using a proxy to check a URL, the configured port was used as the proxys port, while it was not possible to configure a port for the server. Users who have been relying on this wrong behaviour need to edit their rules. The rule has been reworked to clearly show which settings apply the server, and which to the proxy. Change-Id: I2eef53f196032f8b718b3a7aad4c49832ba27191 --- .werks/7385 | 14 +++++++++++ checks/check_http | 46 +++++++++++++++--------------------- tests/unit/checks/test_check_http.py | 35 +++++++++++++++++++-------- 3 files changed, 58 insertions(+), 37 deletions(-) diff --git a/.werks/7385 b/.werks/7385 new file mode 100644 index 0000000..f110d77 --- /dev/null +++ b/.werks/7385 @@ -0,0 +1,14 @@ +Title: check_http: Fix port mixup in URL-mode +Level: 1 +Component: checks +Compatible: incomp +Edition: cre +Version: 1.6.0i1 +Date: 1554103908 +Class: fix + +The check_http check in URL mode mixed up the servers port and the proxy port if a proxy was used. +If users were using a proxy to check a URL, the configured port was used as the proxys port, while +it was not possible to configure a port for the server. Users who have been relying on this wrong +behaviour need to edit their rules. +The rule has been reworked to clearly show which settings apply the server, and which to the proxy. diff --git a/checks/check_http b/checks/check_http index 5e63dd1..7f9b75c 100644 --- a/checks/check_http +++ b/checks/check_http @@ -105,41 +105,19 @@ def _certificate_args(host, proxy, settings, sni_flag): warn, crit = settings["cert_days"] args += ['-C', '%d,%d' % (warn, crit)] - specify_port = proxy.port if proxy else host.port - if specify_port: - args += ['-p', specify_port] - if proxy: args += ['--ssl', '-j', 'CONNECT'] - args.append(proxy.address) - elif sni_flag: args += ['-H', host.address] - address = host.address - if proxy and host.port: - address += ':%s' % host.port - args += [address] - return args def _url_args(host, proxy, settings): args = [] - if proxy: - args += ["-I", proxy.address] - args += ["-H", host.address] - else: - args += ["-I", host.address] - # Proxy *and* virthost is not allowed - if "virthost" in settings: - args += ["-H", settings["virthost"]] - - # TODO: I think this should be overridden by the proxy port - # in the same way as in the cert check. (mo) - if host.port: - args += ['-p', host.port] + if "virthost" in settings: + args += ["-H", settings["virthost"]] if "uri" in settings: args += ['-u', settings["uri"]] @@ -230,6 +208,18 @@ def _common_args(host, proxy, sni_flag): if proxy and proxy.auth: args += ["-b", proxy.auth] + specify_port = proxy.port if proxy else host.port + if specify_port: + args += ['-p', specify_port] + + # last two arguments correspond to -I/-H, respectively + if proxy: + args += [proxy.address] + address = host.address + if proxy and host.port: + address += ':%s' % host.port + args += [address] + return args @@ -242,10 +232,12 @@ def check_http_arguments(params): proxy = _get_proxy(params) sni_flag = bool(params.get("sni")) - args = _common_args(host, proxy, sni_flag) if mode_name == 'cert': - return args + _certificate_args(host, proxy, settings, sni_flag) - return args + _url_args(host, proxy, settings) + args = _certificate_args(host, proxy, settings, sni_flag) + else: + args = _url_args(host, proxy, settings) + + return args + _common_args(host, proxy, sni_flag) def check_http_description(params): diff --git a/tests/unit/checks/test_check_http.py b/tests/unit/checks/test_check_http.py index 037ee8a..5426dc3 100644 --- a/tests/unit/checks/test_check_http.py +++ b/tests/unit/checks/test_check_http.py @@ -13,8 +13,15 @@ pytestmark = pytest.mark.checks 'virthost': ('www.test123.de', True) }), [ - '-I', 'www.test123.de', '-H', 'www.test123.de', '-p', 80, '-u', '/images', - '--onredirect=follow', '-L' + '-H', + 'www.test123.de', + '-u', + '/images', + '--onredirect=follow', + '-L', + '-p', + 80, + 'www.test123.de', ], ), ( @@ -28,8 +35,16 @@ pytestmark = pytest.mark.checks 'virthost': ('www.test123.de', True) }), [ - '-I', '163.172.86.64', '-H', 'www.test123.de', '-p', 3128, '-u', '/images', '--ssl', - '--extended-perfdata', '-j', 'CONNECT' + '-H', + 'www.test123.de', + '-u', + '/images', + '--ssl', + '--extended-perfdata', + '-j', + 'CONNECT', + '163.172.86.64', + 'www.test123.de:3128', ], ), ( @@ -56,7 +71,7 @@ pytestmark = pytest.mark.checks 'port': '42', 'proxy': 'p.roxy:23', }), - ['-C', '10,20', '-p', '23', '--ssl', '-j', 'CONNECT', 'p.roxy', 'www.test123.com:42'], + ['-C', '10,20', '--ssl', '-j', 'CONNECT', '-p', '23', 'p.roxy', 'www.test123.com:42'], ), ( (None, { @@ -66,7 +81,7 @@ pytestmark = pytest.mark.checks 'proxy': '[dead:beef::face]:23', }), [ - '-C', '10,20', '-p', '23', '--ssl', '-j', 'CONNECT', '[dead:beef::face]', + '-C', '10,20', '--ssl', '-j', 'CONNECT', '-p', '23', '[dead:beef::face]', 'www.test123.com:42' ], ), @@ -75,26 +90,26 @@ pytestmark = pytest.mark.checks 'virthost': ("virtual.host", True), 'proxy': "foo.bar", }), - ['-I', 'foo.bar', '-H', 'virtual.host'], + ['-H', 'virtual.host', 'foo.bar', 'virtual.host'], ), ( (None, { 'virthost': ("virtual.host", False), 'proxy': "foo.bar", }), - ['-I', 'foo.bar', '-H', 'virtual.host'], + ['-H', 'virtual.host', 'foo.bar', 'virtual.host'], ), ( (None, { 'virthost': ("virtual.host", True), }), - ['-I', 'virtual.host', '-H', 'virtual.host'], + ['-H', 'virtual.host', 'virtual.host'], ), ( (None, { 'virthost': ("virtual.host", False), }), - ['-I', '$_HOSTADDRESS_4$', '-H', 'virtual.host'], + ['-H', 'virtual.host', '$_HOSTADDRESS_4$'], ), ]) def test_check_http_argument_parsing(check_manager, params, expected_args):