From 23a8092dbf48b8912c13bf0f3a7b74b44d4761ab Mon Sep 17 00:00:00 2001 From: Abhinav Singh Date: Fri, 5 Nov 2021 07:44:18 +0530 Subject: [PATCH 01/11] Rename is_py3 to is_py2 for more logical guard --- proxy/proxy.py | 6 +++--- tests/test_main.py | 50 ++++++++++++++++++++++++++++++++++++---------- 2 files changed, 42 insertions(+), 14 deletions(-) diff --git a/proxy/proxy.py b/proxy/proxy.py index 330c7a9cdb..5426c43523 100644 --- a/proxy/proxy.py +++ b/proxy/proxy.py @@ -205,7 +205,7 @@ def initialize( if input_args is None: input_args = [] - if not Proxy.is_py3(): + if Proxy.is_py2(): print(PY2_DEPRECATION_MESSAGE) sys.exit(1) @@ -476,9 +476,9 @@ def get_default_plugins( return default_plugins @staticmethod - def is_py3() -> bool: + def is_py2() -> bool: """Exists only to avoid mocking sys.version_info in tests.""" - return sys.version_info[0] != 2 + return sys.version_info[0] == 2 @staticmethod def set_open_file_limit(soft_limit: int) -> None: diff --git a/tests/test_main.py b/tests/test_main.py index ec99edbc58..de9157c3a1 100644 --- a/tests/test_main.py +++ b/tests/test_main.py @@ -15,7 +15,7 @@ from unittest import mock from typing import List -from proxy.proxy import main, Proxy +from proxy.proxy import main, Proxy, entry_point from proxy.common.utils import bytes_ from proxy.http.handler import HttpProtocolHandler @@ -72,7 +72,35 @@ def mock_default_args(mock_args: mock.Mock) -> None: @mock.patch('proxy.proxy.EventManager') @mock.patch('proxy.proxy.AcceptorPool') @mock.patch('logging.basicConfig') - def test_init_with_no_arguments( + def test_entry_point( + self, + mock_logging_config: mock.Mock, + mock_acceptor_pool: mock.Mock, + mock_event_manager: mock.Mock, + mock_initialize: mock.Mock, + mock_sleep: mock.Mock, + ) -> None: + mock_sleep.side_effect = KeyboardInterrupt() + + mock_initialize.return_value.enable_events = False + entry_point() + + mock_event_manager.assert_not_called() + mock_acceptor_pool.assert_called_with( + flags=mock_initialize.return_value, + work_klass=HttpProtocolHandler, + event_queue=None, + ) + mock_acceptor_pool.return_value.setup.assert_called() + mock_acceptor_pool.return_value.shutdown.assert_called() + mock_sleep.assert_called() + + @mock.patch('time.sleep') + @mock.patch('proxy.proxy.Proxy.initialize') + @mock.patch('proxy.proxy.EventManager') + @mock.patch('proxy.proxy.AcceptorPool') + @mock.patch('logging.basicConfig') + def test_main_with_no_arguments( self, mock_logging_config: mock.Mock, mock_acceptor_pool: mock.Mock, @@ -133,7 +161,7 @@ def test_pid_file_is_written_and_removed( @mock.patch('time.sleep') @mock.patch('proxy.proxy.EventManager') @mock.patch('proxy.proxy.AcceptorPool') - def test_basic_auth( + def test_basic_auth_flag_is_base64_encoded( self, mock_acceptor_pool: mock.Mock, mock_event_manager: mock.Mock, @@ -156,10 +184,10 @@ def test_basic_auth( @mock.patch('builtins.print') @mock.patch('proxy.proxy.EventManager') @mock.patch('proxy.proxy.AcceptorPool') - @mock.patch('proxy.proxy.Proxy.is_py3') + @mock.patch('proxy.proxy.Proxy.is_py2') def test_main_py3_runs( self, - mock_is_py3: mock.Mock, + mock_is_py2: mock.Mock, mock_acceptor_pool: mock.Mock, mock_event_manager: mock.Mock, mock_print: mock.Mock, @@ -168,11 +196,11 @@ def test_main_py3_runs( mock_sleep.side_effect = KeyboardInterrupt() input_args = ['--basic-auth', 'user:pass'] - mock_is_py3.return_value = True + mock_is_py2.return_value = False main(input_args, num_workers=1) - mock_is_py3.assert_called() + mock_is_py2.assert_called() mock_print.assert_not_called() mock_event_manager.assert_not_called() @@ -180,18 +208,18 @@ def test_main_py3_runs( mock_acceptor_pool.return_value.setup.assert_called() @mock.patch('builtins.print') - @mock.patch('proxy.proxy.Proxy.is_py3') + @mock.patch('proxy.proxy.Proxy.is_py2') def test_main_py2_exit( self, - mock_is_py3: mock.Mock, + mock_is_py2: mock.Mock, mock_print: mock.Mock, ) -> None: - mock_is_py3.return_value = False + mock_is_py2.return_value = True with self.assertRaises(SystemExit) as e: main(num_workers=1) mock_print.assert_called_with(PY2_DEPRECATION_MESSAGE) self.assertEqual(e.exception.code, 1) - mock_is_py3.assert_called() + mock_is_py2.assert_called() @mock.patch('builtins.print') def test_main_version( From a96f4028fd3342561909c579e096c5b3d4be5701 Mon Sep 17 00:00:00 2001 From: Abhinav Singh Date: Fri, 5 Nov 2021 08:15:14 +0530 Subject: [PATCH 02/11] Add stubs for missing tests, add few more tests for core modules --- proxy/http/parser.py | 6 ++++-- tests/http/test_http_parser.py | 26 ++++++++++++++++++++++- tests/http/test_http_proxy.py | 39 ++++++++++++++++++++++++++++++++++ tests/test_main.py | 15 +++++++++++++ 4 files changed, 83 insertions(+), 3 deletions(-) diff --git a/proxy/http/parser.py b/proxy/http/parser.py index 597e641de1..927ca44270 100644 --- a/proxy/http/parser.py +++ b/proxy/http/parser.py @@ -180,7 +180,8 @@ def parse(self, raw: bytes) -> None: more = False else: raise NotImplementedError( - 'Parser shouldn\'t have reached here', + 'Parser shouldn\'t have reached here. ' + + 'This can happen when content length header is missing but their is a body in the payload', ) else: more, raw = self.process(raw) @@ -285,7 +286,8 @@ def build_response(self) -> bytes: headers={} if not self.headers else { self.headers[k][0]: self.headers[k][1] for k in self.headers }, - body=self.body if not self.is_chunked_encoded() else ChunkParser.to_chunks(self.body), + body=self.body if not self.is_chunked_encoded( + ) else ChunkParser.to_chunks(self.body), ) def has_host(self) -> bool: diff --git a/tests/http/test_http_parser.py b/tests/http/test_http_parser.py index ded49c8283..3355920ddc 100644 --- a/tests/http/test_http_parser.py +++ b/tests/http/test_http_parser.py @@ -272,7 +272,8 @@ def test_get_partial_parse2(self) -> None: self.parser.parse(b'Content-Type: text/plain' + CRLF) self.assertEqual(self.parser.buffer, b'') self.assertEqual( - self.parser.headers[b'content-type'], (b'Content-Type', b'text/plain'), + self.parser.headers[b'content-type'], (b'Content-Type', + b'text/plain'), ) self.assertEqual( self.parser.state, @@ -632,3 +633,26 @@ def test_paramiko_doc(self) -> None: self.parser = HttpParser(httpParserTypes.RESPONSE_PARSER) self.parser.parse(response) self.assertEqual(self.parser.state, httpParserStates.COMPLETE) + + def test_request_factory(self) -> None: + r = HttpParser.request( + b'POST http://localhost:12345 HTTP/1.1' + CRLF + b'key: value' + CRLF + b'Content-Length: 13' + CRLF + CRLF + b'Hello from py') + self.assertEqual(r.host, b'localhost') + self.assertEqual(r.port, 12345) + self.assertEqual(r.path, b'/') + self.assertEqual(r.header(b'key'), b'value') + self.assertEqual(r.header(b'KEY'), b'value') + self.assertEqual(r.header(b'content-length'), b'13') + self.assertEqual(r.body, b'Hello from py') + + def test_response_factory(self) -> None: + r = HttpParser.response( + b'HTTP/1.1 200 OK\r\nkey: value\r\n\r\n') + self.assertEqual(r.code, b'200') + self.assertEqual(r.reason, b'OK') + self.assertEqual(r.header(b'key'), b'value') + + def test_parser_shouldnt_have_reached_here(self) -> None: + with self.assertRaises(NotImplementedError): + HttpParser.request(b'POST http://localhost:12345 HTTP/1.1' + CRLF + + b'key: value' + CRLF + CRLF + b'Hello from py') diff --git a/tests/http/test_http_proxy.py b/tests/http/test_http_proxy.py index 13a55a5b3c..5616b9e75f 100644 --- a/tests/http/test_http_proxy.py +++ b/tests/http/test_http_proxy.py @@ -115,3 +115,42 @@ def test_proxy_plugin_before_upstream_connection_can_teardown( self.protocol_handler.run_once() mock_server_conn.assert_not_called() self.plugin.return_value.before_upstream_connection.assert_called() + + def test_proxy_plugin_plugins_can_teardown_from_write_to_descriptors(self) -> None: + pass + + def test_proxy_plugin_retries_on_ssl_want_write_error(self) -> None: + pass + + def test_proxy_plugin_broken_pipe_error_on_write_will_teardown(self) -> None: + pass + + def test_proxy_plugin_plugins_can_teardown_from_read_from_descriptors(self) -> None: + pass + + def test_proxy_plugin_retries_on_ssl_want_read_error(self) -> None: + pass + + def test_proxy_plugin_timeout_error_on_read_will_teardown(self) -> None: + pass + + def test_proxy_plugin_invokes_handle_pipeline_response(self) -> None: + pass + + def test_proxy_plugin_invokes_on_access_log(self) -> None: + pass + + def test_proxy_plugin_skips_server_teardown_when_client_closes_and_server_never_initialized(self) -> None: + pass + + def test_proxy_plugin_invokes_handle_client_data(self) -> None: + pass + + def test_proxy_plugin_handles_pipeline_response(self) -> None: + pass + + def test_proxy_plugin_invokes_resolve_dns(self) -> None: + pass + + def test_proxy_plugin_require_both_host_port_to_connect(self) -> None: + pass diff --git a/tests/test_main.py b/tests/test_main.py index de9157c3a1..ccbdd4a751 100644 --- a/tests/test_main.py +++ b/tests/test_main.py @@ -230,3 +230,18 @@ def test_main_version( main(['--version']) mock_print.assert_called_with(__version__) self.assertEqual(e.exception.code, 0) + + def test_enable_dashboard(self) -> None: + pass + + def test_enable_devtools(self) -> None: + pass + + def test_pac_file(self) -> None: + pass + + def test_imports_plugin(self) -> None: + pass + + def test_cannot_enable_https_proxy_and_tls_interception_mutually(self) -> None: + pass From 834cce3f9e000fde4900f16de5283c9571760784 Mon Sep 17 00:00:00 2001 From: Abhinav Singh Date: Fri, 5 Nov 2021 08:16:46 +0530 Subject: [PATCH 03/11] Lint fixes --- tests/http/test_http_parser.py | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/tests/http/test_http_parser.py b/tests/http/test_http_parser.py index 3355920ddc..feeaffe7d3 100644 --- a/tests/http/test_http_parser.py +++ b/tests/http/test_http_parser.py @@ -272,8 +272,10 @@ def test_get_partial_parse2(self) -> None: self.parser.parse(b'Content-Type: text/plain' + CRLF) self.assertEqual(self.parser.buffer, b'') self.assertEqual( - self.parser.headers[b'content-type'], (b'Content-Type', - b'text/plain'), + self.parser.headers[b'content-type'], ( + b'Content-Type', + b'text/plain', + ), ) self.assertEqual( self.parser.state, @@ -636,7 +638,8 @@ def test_paramiko_doc(self) -> None: def test_request_factory(self) -> None: r = HttpParser.request( - b'POST http://localhost:12345 HTTP/1.1' + CRLF + b'key: value' + CRLF + b'Content-Length: 13' + CRLF + CRLF + b'Hello from py') + b'POST http://localhost:12345 HTTP/1.1' + CRLF + b'key: value' + CRLF + b'Content-Length: 13' + CRLF + CRLF + b'Hello from py', + ) self.assertEqual(r.host, b'localhost') self.assertEqual(r.port, 12345) self.assertEqual(r.path, b'/') @@ -647,12 +650,15 @@ def test_request_factory(self) -> None: def test_response_factory(self) -> None: r = HttpParser.response( - b'HTTP/1.1 200 OK\r\nkey: value\r\n\r\n') + b'HTTP/1.1 200 OK\r\nkey: value\r\n\r\n', + ) self.assertEqual(r.code, b'200') self.assertEqual(r.reason, b'OK') self.assertEqual(r.header(b'key'), b'value') def test_parser_shouldnt_have_reached_here(self) -> None: with self.assertRaises(NotImplementedError): - HttpParser.request(b'POST http://localhost:12345 HTTP/1.1' + CRLF + - b'key: value' + CRLF + CRLF + b'Hello from py') + HttpParser.request( + b'POST http://localhost:12345 HTTP/1.1' + CRLF + + b'key: value' + CRLF + CRLF + b'Hello from py', + ) From 09c35f441726ead579cc9bf27a783ba5ca6f796d Mon Sep 17 00:00:00 2001 From: Abhinav Singh Date: Fri, 5 Nov 2021 08:18:25 +0530 Subject: [PATCH 04/11] Line too long fix --- tests/http/test_http_parser.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/http/test_http_parser.py b/tests/http/test_http_parser.py index feeaffe7d3..29e0cebdf2 100644 --- a/tests/http/test_http_parser.py +++ b/tests/http/test_http_parser.py @@ -638,7 +638,10 @@ def test_paramiko_doc(self) -> None: def test_request_factory(self) -> None: r = HttpParser.request( - b'POST http://localhost:12345 HTTP/1.1' + CRLF + b'key: value' + CRLF + b'Content-Length: 13' + CRLF + CRLF + b'Hello from py', + b'POST http://localhost:12345 HTTP/1.1' + CRLF + + b'key: value' + CRLF + + b'Content-Length: 13' + CRLF + CRLF + + b'Hello from py', ) self.assertEqual(r.host, b'localhost') self.assertEqual(r.port, 12345) From 00bcecc37d6fa44c14c2b1f189a0709a1fa8293a Mon Sep 17 00:00:00 2001 From: Abhinav Singh Date: Fri, 5 Nov 2021 08:24:01 +0530 Subject: [PATCH 05/11] Remove unnecessary KeyboardInterrupt --- proxy/core/acceptor/threadless.py | 2 -- proxy/core/acceptor/work.py | 2 +- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/proxy/core/acceptor/threadless.py b/proxy/core/acceptor/threadless.py index 8caf21739b..5ed8ac36e2 100644 --- a/proxy/core/acceptor/threadless.py +++ b/proxy/core/acceptor/threadless.py @@ -198,8 +198,6 @@ def run(self) -> None: self.loop = asyncio.get_event_loop() while not self.running.is_set(): self.run_once() - except KeyboardInterrupt: - pass finally: assert self.selector is not None self.selector.unregister(self.client_queue) diff --git a/proxy/core/acceptor/work.py b/proxy/core/acceptor/work.py index dcfabc2831..5556a31941 100644 --- a/proxy/core/acceptor/work.py +++ b/proxy/core/acceptor/work.py @@ -73,7 +73,7 @@ def run(self) -> None: compatibility with threaded mode where work class is started as a separate thread. """ - pass + pass # pragma: no cover def publish_event( self, From 6baac3c0d7d787ce6af34689229f83bef5da937e Mon Sep 17 00:00:00 2001 From: Abhinav Singh Date: Fri, 5 Nov 2021 08:29:02 +0530 Subject: [PATCH 06/11] Consistent workflow names --- .github/workflows/test-brew.yml | 4 ++-- .github/workflows/test-dashboard.yml | 4 ++-- .github/workflows/test-docker.yml | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/.github/workflows/test-brew.yml b/.github/workflows/test-brew.yml index c2cff08779..7c31ce1f50 100644 --- a/.github/workflows/test-brew.yml +++ b/.github/workflows/test-brew.yml @@ -1,12 +1,12 @@ --- -name: Proxy.py Brew +name: brew on: [push, pull_request] # yamllint disable-line rule:truthy jobs: build: runs-on: ${{ matrix.os }}-latest - name: Brew - Python ${{ matrix.python }} on ${{ matrix.os }} + name: 🐍${{ matrix.python }} @ ${{ matrix.os }} strategy: matrix: os: [macOS] diff --git a/.github/workflows/test-dashboard.yml b/.github/workflows/test-dashboard.yml index 513256f9dc..7965993ce9 100644 --- a/.github/workflows/test-dashboard.yml +++ b/.github/workflows/test-dashboard.yml @@ -1,12 +1,12 @@ --- -name: Proxy.py Dashboard +name: dashboard on: [push, pull_request] # yamllint disable-line rule:truthy jobs: build: runs-on: ${{ matrix.os }}-latest - name: Dashboard - Node ${{ matrix.node }} on ${{ matrix.os }} + name: Node ${{ matrix.node }} @ ${{ matrix.os }} strategy: matrix: os: [macOS, ubuntu, windows] diff --git a/.github/workflows/test-docker.yml b/.github/workflows/test-docker.yml index d6dfef6ee2..2d81652db7 100644 --- a/.github/workflows/test-docker.yml +++ b/.github/workflows/test-docker.yml @@ -1,12 +1,12 @@ --- -name: Proxy.py Docker +name: docker on: [push, pull_request] # yamllint disable-line rule:truthy jobs: build: runs-on: ${{ matrix.os }}-latest - name: Docker - Python ${{ matrix.python }} on ${{ matrix.os }} + name: 🐍${{ matrix.python }} @ ${{ matrix.os }} strategy: matrix: os: [ubuntu] From 22136a012599455a2fa487612bae3e591a839ad7 Mon Sep 17 00:00:00 2001 From: Abhinav Singh Date: Fri, 5 Nov 2021 08:50:26 +0530 Subject: [PATCH 07/11] Update homebrew formulae. Doesnt seems to work now --- helper/homebrew/develop/proxy.rb | 5 ----- helper/homebrew/stable/proxy.rb | 8 ++------ 2 files changed, 2 insertions(+), 11 deletions(-) diff --git a/helper/homebrew/develop/proxy.rb b/helper/homebrew/develop/proxy.rb index d695e53ed8..37cff9a783 100644 --- a/helper/homebrew/develop/proxy.rb +++ b/helper/homebrew/develop/proxy.rb @@ -9,11 +9,6 @@ class Proxy < Formula depends_on "python" - resource "typing-extensions" do - url "https://files.pythonhosted.org/packages/6a/28/d32852f2af6b5ead85d396249d5bdf450833f3a69896d76eb480d9c5e406/typing_extensions-3.7.4.2.tar.gz" - sha256 "79ee589a3caca649a9bfd2a8de4709837400dfa00b6cc81962a1e6a1815969ae" - end - def install virtualenv_install_with_resources end diff --git a/helper/homebrew/stable/proxy.rb b/helper/homebrew/stable/proxy.rb index 83b38cbb1a..7d0e3a9fc5 100644 --- a/helper/homebrew/stable/proxy.rb +++ b/helper/homebrew/stable/proxy.rb @@ -5,15 +5,11 @@ class Proxy < Formula Network monitoring, controls & Application development, testing, debugging." homepage "https://github.com/abhinavsingh/proxy.py" url "https://github.com/abhinavsingh/proxy.py/archive/master.zip" - version "2.2.0" + sha256 "715687cebd451285d266f29d6509a64becc93da21f61ba9b4414e7dc4ecaaeed" + version "2.3.1" depends_on "python" - resource "typing-extensions" do - url "https://files.pythonhosted.org/packages/6a/28/d32852f2af6b5ead85d396249d5bdf450833f3a69896d76eb480d9c5e406/typing_extensions-3.7.4.2.tar.gz" - sha256 "79ee589a3caca649a9bfd2a8de4709837400dfa00b6cc81962a1e6a1815969ae" - end - def install virtualenv_install_with_resources end From f7e5eb5a03afdb4035e0ba67d622e7e9067c2d88 Mon Sep 17 00:00:00 2001 From: Abhinav Singh Date: Fri, 5 Nov 2021 09:51:30 +0530 Subject: [PATCH 08/11] test_enable_dashboard and test_enable_events --- proxy/proxy.py | 40 ++++++++++++------------ tests/test_main.py | 77 +++++++++++++++++++++++++++++++++++++--------- 2 files changed, 82 insertions(+), 35 deletions(-) diff --git a/proxy/proxy.py b/proxy/proxy.py index 5426c43523..3590db85af 100644 --- a/proxy/proxy.py +++ b/proxy/proxy.py @@ -23,7 +23,7 @@ import inspect from types import TracebackType -from typing import Dict, List, Optional, Any, Tuple, Type, Union, cast +from typing import Dict, List, Optional, Any, Type, Union, cast from proxy.core.acceptor.work import Work @@ -230,16 +230,14 @@ def initialize( Proxy.set_open_file_limit(args.open_file_limit) # Load plugins - default_plugins = Proxy.get_default_plugins(args) + default_plugins = [bytes_(p) for p in Proxy.get_default_plugins(args)] + extra_plugins = [] if args.plugins.strip() == '' else [ + p if isinstance(p, type) else bytes_(p) + for p in opts.get('plugins', args.plugins.split(text_(COMMA))) + ] # Load default plugins along with user provided --plugins - plugins = Proxy.load_plugins( - [bytes_(p) for p in collections.OrderedDict(default_plugins).keys()] + - [ - p if isinstance(p, type) else bytes_(p) - for p in opts.get('plugins', args.plugins.split(text_(COMMA))) - ], - ) + plugins = Proxy.load_plugins(default_plugins + extra_plugins) # proxy.py currently cannot serve over HTTPS and also perform TLS interception # at the same time. Check if user is trying to enable both feature @@ -449,31 +447,31 @@ def import_plugin(plugin: Union[bytes, type]) -> Any: @staticmethod def get_default_plugins( args: argparse.Namespace, - ) -> List[Tuple[str, bool]]: + ) -> List[str]: # Prepare list of plugins to load based upon # --enable-*, --disable-* and --basic-auth flags. - default_plugins: List[Tuple[str, bool]] = [] + default_plugins: List[str] = [] if args.basic_auth is not None: - default_plugins.append((PLUGIN_PROXY_AUTH, True)) + default_plugins.append(PLUGIN_PROXY_AUTH) if args.enable_dashboard: - default_plugins.append((PLUGIN_WEB_SERVER, True)) + default_plugins.append(PLUGIN_WEB_SERVER) args.enable_static_server = True - default_plugins.append((PLUGIN_DASHBOARD, True)) - default_plugins.append((PLUGIN_INSPECT_TRAFFIC, True)) + default_plugins.append(PLUGIN_DASHBOARD) + default_plugins.append(PLUGIN_INSPECT_TRAFFIC) args.enable_events = True args.enable_devtools = True if args.enable_devtools: - default_plugins.append((PLUGIN_DEVTOOLS_PROTOCOL, True)) - default_plugins.append((PLUGIN_WEB_SERVER, True)) + default_plugins.append(PLUGIN_DEVTOOLS_PROTOCOL) + default_plugins.append(PLUGIN_WEB_SERVER) if not args.disable_http_proxy: - default_plugins.append((PLUGIN_HTTP_PROXY, True)) + default_plugins.append(PLUGIN_HTTP_PROXY) if args.enable_web_server or \ args.pac_file is not None or \ args.enable_static_server: - default_plugins.append((PLUGIN_WEB_SERVER, True)) + default_plugins.append(PLUGIN_WEB_SERVER) if args.pac_file is not None: - default_plugins.append((PLUGIN_PAC_FILE, True)) - return default_plugins + default_plugins.append(PLUGIN_PAC_FILE) + return collections.OrderedDict.fromkeys(default_plugins).keys() @staticmethod def is_py2() -> bool: diff --git a/tests/test_main.py b/tests/test_main.py index ccbdd4a751..f869430e33 100644 --- a/tests/test_main.py +++ b/tests/test_main.py @@ -19,7 +19,7 @@ from proxy.common.utils import bytes_ from proxy.http.handler import HttpProtocolHandler -from proxy.common.constants import DEFAULT_LOG_LEVEL, DEFAULT_LOG_FILE, DEFAULT_LOG_FORMAT, DEFAULT_BASIC_AUTH +from proxy.common.constants import DEFAULT_ENABLE_DASHBOARD, DEFAULT_LOG_LEVEL, DEFAULT_LOG_FILE, DEFAULT_LOG_FORMAT, DEFAULT_BASIC_AUTH from proxy.common.constants import DEFAULT_TIMEOUT, DEFAULT_DEVTOOLS_WS_PATH, DEFAULT_DISABLE_HTTP_PROXY from proxy.common.constants import DEFAULT_ENABLE_STATIC_SERVER, DEFAULT_ENABLE_EVENTS, DEFAULT_ENABLE_DEVTOOLS from proxy.common.constants import DEFAULT_ENABLE_WEB_SERVER, DEFAULT_THREADLESS, DEFAULT_CERT_FILE, DEFAULT_KEY_FILE @@ -66,25 +66,22 @@ def mock_default_args(mock_args: mock.Mock) -> None: mock_args.enable_static_server = DEFAULT_ENABLE_STATIC_SERVER mock_args.enable_devtools = DEFAULT_ENABLE_DEVTOOLS mock_args.enable_events = DEFAULT_ENABLE_EVENTS + mock_args.enable_dashboard = DEFAULT_ENABLE_DASHBOARD @mock.patch('time.sleep') @mock.patch('proxy.proxy.Proxy.initialize') @mock.patch('proxy.proxy.EventManager') @mock.patch('proxy.proxy.AcceptorPool') - @mock.patch('logging.basicConfig') def test_entry_point( self, - mock_logging_config: mock.Mock, mock_acceptor_pool: mock.Mock, mock_event_manager: mock.Mock, mock_initialize: mock.Mock, mock_sleep: mock.Mock, ) -> None: mock_sleep.side_effect = KeyboardInterrupt() - mock_initialize.return_value.enable_events = False entry_point() - mock_event_manager.assert_not_called() mock_acceptor_pool.assert_called_with( flags=mock_initialize.return_value, @@ -99,20 +96,16 @@ def test_entry_point( @mock.patch('proxy.proxy.Proxy.initialize') @mock.patch('proxy.proxy.EventManager') @mock.patch('proxy.proxy.AcceptorPool') - @mock.patch('logging.basicConfig') def test_main_with_no_arguments( self, - mock_logging_config: mock.Mock, mock_acceptor_pool: mock.Mock, mock_event_manager: mock.Mock, mock_initialize: mock.Mock, mock_sleep: mock.Mock, ) -> None: mock_sleep.side_effect = KeyboardInterrupt() - - input_args: List[str] = [] mock_initialize.return_value.enable_events = False - main(input_args) + main([]) mock_event_manager.assert_not_called() mock_acceptor_pool.assert_called_with( flags=mock_initialize.return_value, @@ -123,6 +116,66 @@ def test_main_with_no_arguments( mock_acceptor_pool.return_value.shutdown.assert_called() mock_sleep.assert_called() + @mock.patch('time.sleep') + @mock.patch('proxy.proxy.Proxy.initialize') + @mock.patch('proxy.proxy.EventManager') + @mock.patch('proxy.proxy.AcceptorPool') + def test_enable_events( + self, + mock_acceptor_pool: mock.Mock, + mock_event_manager: mock.Mock, + mock_initialize: mock.Mock, + mock_sleep: mock.Mock, + ) -> None: + mock_sleep.side_effect = KeyboardInterrupt() + mock_initialize.return_value.enable_events = True + main([]) + mock_event_manager.assert_called_once() + mock_event_manager.return_value.start_event_dispatcher.assert_called_once() + mock_event_manager.return_value.stop_event_dispatcher.assert_called_once() + mock_acceptor_pool.assert_called_with( + flags=mock_initialize.return_value, + work_klass=HttpProtocolHandler, + event_queue=mock_event_manager.return_value.event_queue, + ) + mock_acceptor_pool.return_value.setup.assert_called() + mock_acceptor_pool.return_value.shutdown.assert_called() + mock_sleep.assert_called() + + @mock.patch('time.sleep') + @mock.patch('proxy.proxy.Proxy.load_plugins') + @mock.patch('proxy.common.flag.FlagParser.parse_args') + @mock.patch('proxy.proxy.EventManager') + @mock.patch('proxy.proxy.AcceptorPool') + def test_enable_dashboard( + self, + mock_acceptor_pool: mock.Mock, + mock_event_manager: mock.Mock, + mock_parse_args: mock.Mock, + mock_load_plugins: mock.Mock, + mock_sleep: mock.Mock, + ) -> None: + mock_sleep.side_effect = KeyboardInterrupt() + mock_args = mock_parse_args.return_value + self.mock_default_args(mock_args) + mock_args.enable_dashboard = True + main(['--enable-dashboard']) + mock_load_plugins.assert_called() + self.assertEqual(mock_load_plugins.call_args_list[0][0][0], [ + b'proxy.http.server.HttpWebServerPlugin', + b'proxy.dashboard.dashboard.ProxyDashboard', + b'proxy.dashboard.inspect_traffic.InspectTrafficPlugin', + b'proxy.http.inspector.DevtoolsProtocolPlugin', + b'proxy.http.proxy.HttpProxyPlugin', + ]) + mock_parse_args.assert_called_once() + mock_acceptor_pool.assert_called() + mock_acceptor_pool.return_value.setup.assert_called() + # dashboard will also enable eventing + mock_event_manager.assert_called_once() + mock_event_manager.return_value.start_event_dispatcher.assert_called_once() + mock_event_manager.return_value.stop_event_dispatcher.assert_called_once() + @mock.patch('time.sleep') @mock.patch('os.remove') @mock.patch('os.path.exists') @@ -145,7 +198,6 @@ def test_pid_file_is_written_and_removed( mock_args = mock_parse_args.return_value self.mock_default_args(mock_args) mock_args.pid_file = pid_file - mock_args.enable_dashboard = False main(['--pid-file', pid_file]) mock_parse_args.assert_called_once() mock_acceptor_pool.assert_called() @@ -231,9 +283,6 @@ def test_main_version( mock_print.assert_called_with(__version__) self.assertEqual(e.exception.code, 0) - def test_enable_dashboard(self) -> None: - pass - def test_enable_devtools(self) -> None: pass From c0e61b8e01539b0ea18367cafb6a001ab9748958 Mon Sep 17 00:00:00 2001 From: Abhinav Singh Date: Fri, 5 Nov 2021 09:52:06 +0530 Subject: [PATCH 09/11] test_enable_dashboard and test_enable_events --- tests/test_main.py | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/tests/test_main.py b/tests/test_main.py index f869430e33..38751791e9 100644 --- a/tests/test_main.py +++ b/tests/test_main.py @@ -161,13 +161,15 @@ def test_enable_dashboard( mock_args.enable_dashboard = True main(['--enable-dashboard']) mock_load_plugins.assert_called() - self.assertEqual(mock_load_plugins.call_args_list[0][0][0], [ - b'proxy.http.server.HttpWebServerPlugin', - b'proxy.dashboard.dashboard.ProxyDashboard', - b'proxy.dashboard.inspect_traffic.InspectTrafficPlugin', - b'proxy.http.inspector.DevtoolsProtocolPlugin', - b'proxy.http.proxy.HttpProxyPlugin', - ]) + self.assertEqual( + mock_load_plugins.call_args_list[0][0][0], [ + b'proxy.http.server.HttpWebServerPlugin', + b'proxy.dashboard.dashboard.ProxyDashboard', + b'proxy.dashboard.inspect_traffic.InspectTrafficPlugin', + b'proxy.http.inspector.DevtoolsProtocolPlugin', + b'proxy.http.proxy.HttpProxyPlugin', + ], + ) mock_parse_args.assert_called_once() mock_acceptor_pool.assert_called() mock_acceptor_pool.return_value.setup.assert_called() From c8f928f9ff50194f559d72df44797081701cab91 Mon Sep 17 00:00:00 2001 From: Abhinav Singh Date: Fri, 5 Nov 2021 10:13:56 +0530 Subject: [PATCH 10/11] Fix problem where empty plugin string was passed as plugin module --- proxy/proxy.py | 5 +++-- tests/test_main.py | 5 ++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/proxy/proxy.py b/proxy/proxy.py index 3590db85af..aaee404a67 100644 --- a/proxy/proxy.py +++ b/proxy/proxy.py @@ -231,9 +231,10 @@ def initialize( # Load plugins default_plugins = [bytes_(p) for p in Proxy.get_default_plugins(args)] - extra_plugins = [] if args.plugins.strip() == '' else [ + extra_plugins = [ p if isinstance(p, type) else bytes_(p) for p in opts.get('plugins', args.plugins.split(text_(COMMA))) + if not (isinstance(p, str) and len(p) == 0) ] # Load default plugins along with user provided --plugins @@ -471,7 +472,7 @@ def get_default_plugins( default_plugins.append(PLUGIN_WEB_SERVER) if args.pac_file is not None: default_plugins.append(PLUGIN_PAC_FILE) - return collections.OrderedDict.fromkeys(default_plugins).keys() + return list(collections.OrderedDict.fromkeys(default_plugins).keys()) @staticmethod def is_py2() -> bool: diff --git a/tests/test_main.py b/tests/test_main.py index 38751791e9..aa7b7df488 100644 --- a/tests/test_main.py +++ b/tests/test_main.py @@ -13,18 +13,17 @@ import os from unittest import mock -from typing import List from proxy.proxy import main, Proxy, entry_point from proxy.common.utils import bytes_ from proxy.http.handler import HttpProtocolHandler -from proxy.common.constants import DEFAULT_ENABLE_DASHBOARD, DEFAULT_LOG_LEVEL, DEFAULT_LOG_FILE, DEFAULT_LOG_FORMAT, DEFAULT_BASIC_AUTH +from proxy.common.constants import DEFAULT_ENABLE_DASHBOARD, DEFAULT_LOG_LEVEL, DEFAULT_LOG_FILE, DEFAULT_LOG_FORMAT from proxy.common.constants import DEFAULT_TIMEOUT, DEFAULT_DEVTOOLS_WS_PATH, DEFAULT_DISABLE_HTTP_PROXY from proxy.common.constants import DEFAULT_ENABLE_STATIC_SERVER, DEFAULT_ENABLE_EVENTS, DEFAULT_ENABLE_DEVTOOLS from proxy.common.constants import DEFAULT_ENABLE_WEB_SERVER, DEFAULT_THREADLESS, DEFAULT_CERT_FILE, DEFAULT_KEY_FILE from proxy.common.constants import DEFAULT_CA_CERT_FILE, DEFAULT_CA_KEY_FILE, DEFAULT_CA_SIGNING_KEY_FILE -from proxy.common.constants import DEFAULT_PAC_FILE, DEFAULT_PLUGINS, DEFAULT_PID_FILE, DEFAULT_PORT +from proxy.common.constants import DEFAULT_PAC_FILE, DEFAULT_PLUGINS, DEFAULT_PID_FILE, DEFAULT_PORT, DEFAULT_BASIC_AUTH from proxy.common.constants import DEFAULT_NUM_WORKERS, DEFAULT_OPEN_FILE_LIMIT, DEFAULT_IPV6_HOSTNAME from proxy.common.constants import DEFAULT_SERVER_RECVBUF_SIZE, DEFAULT_CLIENT_RECVBUF_SIZE, PY2_DEPRECATION_MESSAGE from proxy.common.version import __version__ From d9d181bbb151cb06b60129e995b85724202d16fe Mon Sep 17 00:00:00 2001 From: Abhinav Singh Date: Fri, 5 Nov 2021 13:23:01 +0530 Subject: [PATCH 11/11] test_enable_devtools and remove redundant guards for None and "" which was there due to a bug --- proxy/proxy.py | 13 ++++++------- tests/test_main.py | 48 +++++++++++++++++++++++++++++++++++++--------- 2 files changed, 45 insertions(+), 16 deletions(-) diff --git a/proxy/proxy.py b/proxy/proxy.py index aaee404a67..988dec4535 100644 --- a/proxy/proxy.py +++ b/proxy/proxy.py @@ -412,8 +412,7 @@ def load_plugins( } for plugin_ in plugins: klass, module_name = Proxy.import_plugin(plugin_) - if klass is None and module_name is None: - continue + assert klass and module_name mro = list(inspect.getmro(klass)) mro.reverse() iterator = iter(mro) @@ -432,8 +431,7 @@ def import_plugin(plugin: Union[bytes, type]) -> Any: klass = plugin else: plugin_ = text_(plugin.strip()) - if plugin_ == '': - return (None, None) + assert plugin_ != '' module_name, klass_name = plugin_.rsplit(text_(DOT), 1) klass = getattr( importlib.import_module( @@ -449,8 +447,9 @@ def import_plugin(plugin: Union[bytes, type]) -> Any: def get_default_plugins( args: argparse.Namespace, ) -> List[str]: - # Prepare list of plugins to load based upon - # --enable-*, --disable-* and --basic-auth flags. + """Prepare list of plugins to load based upon + --enable-*, --disable-* and --basic-auth flags. + """ default_plugins: List[str] = [] if args.basic_auth is not None: default_plugins.append(PLUGIN_PROXY_AUTH) @@ -514,7 +513,7 @@ def main( # at runtime. Example, updating flags, plugin # configuration etc. # - # TODO: Python shell within running proxy.py environment + # TODO: Python shell within running proxy.py environment? while True: time.sleep(1) except KeyboardInterrupt: diff --git a/tests/test_main.py b/tests/test_main.py index aa7b7df488..8f4ffba2d9 100644 --- a/tests/test_main.py +++ b/tests/test_main.py @@ -177,6 +177,39 @@ def test_enable_dashboard( mock_event_manager.return_value.start_event_dispatcher.assert_called_once() mock_event_manager.return_value.stop_event_dispatcher.assert_called_once() + @mock.patch('time.sleep') + @mock.patch('proxy.proxy.Proxy.load_plugins') + @mock.patch('proxy.common.flag.FlagParser.parse_args') + @mock.patch('proxy.proxy.EventManager') + @mock.patch('proxy.proxy.AcceptorPool') + def test_enable_devtools( + self, + mock_acceptor_pool: mock.Mock, + mock_event_manager: mock.Mock, + mock_parse_args: mock.Mock, + mock_load_plugins: mock.Mock, + mock_sleep: mock.Mock, + ) -> None: + mock_sleep.side_effect = KeyboardInterrupt() + mock_args = mock_parse_args.return_value + self.mock_default_args(mock_args) + mock_args.enable_devtools = True + main(['--enable-devtools']) + mock_load_plugins.assert_called() + print(mock_load_plugins.call_args_list[0][0][0]) + self.assertEqual( + mock_load_plugins.call_args_list[0][0][0], [ + b'proxy.http.inspector.DevtoolsProtocolPlugin', + b'proxy.http.server.HttpWebServerPlugin', + b'proxy.http.proxy.HttpProxyPlugin', + ], + ) + mock_parse_args.assert_called_once() + mock_acceptor_pool.assert_called() + mock_acceptor_pool.return_value.setup.assert_called() + # Currently --enable-devtools alone doesn't enable eventing core + mock_event_manager.assert_not_called() + @mock.patch('time.sleep') @mock.patch('os.remove') @mock.patch('os.path.exists') @@ -284,14 +317,11 @@ def test_main_version( mock_print.assert_called_with(__version__) self.assertEqual(e.exception.code, 0) - def test_enable_devtools(self) -> None: - pass - - def test_pac_file(self) -> None: - pass + # def test_pac_file(self) -> None: + # pass - def test_imports_plugin(self) -> None: - pass + # def test_imports_plugin(self) -> None: + # pass - def test_cannot_enable_https_proxy_and_tls_interception_mutually(self) -> None: - pass + # def test_cannot_enable_https_proxy_and_tls_interception_mutually(self) -> None: + # pass