Fix to only add assert_hostname for https urls (#578)

* Fix to only add assert_hostname for https urls

Given an http:// server url and `'VERIFY_SERVER_CERT': False`,
don't pass in assert_hostname (it will error)

* Update changelog

* Use url_parts and add additional test

Author: basepi

CHANGELOG.md

@@ -1,5 +1,11 @@
 # Changelog
 
+## Unreleased
+[Check the diff](https://github.com/elastic/apm-agent-python/compare/v5.1.1...master)
+
+### Bugfixes
+ * fixed an issue with http server_url and `'VERIFY_SERVER_CERT': False` (#570, #578)
+
 ## v5.1.1 
 [Check the diff](https://github.com/elastic/apm-agent-python/compare/v5.1.0...v5.1.1)
 

elasticapm/transport/http.py

@@ -51,12 +51,12 @@ def __init__(self, url, **kwargs):
         super(Transport, self).__init__(url, **kwargs)
         url_parts = compat.urlparse.urlparse(url)
         pool_kwargs = {"cert_reqs": "CERT_REQUIRED", "ca_certs": certifi.where(), "block": True}
-        if self._server_cert:
+        if self._server_cert and url_parts.scheme != "http":
             pool_kwargs.update(
                 {"assert_fingerprint": self.cert_fingerprint, "assert_hostname": False, "cert_reqs": ssl.CERT_NONE}
             )
             del pool_kwargs["ca_certs"]
-        elif not self._verify_server_cert:
+        elif not self._verify_server_cert and url_parts.scheme != "http":
             pool_kwargs["cert_reqs"] = ssl.CERT_NONE
             pool_kwargs["assert_hostname"] = False
         proxies = compat.getproxies_environment()

tests/transports/test_urllib3.py

@@ -188,6 +188,38 @@ def test_ssl_verify_disable(waiting_httpsserver):
         transport.close()
 
 
+def test_ssl_verify_disable_http(waiting_httpserver):
+    """
+    Make sure that ``assert_hostname`` isn't passed in for http requests, even
+    with verify_server_cert=False
+    """
+    waiting_httpserver.serve_content(code=202, content="", headers={"Location": "http://example.com/foo"})
+    transport = Transport(waiting_httpserver.url, verify_server_cert=False)
+    try:
+        url = transport.send(compat.b("x"))
+        assert url == "http://example.com/foo"
+    finally:
+        transport.close()
+
+
+def test_ssl_cert_pinning_http(waiting_httpserver):
+    """
+    Won't fail, as with the other cert pinning test, since certs aren't relevant
+    for http, only https.
+    """
+    waiting_httpserver.serve_content(code=202, content="", headers={"Location": "http://example.com/foo"})
+    transport = Transport(
+        waiting_httpserver.url,
+        server_cert=os.path.join(os.path.dirname(__file__), "wrong_cert.pem"),
+        verify_server_cert=True,
+    )
+    try:
+        url = transport.send(compat.b("x"))
+        assert url == "http://example.com/foo"
+    finally:
+        transport.close()
+
+
 def test_ssl_cert_pinning(waiting_httpsserver):
     waiting_httpsserver.serve_content(code=202, content="", headers={"Location": "https://example.com/foo"})
     cur_dir = os.path.dirname(os.path.realpath(__file__))