From c2adcb58f4f5f221d10599db2c97e6cf828157c1 Mon Sep 17 00:00:00 2001 From: Maximilian Hils Date: Fri, 5 Jan 2018 16:04:02 +0100 Subject: [PATCH 1/4] fix test flow timestamps to values that don't overflow in certain timezones --- mitmproxy/test/tflow.py | 22 ++++++++++----------- mitmproxy/test/tutils.py | 8 ++++---- test/mitmproxy/addons/test_cut.py | 15 +++++++------- test/mitmproxy/addons/test_view.py | 2 +- test/mitmproxy/net/http/test_response.py | 4 ++-- test/mitmproxy/tools/console/test_common.py | 3 --- 6 files changed, 25 insertions(+), 29 deletions(-) diff --git a/mitmproxy/test/tflow.py b/mitmproxy/test/tflow.py index 91747866c..05d194d68 100644 --- a/mitmproxy/test/tflow.py +++ b/mitmproxy/test/tflow.py @@ -53,8 +53,8 @@ def twebsocketflow(client_conn=True, server_conn=True, messages=True, err=None, sec_websocket_version="13", sec_websocket_key="1234", ), - timestamp_start=1, - timestamp_end=2, + timestamp_start=946681200, + timestamp_end=946681201, content=b'' ) resp = http.HTTPResponse( @@ -66,8 +66,8 @@ def twebsocketflow(client_conn=True, server_conn=True, messages=True, err=None, upgrade='websocket', sec_websocket_accept=b'', ), - timestamp_start=1, - timestamp_end=2, + timestamp_start=946681202, + timestamp_end=946681203, content=b'', ) handshake_flow = http.HTTPFlow(client_conn, server_conn) @@ -158,9 +158,9 @@ def tclient_conn(): clientcert=None, mitmcert=None, ssl_established=False, - timestamp_start=1, - timestamp_ssl_setup=2, - timestamp_end=3, + timestamp_start=946681200, + timestamp_ssl_setup=946681201, + timestamp_end=946681206, sni="address", cipher_name="cipher", alpn_proto_negotiated=b"http/1.1", @@ -182,10 +182,10 @@ def tserver_conn(): source_address=("address", 22), ip_address=("192.168.0.1", 22), cert=None, - timestamp_start=1, - timestamp_tcp_setup=2, - timestamp_ssl_setup=3, - timestamp_end=4, + timestamp_start=946681202, + timestamp_tcp_setup=946681203, + timestamp_ssl_setup=946681204, + timestamp_end=946681205, ssl_established=False, sni="address", alpn_proto_negotiated=None, diff --git a/mitmproxy/test/tutils.py b/mitmproxy/test/tutils.py index cd9f3b3f6..d5b52bbec 100644 --- a/mitmproxy/test/tutils.py +++ b/mitmproxy/test/tutils.py @@ -31,8 +31,8 @@ def treq(**kwargs): http_version=b"HTTP/1.1", headers=http.Headers(((b"header", b"qvalue"), (b"content-length", b"7"))), content=b"content", - timestamp_start=1, - timestamp_end=2, + timestamp_start=946681200, + timestamp_end=946681201, ) default.update(kwargs) return http.Request(**default) @@ -49,8 +49,8 @@ def tresp(**kwargs): reason=b"OK", headers=http.Headers(((b"header-response", b"svalue"), (b"content-length", b"7"))), content=b"message", - timestamp_start=1, - timestamp_end=2, + timestamp_start=946681202, + timestamp_end=946681203, ) default.update(kwargs) return http.Response(**default) diff --git a/test/mitmproxy/addons/test_cut.py b/test/mitmproxy/addons/test_cut.py index 71e699db6..97577c60c 100644 --- a/test/mitmproxy/addons/test_cut.py +++ b/test/mitmproxy/addons/test_cut.py @@ -23,8 +23,8 @@ def test_extract(): ["request.text", "content"], ["request.content", b"content"], ["request.raw_content", b"content"], - ["request.timestamp_start", "1"], - ["request.timestamp_end", "2"], + ["request.timestamp_start", "946681200"], + ["request.timestamp_end", "946681201"], ["request.header[header]", "qvalue"], ["response.status_code", "200"], @@ -33,8 +33,8 @@ def test_extract(): ["response.content", b"message"], ["response.raw_content", b"message"], ["response.header[header-response]", "svalue"], - ["response.timestamp_start", "1"], - ["response.timestamp_end", "2"], + ["response.timestamp_start", "946681202"], + ["response.timestamp_end", "946681203"], ["client_conn.address.port", "22"], ["client_conn.address.host", "127.0.0.1"], @@ -49,10 +49,9 @@ def test_extract(): ["server_conn.sni", "address"], ["server_conn.ssl_established", "false"], ] - for t in tests: - ret = cut.extract(t[0], tf) - if ret != t[1]: - raise AssertionError("%s: Expected %s, got %s" % (t[0], t[1], ret)) + for spec, expected in tests: + ret = cut.extract(spec, tf) + assert spec and ret == expected with open(tutils.test_data.path("mitmproxy/net/data/text_cert"), "rb") as f: d = f.read() diff --git a/test/mitmproxy/addons/test_view.py b/test/mitmproxy/addons/test_view.py index 1c76eb219..6e4af367b 100644 --- a/test/mitmproxy/addons/test_view.py +++ b/test/mitmproxy/addons/test_view.py @@ -41,7 +41,7 @@ def test_order_generators(): tf = tflow.tflow(resp=True) rs = view.OrderRequestStart(v) - assert rs.generate(tf) == 1 + assert rs.generate(tf) == 946681200 rm = view.OrderRequestMethod(v) assert rm.generate(tf) == tf.request.method diff --git a/test/mitmproxy/net/http/test_response.py b/test/mitmproxy/net/http/test_response.py index a77435c93..af35bab35 100644 --- a/test/mitmproxy/net/http/test_response.py +++ b/test/mitmproxy/net/http/test_response.py @@ -150,10 +150,10 @@ class TestResponseUtils: n = time.time() r.headers["date"] = email.utils.formatdate(n) pre = r.headers["date"] - r.refresh(1) + r.refresh(946681202) assert pre == r.headers["date"] - r.refresh(61) + r.refresh(946681262) d = email.utils.parsedate_tz(r.headers["date"]) d = email.utils.mktime_tz(d) # Weird that this is not exact... diff --git a/test/mitmproxy/tools/console/test_common.py b/test/mitmproxy/tools/console/test_common.py index 3ab4fd677..a996c0104 100644 --- a/test/mitmproxy/tools/console/test_common.py +++ b/test/mitmproxy/tools/console/test_common.py @@ -1,10 +1,7 @@ from mitmproxy.test import tflow from mitmproxy.tools.console import common -from ....conftest import skip_appveyor - -@skip_appveyor def test_format_flow(): f = tflow.tflow(resp=True) assert common.format_flow(f, True) From 9b03ab59ef2f3c3f40b0db0d204b00d819fcc5f5 Mon Sep 17 00:00:00 2001 From: Maximilian Hils Date: Fri, 5 Jan 2018 16:05:14 +0100 Subject: [PATCH 2/4] minor fixes --- mitmproxy/contentviews/base.py | 4 +++- mitmproxy/tools/console/common.py | 32 +++++++++++--------------- mitmproxy/tools/console/defaultkeys.py | 2 +- 3 files changed, 18 insertions(+), 20 deletions(-) diff --git a/mitmproxy/contentviews/base.py b/mitmproxy/contentviews/base.py index 97740eea0..bdab1e995 100644 --- a/mitmproxy/contentviews/base.py +++ b/mitmproxy/contentviews/base.py @@ -43,9 +43,11 @@ def format_dict( ) -> typing.Iterator[TViewLine]: """ Helper function that transforms the given dictionary into a list of + [ ("key", key ) ("value", value) - tuples, where key is padded to a uniform width. + ] + entries, where key is padded to a uniform width. """ max_key_len = max(len(k) for k in d.keys()) max_key_len = min(max_key_len, KEY_MAX) diff --git a/mitmproxy/tools/console/common.py b/mitmproxy/tools/console/common.py index 47a30272d..1ef1b751c 100644 --- a/mitmproxy/tools/console/common.py +++ b/mitmproxy/tools/console/common.py @@ -205,19 +205,15 @@ def format_flow(f, focus, extended=False, hostheader=False, max_url_len=False): focus=focus, extended=extended, max_url_len=max_url_len, - - intercepted = f.intercepted, - acked = acked, - - req_timestamp = f.request.timestamp_start, - req_is_replay = f.request.is_replay, - req_method = f.request.method, - req_url = f.request.pretty_url if hostheader else f.request.url, - req_http_version = f.request.http_version, - - err_msg = f.error.msg if f.error else None, - - marked = f.marked, + intercepted=f.intercepted, + acked=acked, + req_timestamp=f.request.timestamp_start, + req_is_replay=f.request.is_replay, + req_method=f.request.method, + req_url=f.request.pretty_url if hostheader else f.request.url, + req_http_version=f.request.http_version, + err_msg=f.error.msg if f.error else None, + marked=f.marked, ) if f.response: if f.response.raw_content: @@ -232,11 +228,11 @@ def format_flow(f, focus, extended=False, hostheader=False, max_url_len=False): roundtrip = human.pretty_duration(duration) d.update(dict( - resp_code = f.response.status_code, - resp_reason = f.response.reason, - resp_is_replay = f.response.is_replay, - resp_clen = contentdesc, - roundtrip = roundtrip, + resp_code=f.response.status_code, + resp_reason=f.response.reason, + resp_is_replay=f.response.is_replay, + resp_clen=contentdesc, + roundtrip=roundtrip, )) t = f.response.headers.get("content-type") diff --git a/mitmproxy/tools/console/defaultkeys.py b/mitmproxy/tools/console/defaultkeys.py index f8a3df2df..50941f557 100644 --- a/mitmproxy/tools/console/defaultkeys.py +++ b/mitmproxy/tools/console/defaultkeys.py @@ -59,7 +59,7 @@ def map(km): km.add("M", "view.marked.toggle", ["flowlist"], "Toggle viewing marked flows") km.add( "n", - "console.command view.create get https://google.com", + "console.command view.create get https://example.com/", ["flowlist"], "Create a new flow" ) From 2e2daeed892f46622ae004ba7650cde798de4a5f Mon Sep 17 00:00:00 2001 From: Maximilian Hils Date: Fri, 5 Jan 2018 16:09:43 +0100 Subject: [PATCH 3/4] refactor common.format_keyvals the semantics here were really quite unclear, now it is hopefully a bit more obvious what's happening. Once we are Python 3.6+ exclusively, we may consider changing the signature to accept a (order-preserving) dict instead of a list. --- mitmproxy/tools/console/common.py | 72 +++++++++------- mitmproxy/tools/console/flowdetailview.py | 96 ++++++++++----------- mitmproxy/tools/console/flowview.py | 3 +- mitmproxy/tools/console/help.py | 4 +- test/mitmproxy/tools/console/test_common.py | 25 ++++++ test/mitmproxy/tools/console/test_master.py | 13 --- 6 files changed, 116 insertions(+), 97 deletions(-) diff --git a/mitmproxy/tools/console/common.py b/mitmproxy/tools/console/common.py index 1ef1b751c..8a8427995 100644 --- a/mitmproxy/tools/console/common.py +++ b/mitmproxy/tools/console/common.py @@ -1,9 +1,10 @@ import platform +import typing +from functools import lru_cache import urwid import urwid.util -from functools import lru_cache from mitmproxy.utils import human # Detect Windows Subsystem for Linux @@ -43,41 +44,48 @@ def highlight_key(str, key, textattr="text", keyattr="key"): KEY_MAX = 30 -def format_keyvals(lst, key="key", val="text", indent=0): +def format_keyvals( + entries: typing.List[typing.Tuple[str, typing.Union[None, str, urwid.Widget]]], + key_format: str = "key", + value_format: str = "text", + indent: int = 0 +) -> typing.List[urwid.Columns]: """ - Format a list of (key, value) tuples. + Format a list of (key, value) tuples. - If key is None, it's treated specially: - - We assume a sub-value, and add an extra indent. - - The value is treated as a pre-formatted list of directives. + Args: + entries: The list to format. keys must be strings, values can also be None or urwid widgets. + The latter makes it possible to use the result of format_keyvals() as a value. + key_format: The display attribute for the key. + value_format: The display attribute for the value. + indent: Additional indent to apply. """ + max_key_len = max((len(k) for k, v in entries if k is not None), default=0) + max_key_len = min(max_key_len, KEY_MAX) + + if indent > 2: + indent -= 2 # We use dividechars=2 below, which already adds two empty spaces + ret = [] - if lst: - maxk = min(max(len(i[0]) for i in lst if i and i[0]), KEY_MAX) - for i, kv in enumerate(lst): - if kv is None: - ret.append(urwid.Text("")) - else: - if isinstance(kv[1], urwid.Widget): - v = kv[1] - elif kv[1] is None: - v = urwid.Text("") - else: - v = urwid.Text([(val, kv[1])]) - ret.append( - urwid.Columns( - [ - ("fixed", indent, urwid.Text("")), - ( - "fixed", - maxk, - urwid.Text([(key, kv[0] or "")]) - ), - v - ], - dividechars = 2 - ) - ) + for k, v in entries: + if v is None: + v = urwid.Text("") + elif not isinstance(v, urwid.Widget): + v = urwid.Text([(value_format, v)]) + ret.append( + urwid.Columns( + [ + ("fixed", indent, urwid.Text("")), + ( + "fixed", + max_key_len, + urwid.Text([(key_format, k)]) + ), + v + ], + dividechars=2 + ) + ) return ret diff --git a/mitmproxy/tools/console/flowdetailview.py b/mitmproxy/tools/console/flowdetailview.py index 28fe1fbc0..32ac4b605 100644 --- a/mitmproxy/tools/console/flowdetailview.py +++ b/mitmproxy/tools/console/flowdetailview.py @@ -23,157 +23,157 @@ def flowdetails(state, flow: http.HTTPFlow): metadata = flow.metadata if metadata is not None and len(metadata) > 0: - parts = [[str(k), repr(v)] for k, v in metadata.items()] + parts = [(str(k), repr(v)) for k, v in metadata.items()] text.append(urwid.Text([("head", "Metadata:")])) - text.extend(common.format_keyvals(parts, key="key", val="text", indent=4)) + text.extend(common.format_keyvals(parts, indent=4)) if sc is not None and sc.ip_address: text.append(urwid.Text([("head", "Server Connection:")])) parts = [ - ["Address", human.format_address(sc.address)], + ("Address", human.format_address(sc.address)), ] if sc.ip_address: - parts.append(["Resolved Address", human.format_address(sc.ip_address)]) + parts.append(("Resolved Address", human.format_address(sc.ip_address))) if resp: - parts.append(["HTTP Version", resp.http_version]) + parts.append(("HTTP Version", resp.http_version)) if sc.alpn_proto_negotiated: - parts.append(["ALPN", sc.alpn_proto_negotiated]) + parts.append(("ALPN", sc.alpn_proto_negotiated)) text.extend( - common.format_keyvals(parts, key="key", val="text", indent=4) + common.format_keyvals(parts, indent=4) ) c = sc.cert if c: text.append(urwid.Text([("head", "Server Certificate:")])) parts = [ - ["Type", "%s, %s bits" % c.keyinfo], - ["SHA1 digest", c.digest("sha1")], - ["Valid to", str(c.notafter)], - ["Valid from", str(c.notbefore)], - ["Serial", str(c.serial)], - [ + ("Type", "%s, %s bits" % c.keyinfo), + ("SHA1 digest", c.digest("sha1")), + ("Valid to", str(c.notafter)), + ("Valid from", str(c.notbefore)), + ("Serial", str(c.serial)), + ( "Subject", urwid.BoxAdapter( urwid.ListBox( common.format_keyvals( c.subject, - key="highlight", - val="text" + key_format="highlight" ) ), len(c.subject) ) - ], - [ + ), + ( "Issuer", urwid.BoxAdapter( urwid.ListBox( common.format_keyvals( - c.issuer, key="highlight", val="text" + c.issuer, + key_format="highlight" ) ), len(c.issuer) ) - ] + ) ] if c.altnames: parts.append( - [ + ( "Alt names", ", ".join(strutils.bytes_to_escaped_str(x) for x in c.altnames) - ] + ) ) text.extend( - common.format_keyvals(parts, key="key", val="text", indent=4) + common.format_keyvals(parts, indent=4) ) if cc is not None: text.append(urwid.Text([("head", "Client Connection:")])) parts = [ - ["Address", "{}:{}".format(cc.address[0], cc.address[1])], + ("Address", "{}:{}".format(cc.address[0], cc.address[1])), ] if req: - parts.append(["HTTP Version", req.http_version]) + parts.append(("HTTP Version", req.http_version)) if cc.tls_version: - parts.append(["TLS Version", cc.tls_version]) + parts.append(("TLS Version", cc.tls_version)) if cc.sni: - parts.append(["Server Name Indication", cc.sni]) + parts.append(("Server Name Indication", cc.sni)) if cc.cipher_name: - parts.append(["Cipher Name", cc.cipher_name]) + parts.append(("Cipher Name", cc.cipher_name)) if cc.alpn_proto_negotiated: - parts.append(["ALPN", cc.alpn_proto_negotiated]) + parts.append(("ALPN", cc.alpn_proto_negotiated)) text.extend( - common.format_keyvals(parts, key="key", val="text", indent=4) + common.format_keyvals(parts, indent=4) ) parts = [] if cc is not None and cc.timestamp_start: parts.append( - [ + ( "Client conn. established", maybe_timestamp(cc, "timestamp_start") - ] + ) ) if cc.ssl_established: parts.append( - [ + ( "Client conn. TLS handshake", maybe_timestamp(cc, "timestamp_ssl_setup") - ] + ) ) if sc is not None and sc.timestamp_start: parts.append( - [ + ( "Server conn. initiated", maybe_timestamp(sc, "timestamp_start") - ] + ) ) parts.append( - [ + ( "Server conn. TCP handshake", maybe_timestamp(sc, "timestamp_tcp_setup") - ] + ) ) if sc.ssl_established: parts.append( - [ + ( "Server conn. TLS handshake", maybe_timestamp(sc, "timestamp_ssl_setup") - ] + ) ) if req is not None and req.timestamp_start: parts.append( - [ + ( "First request byte", maybe_timestamp(req, "timestamp_start") - ] + ) ) parts.append( - [ + ( "Request complete", maybe_timestamp(req, "timestamp_end") - ] + ) ) if resp is not None and resp.timestamp_start: parts.append( - [ + ( "First response byte", maybe_timestamp(resp, "timestamp_start") - ] + ) ) parts.append( - [ + ( "Response complete", maybe_timestamp(resp, "timestamp_end") - ] + ) ) if parts: @@ -181,6 +181,6 @@ def flowdetails(state, flow: http.HTTPFlow): parts = sorted(parts, key=lambda p: p[1]) text.append(urwid.Text([("head", "Timing:")])) - text.extend(common.format_keyvals(parts, key="key", val="text", indent=4)) + text.extend(common.format_keyvals(parts, indent=4)) return searchable.Searchable(text) diff --git a/mitmproxy/tools/console/flowview.py b/mitmproxy/tools/console/flowview.py index 05d2573f8..8d572f7b1 100644 --- a/mitmproxy/tools/console/flowview.py +++ b/mitmproxy/tools/console/flowview.py @@ -154,8 +154,7 @@ class FlowDetails(tabs.Tabs): if conn: txt = common.format_keyvals( [(h + ":", v) for (h, v) in conn.headers.items(multi=True)], - key = "header", - val = "text" + key_format="header" ) viewmode = self.master.commands.call("console.flowview.mode") msg, body = self.content_view(viewmode, conn) diff --git a/mitmproxy/tools/console/help.py b/mitmproxy/tools/console/help.py index 439289f63..1b4b9ac61 100644 --- a/mitmproxy/tools/console/help.py +++ b/mitmproxy/tools/console/help.py @@ -76,7 +76,7 @@ class HelpView(tabs.Tabs, layoutwidget.LayoutWidget): def filtexp(self): text = [] - text.extend(common.format_keyvals(flowfilter.help, key="key", val="text", indent=4)) + text.extend(common.format_keyvals(flowfilter.help, indent=4)) text.append( urwid.Text( [ @@ -96,7 +96,7 @@ class HelpView(tabs.Tabs, layoutwidget.LayoutWidget): ("!(~q & ~t \"text/html\")", "Anything but requests with a text/html content type."), ] text.extend( - common.format_keyvals(examples, key="key", val="text", indent=4) + common.format_keyvals(examples, indent=4) ) return CListBox(text) diff --git a/test/mitmproxy/tools/console/test_common.py b/test/mitmproxy/tools/console/test_common.py index a996c0104..72438c496 100644 --- a/test/mitmproxy/tools/console/test_common.py +++ b/test/mitmproxy/tools/console/test_common.py @@ -1,3 +1,5 @@ +import urwid + from mitmproxy.test import tflow from mitmproxy.tools.console import common @@ -7,3 +9,26 @@ def test_format_flow(): assert common.format_flow(f, True) assert common.format_flow(f, True, hostheader=True) assert common.format_flow(f, True, extended=True) + + +def test_format_keyvals(): + assert common.format_keyvals( + [ + ("aa", "bb"), + ("cc", "dd"), + ("ee", None), + ] + ) + wrapped = urwid.BoxAdapter( + urwid.ListBox( + urwid.SimpleFocusListWalker( + common.format_keyvals([("foo", "bar")]) + ) + ), 1 + ) + assert wrapped.render((30, )) + assert common.format_keyvals( + [ + ("aa", wrapped) + ] + ) diff --git a/test/mitmproxy/tools/console/test_master.py b/test/mitmproxy/tools/console/test_master.py index 3aa0dc546..9779a4824 100644 --- a/test/mitmproxy/tools/console/test_master.py +++ b/test/mitmproxy/tools/console/test_master.py @@ -4,22 +4,9 @@ from mitmproxy import options from mitmproxy.test import tflow from mitmproxy.test import tutils from mitmproxy.tools import console -from mitmproxy.tools.console import common from ... import tservers -def test_format_keyvals(): - assert common.format_keyvals( - [ - ("aa", "bb"), - None, - ("cc", "dd"), - (None, "dd"), - (None, "dd"), - ] - ) - - def test_options(): assert options.Options(replay_kill_extra=True) From a6bd53534b0a89c4ef055bc38b26660712226db8 Mon Sep 17 00:00:00 2001 From: Maximilian Hils Date: Fri, 5 Jan 2018 16:12:14 +0100 Subject: [PATCH 4/4] fix #1833 --- mitmproxy/tools/console/flowview.py | 25 ++++++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/mitmproxy/tools/console/flowview.py b/mitmproxy/tools/console/flowview.py index 8d572f7b1..a4b629d40 100644 --- a/mitmproxy/tools/console/flowview.py +++ b/mitmproxy/tools/console/flowview.py @@ -13,6 +13,7 @@ from mitmproxy.tools.console import flowdetailview from mitmproxy.tools.console import searchable from mitmproxy.tools.console import tabs import mitmproxy.tools.console.master # noqa +from mitmproxy.utils import strutils class SearchError(Exception): @@ -152,8 +153,30 @@ class FlowDetails(tabs.Tabs): def conn_text(self, conn): if conn: + hdrs = [] + for k, v in conn.headers.fields: + # This will always force an ascii representation of headers. For example, if the server sends a + # + # X-Authors: Made with ❤ in Hamburg + # + # header, mitmproxy will display the following: + # + # X-Authors: Made with \xe2\x9d\xa4 in Hamburg. + # + # The alternative would be to just use the header's UTF-8 representation and maybe + # do `str.replace("\t", "\\t")` to exempt tabs from urwid's special characters escaping [1]. + # That would in some terminals allow rendering UTF-8 characters, but the mapping + # wouldn't be bijective, i.e. a user couldn't distinguish "\\t" and "\t". + # Also, from a security perspective, a mitmproxy user couldn't be fooled by homoglyphs. + # + # 1) https://github.com/mitmproxy/mitmproxy/issues/1833 + # https://github.com/urwid/urwid/blob/6608ee2c9932d264abd1171468d833b7a4082e13/urwid/display_common.py#L35-L36, + + k = strutils.bytes_to_escaped_str(k) + ":" + v = strutils.bytes_to_escaped_str(v) + hdrs.append((k, v)) txt = common.format_keyvals( - [(h + ":", v) for (h, v) in conn.headers.items(multi=True)], + hdrs, key_format="header" ) viewmode = self.master.commands.call("console.flowview.mode")