From 05f192b6ca7d9a9d41ed17f329f25b4d470997d5 Mon Sep 17 00:00:00 2001 From: Alex Vandiver Date: Wed, 21 Dec 2016 13:38:24 -0800 Subject: [PATCH 1/2] Place an upper limit on the number of redirects to follow (default 5) Without an upper bound on the number of redirects, redirects loops (caused either by client- or server-side bugs) will lead to infinite loops in the collector, potentially preventing any statistics from being reported at all. Set a default of 5 redirects, maximum, with a configurable setting to adjust this. --- src/collectors/httpd/httpd.py | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/collectors/httpd/httpd.py b/src/collectors/httpd/httpd.py index 4ceeab48c..4539451d1 100644 --- a/src/collectors/httpd/httpd.py +++ b/src/collectors/httpd/httpd.py @@ -44,6 +44,7 @@ def get_default_config_help(self): 'urls': "Urls to server-status in auto format, comma seperated," + " Format 'nickname http://host:port/server-status?auto, " + ", nickname http://host:port/server-status?auto, etc'", + 'redirects': "The maximum number of redirect requests to follow.", }) return config_help @@ -53,8 +54,9 @@ def get_default_config(self): """ config = super(HttpdCollector, self).get_default_config() config.update({ - 'path': 'httpd', - 'urls': ['localhost http://localhost:8080/server-status?auto'] + 'path': 'httpd', + 'urls': ['localhost http://localhost:8080/server-status?auto'], + 'redirects': 5, }) return config @@ -63,8 +65,8 @@ def collect(self): url = self.urls[nickname] try: + redirects = 0 while True: - # Parse Url parts = urlparse.urlparse(url) @@ -93,6 +95,10 @@ def collect(self): break url = headers['location'] connection.close() + + redirects += 1 + if redirects > self.config['redirects']: + raise Exception("Too many redirects!") except Exception, e: self.log.error( "Error retrieving HTTPD stats for host %s:%s, url '%s': %s", From 8bd058581d3c87a139423a263894c0af57a8e656 Mon Sep 17 00:00:00 2001 From: Alex Vandiver Date: Wed, 21 Dec 2016 13:49:05 -0800 Subject: [PATCH 2/2] Support HTTPS mod_status URLs and redirects Previously, HttpdCollector assumed all URLs on their default ports were requests to port 80. In the case of a `http://localhost/` -> `https://localhost/` redirect, the latter URL is parsed by `urlparse` as "Host localhost on the default port" (`parts.netloc` aka `parts[1]` did not contain a `:`), and a request is made to `http://localhost/`, which promptly re-redirects to HTTPS again. Transition to using the attribute names of `urlparse` for readability, and support both HTTP and HTTPS URLs, by examining the scheme and constructing the correct httplib class accordingly. --- src/collectors/httpd/httpd.py | 23 +++++++++-------------- 1 file changed, 9 insertions(+), 14 deletions(-) diff --git a/src/collectors/httpd/httpd.py b/src/collectors/httpd/httpd.py index 4539451d1..ee1e5a0e1 100644 --- a/src/collectors/httpd/httpd.py +++ b/src/collectors/httpd/httpd.py @@ -70,21 +70,16 @@ def collect(self): # Parse Url parts = urlparse.urlparse(url) - # Parse host and port - endpoint = parts[1].split(':') - if len(endpoint) > 1: - service_host = endpoint[0] - service_port = int(endpoint[1]) + # Set httplib class + if parts.scheme == 'http': + connection = httplib.HTTPConnection(parts.netloc) + elif parts.scheme == 'https': + connection = httplib.HTTPSConnection(parts.netloc) else: - service_host = endpoint[0] - service_port = 80 + raise Exception("Invalid scheme: %s" % parts.scheme) # Setup Connection - connection = httplib.HTTPConnection(service_host, - service_port) - - url = "%s?%s" % (parts[2], parts[4]) - + url = "%s?%s" % (parts.path, parts.query) connection.request("GET", url) response = connection.getresponse() data = response.read() @@ -101,8 +96,8 @@ def collect(self): raise Exception("Too many redirects!") except Exception, e: self.log.error( - "Error retrieving HTTPD stats for host %s:%s, url '%s': %s", - service_host, str(service_port), url, e) + "Error retrieving HTTPD stats for '%s': %s", + url, e) continue exp = re.compile('^([A-Za-z ]+):\s+(.+)$')