Skip to content

Commit

Permalink
Fix httplib invalid scheme detection for HTTPS (#121). (#122)
Browse files Browse the repository at this point in the history
* Libraries utilizing urllib3 now properly get matched as https when an https request is made.
        + botocore and requests utilize urllib3, so any underlying https request now properly
          identifies as https.
  • Loading branch information
chanchiem authored and haotianw465 committed Jan 18, 2019
1 parent 9e9063f commit 3a44acd
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 3 deletions.
12 changes: 11 additions & 1 deletion aws_xray_sdk/ext/httplib/patch.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
import sys
import wrapt

import urllib3.connection

from aws_xray_sdk.core import xray_recorder
from aws_xray_sdk.core.models import http
from aws_xray_sdk.core.exceptions.exceptions import SegmentNotFoundException
Expand Down Expand Up @@ -92,7 +94,15 @@ def decompose_args(method, url, body, headers, encode_chunked=False):
if subsegment:
inject_trace_header(headers, subsegment)

ssl_cxt = getattr(instance, '_context', None)
if issubclass(instance.__class__, urllib3.connection.HTTPSConnection):
ssl_cxt = getattr(instance, 'ssl_context', None)
elif issubclass(instance.__class__, httplib.HTTPSConnection):
ssl_cxt = getattr(instance, '_context', None)
else:
# In this case, the patcher can't determine which module the connection instance is from.
# We default to it to check ssl_context but may be None so that the default scheme would be
# (and may falsely be) http.
ssl_cxt = getattr(instance, 'ssl_context', None)
scheme = 'https' if ssl_cxt and type(ssl_cxt).__name__ == 'SSLContext' else 'http'
xray_url = '{}://{}{}'.format(scheme, instance.host, url)
xray_data = _XRay_Data(method, instance.host, xray_url)
Expand Down
29 changes: 27 additions & 2 deletions tests/ext/httplib/test_httplib.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,12 +37,15 @@ def construct_ctx():
unpatch()


def _do_req(url, method='GET'):
def _do_req(url, method='GET', use_https=True):
parts = urlparse(url)
host, _, port = parts.netloc.partition(':')
if port == '':
port = None
conn = httplib.HTTPSConnection(parts.netloc, port)
if use_https:
conn = httplib.HTTPSConnection(parts.netloc, port)
else:
conn = httplib.HTTPConnection(parts.netloc, port)

path = '{}?{}'.format(parts.path, parts.query) if parts.query else parts.path
conn.request(method, path)
Expand Down Expand Up @@ -116,3 +119,25 @@ def test_invalid_url():

exception = subsegment.cause['exceptions'][0]
assert exception.type == 'gaierror'


def test_correct_identify_http():
status_code = 200
url = 'http://{}/status/{}?foo=bar&baz=foo'.format(BASE_URL, status_code)
_do_req(url, use_https=False)
subsegment = xray_recorder.current_segment().subsegments[0]
assert subsegment.name == strip_url(url)

http_meta = subsegment.http
assert http_meta['request']['url'].split(":")[0] == 'http'


def test_correct_identify_https():
status_code = 200
url = 'https://{}/status/{}?foo=bar&baz=foo'.format(BASE_URL, status_code)
_do_req(url, use_https=True)
subsegment = xray_recorder.current_segment().subsegments[0]
assert subsegment.name == strip_url(url)

https_meta = subsegment.http
assert https_meta['request']['url'].split(":")[0] == 'https'

0 comments on commit 3a44acd

Please sign in to comment.