From d5b76dc20060d4196fce9cd9c26598ca567a3b1e Mon Sep 17 00:00:00 2001 From: Pavel Minaev Date: Wed, 5 Dec 2018 18:22:04 -0800 Subject: [PATCH] Fix #874: Watch window freezes the system when Name contains non-ASCII symbols Fix #1064: test_module_events is failing on Windows Use Unicode literals throughout wrapper.py, and fix pathname handling. Add evaluation test for Unicode character in an expression. Fix path pattern such that its __eq__ is used deterministically. --- pytests/func/test_breakpoints.py | 10 +++---- pytests/func/test_django.py | 10 +++---- pytests/func/test_evaluate.py | 44 +++++++++++++++++++++++++++ pytests/func/test_exception.py | 6 ++-- pytests/func/test_flask.py | 10 +++---- pytests/func/test_path_mapping.py | 6 ++-- pytests/func/test_vs_specific.py | 8 ++--- pytests/helpers/pathutils.py | 14 ++++++++- pytests/helpers/pattern.py | 29 ++++++++++++------ src/ptvsd/wrapper.py | 50 +++++++++++++------------------ 10 files changed, 123 insertions(+), 64 deletions(-) diff --git a/pytests/func/test_breakpoints.py b/pytests/func/test_breakpoints.py index 8088ea21b..18036c97a 100644 --- a/pytests/func/test_breakpoints.py +++ b/pytests/func/test_breakpoints.py @@ -13,7 +13,7 @@ from pytests.helpers.pathutils import get_test_root from pytests.helpers.session import DebugSession from pytests.helpers.timeline import Event -from pytests.helpers.pattern import ANY +from pytests.helpers.pattern import ANY, Path BP_TEST_ROOT = get_test_root('bp') @@ -33,7 +33,7 @@ def test_path_with_ampersand(run_as, start_method): session.start_debugging() hit = session.wait_for_thread_stopped('breakpoint') frames = hit.stacktrace.body['stackFrames'] - assert frames[0]['source']['path'] == ANY.path(testfile) + assert frames[0]['source']['path'] == Path(testfile) session.send_request('continue').wait_for_response(freeze=False) session.wait_for_exit() @@ -54,7 +54,7 @@ def test_path_with_unicode(run_as, start_method): session.start_debugging() hit = session.wait_for_thread_stopped('breakpoint') frames = hit.stacktrace.body['stackFrames'] - assert frames[0]['source']['path'] == ANY.path(testfile) + assert frames[0]['source']['path'] == Path(testfile) assert u'ಏನಾದರೂ_ಮಾಡು' == frames[0]['name'] session.send_request('continue').wait_for_response(freeze=False) @@ -159,13 +159,13 @@ def script2(): hit = session.wait_for_thread_stopped() frames = hit.stacktrace.body['stackFrames'] assert bp_script2_line == frames[0]['line'] - assert frames[0]['source']['path'] == ANY.path(script2) + assert frames[0]['source']['path'] == Path(script2) session.send_request('continue').wait_for_response(freeze=False) hit = session.wait_for_thread_stopped() frames = hit.stacktrace.body['stackFrames'] assert bp_script1_line == frames[0]['line'] - assert frames[0]['source']['path'] == ANY.path(script1) + assert frames[0]['source']['path'] == Path(script1) session.send_request('continue').wait_for_response(freeze=False) session.wait_for_exit() diff --git a/pytests/func/test_django.py b/pytests/func/test_django.py index aca21c902..7b784b5c6 100644 --- a/pytests/func/test_django.py +++ b/pytests/func/test_django.py @@ -8,7 +8,7 @@ import pytest import sys -from pytests.helpers.pattern import ANY +from pytests.helpers.pattern import ANY, Path from pytests.helpers.session import DebugSession from pytests.helpers.timeline import Event from pytests.helpers.pathutils import get_test_root @@ -65,7 +65,7 @@ def test_django_breakpoint_no_multiproc(bp_target, start_method): 'name': bp_name, 'source': { 'sourceReference': ANY, - 'path': ANY.path(bp_file), + 'path': Path(bp_file), }, 'line': bp_line, 'column': 1, @@ -155,7 +155,7 @@ def test_django_exception_no_multiproc(ex_type, start_method): 'details': { 'message': 'Hello', 'typeName': ANY.such_that(lambda s: s.endswith('ArithmeticError')), - 'source': ANY.path(DJANGO1_MANAGE), + 'source': Path(DJANGO1_MANAGE), 'stackTrace': ANY.such_that(lambda s: True), } } @@ -170,7 +170,7 @@ def test_django_exception_no_multiproc(ex_type, start_method): 'name': 'bad_route_' + ex_type, 'source': { 'sourceReference': ANY, - 'path': ANY.path(DJANGO1_MANAGE), + 'path': Path(DJANGO1_MANAGE), }, 'line': ex_line, 'column': 1, @@ -240,7 +240,7 @@ def test_django_breakpoint_multiproc(start_method): 'name': 'home', 'source': { 'sourceReference': ANY.int, - 'path': ANY.path(DJANGO1_MANAGE), + 'path': Path(DJANGO1_MANAGE), }, 'line': bp_line, 'column': 1, diff --git a/pytests/func/test_evaluate.py b/pytests/func/test_evaluate.py index 814dc9c7c..ce70501cc 100644 --- a/pytests/func/test_evaluate.py +++ b/pytests/func/test_evaluate.py @@ -4,6 +4,8 @@ from __future__ import print_function, with_statement, absolute_import +import sys + from pytests.helpers.pattern import ANY from pytests.helpers.session import DebugSession from pytests.helpers.timeline import Event @@ -314,3 +316,45 @@ def my_func(): session.send_request('continue').wait_for_response() session.wait_for_exit() + + +def test_unicode(pyfile, run_as, start_method): + # On Python 3, variable names can contain Unicode characters. + # On Python 2, they must be ASCII, but using a Unicode character in an expression should not crash debugger. + + @pyfile + def code_to_debug(): + from dbgimporter import import_and_enable_debugger + import_and_enable_debugger() + import ptvsd + # Since Unicode variable name is a SyntaxError at parse time in Python 2, + # this needs to do a roundabout way of setting it to avoid parse issues. + globals()[u'\u16A0'] = 123 + ptvsd.break_into_debugger() + print('break') + + with DebugSession() as session: + session.initialize( + target=(run_as, code_to_debug), + start_method=start_method, + ignore_unobserved=[Event('continued')], + ) + session.start_debugging() + hit = session.wait_for_thread_stopped() + + resp_eval = session.send_request('evaluate', arguments={ + 'expression': '\u16A0', 'frameId': hit.frame_id, + }).wait_for_response() + + if sys.version_info >= (3,): + assert resp_eval.body == ANY.dict_with({ + 'type': 'int', + 'result': '123' + }) + else: + assert resp_eval.body == ANY.dict_with({ + 'type': 'SyntaxError' + }) + + session.send_request('continue').wait_for_response(freeze=False) + session.wait_for_exit() diff --git a/pytests/func/test_exception.py b/pytests/func/test_exception.py index 450e98e72..7ec5f8e7f 100644 --- a/pytests/func/test_exception.py +++ b/pytests/func/test_exception.py @@ -8,7 +8,7 @@ from pytests.helpers.session import DebugSession from pytests.helpers.timeline import Event -from pytests.helpers.pattern import ANY +from pytests.helpers.pattern import ANY, Path @pytest.mark.parametrize('raised', ['raisedOn', 'raisedOff']) @@ -47,7 +47,7 @@ def raise_with_except(): 'details': ANY.dict_with({ 'typeName': ANY.such_that(lambda s: s.endswith('ArithmeticError')), 'message': 'bad code', - 'source': ANY.path(code_to_debug), + 'source': Path(code_to_debug), }), }) @@ -102,7 +102,7 @@ def raise_without_except(): 'details': ANY.dict_with({ 'typeName': ANY.such_that(lambda s: s.endswith('ArithmeticError')), 'message': 'bad code', - 'source': ANY.path(code_to_debug), + 'source': Path(code_to_debug), }), }) diff --git a/pytests/func/test_flask.py b/pytests/func/test_flask.py index 8b5749390..c52d434b1 100644 --- a/pytests/func/test_flask.py +++ b/pytests/func/test_flask.py @@ -9,7 +9,7 @@ import pytest import sys -from pytests.helpers.pattern import ANY +from pytests.helpers.pattern import ANY, Path from pytests.helpers.session import DebugSession from pytests.helpers.timeline import Event from pytests.helpers.webhelper import get_web_content, wait_for_connection @@ -84,7 +84,7 @@ def test_flask_breakpoint_no_multiproc(bp_target, start_method): 'name': bp_name, 'source': { 'sourceReference': ANY.int, - 'path': ANY.path(bp_file), + 'path': Path(bp_file), }, 'line': bp_line, 'column': 1, @@ -165,7 +165,7 @@ def test_flask_exception_no_multiproc(ex_type, start_method): 'details': { 'message': 'Hello', 'typeName': ANY.such_that(lambda s: s.endswith('ArithmeticError')), - 'source': ANY.path(FLASK1_APP), + 'source': Path(FLASK1_APP), 'stackTrace': ANY.such_that(lambda s: True) } } @@ -180,7 +180,7 @@ def test_flask_exception_no_multiproc(ex_type, start_method): 'name': 'bad_route_' + ex_type, 'source': { 'sourceReference': ANY.int, - 'path': ANY.path(FLASK1_APP), + 'path': Path(FLASK1_APP), }, 'line': ex_line, 'column': 1, @@ -258,7 +258,7 @@ def test_flask_breakpoint_multiproc(start_method): 'name': 'home', 'source': { 'sourceReference': ANY.int, - 'path': ANY.path(FLASK1_APP), + 'path': Path(FLASK1_APP), }, 'line': bp_line, 'column': 1, diff --git a/pytests/func/test_path_mapping.py b/pytests/func/test_path_mapping.py index 32d3ab983..d52c2b7d5 100644 --- a/pytests/func/test_path_mapping.py +++ b/pytests/func/test_path_mapping.py @@ -6,7 +6,7 @@ import os from shutil import copyfile -from pytests.helpers.pattern import ANY +from pytests.helpers.pattern import Path from pytests.helpers.session import DebugSession from pytests.helpers.timeline import Event @@ -46,10 +46,10 @@ def code_to_debug(): session.start_debugging() hit = session.wait_for_thread_stopped('breakpoint') frames = hit.stacktrace.body['stackFrames'] - assert frames[0]['source']['path'] == ANY.path(path_local) + assert frames[0]['source']['path'] == Path(path_local) remote_code_path = session.read_json() - assert path_remote == ANY.path(remote_code_path) + assert path_remote == Path(remote_code_path) session.send_request('continue').wait_for_response(freeze=False) session.wait_for_exit() diff --git a/pytests/func/test_vs_specific.py b/pytests/func/test_vs_specific.py index 0119af3f6..35ea881a5 100644 --- a/pytests/func/test_vs_specific.py +++ b/pytests/func/test_vs_specific.py @@ -7,7 +7,7 @@ import pytest from pytests.helpers.session import DebugSession from pytests.helpers.timeline import Event -from pytests.helpers.pattern import ANY +from pytests.helpers.pattern import Path @pytest.mark.parametrize('module', [True, False]) @@ -87,9 +87,9 @@ def test_code(): modules = session.all_occurrences_of(Event('module')) modules = [(m.body['module']['name'], m.body['module']['path']) for m in modules] assert modules[:3] == [ - ('module2', ANY.path(module2)), - ('module1', ANY.path(module1)), - ('__main__', ANY.path(test_code)), + ('module2', Path(module2)), + ('module1', Path(module1)), + ('__main__', Path(test_code)), ] session.send_request('continue').wait_for_response(freeze=False) diff --git a/pytests/helpers/pathutils.py b/pytests/helpers/pathutils.py index 3ce599feb..5d2f1b42f 100644 --- a/pytests/helpers/pathutils.py +++ b/pytests/helpers/pathutils.py @@ -3,6 +3,10 @@ # for license information. import os.path +import sys + +import ptvsd.compat + def get_test_root(name): pytests_dir = os.path.dirname(os.path.dirname(__file__)) @@ -11,10 +15,18 @@ def get_test_root(name): return p return None + def compare_path(left, right, show=True): + # If there's a unicode/bytes mismatch, make both unicode. + if isinstance(left, ptvsd.compat.unicode): + if not isinstance(right, ptvsd.compat.unicode): + right = right.decode(sys.getfilesystemencoding()) + elif isinstance(right, ptvsd.compat.unicode): + left = right.decode(sys.getfilesystemencoding()) + n_left = os.path.normcase(left) n_right = os.path.normcase(right) if show: print('LEFT : ' + n_left) print('RIGHT: ' + n_right) - return str(n_left) == str(n_right) + return n_left == n_right diff --git a/pytests/helpers/pattern.py b/pytests/helpers/pattern.py index 3374d0ccc..b1ffa8b97 100644 --- a/pytests/helpers/pattern.py +++ b/pytests/helpers/pattern.py @@ -63,15 +63,6 @@ def __ne__(self, other): items = dict(items) return AnyDictWith(items) - @staticmethod - def path(p): - class AnyStrPath(str): - def __eq__(self, other): - return compare_path(self, other, show=False) - def __ne__(self, other): - return not (self == other) - return AnyStrPath(p) - class Maybe(BasePattern): """A pattern that matches if condition is True. @@ -118,6 +109,26 @@ def __eq__(self, value): return self.obj is value +class Path(object): + """A pattern that matches strings as path, using os.path.normcase before comparison, + and sys.getfilesystemencoding() to compare Unicode and non-Unicode strings. + """ + + def __init__(self, s): + self.s = s + + def __repr__(self): + return 'Path(%r)' % (self.s,) + + def __eq__(self, other): + if not (isinstance(other, bytes) or isinstance(other, unicode)): + return NotImplemented + return compare_path(self.s, other, show=False) + + def __ne__(self, other): + return not (self == other) + + SUCCESS = Success(True) FAILURE = Success(False) diff --git a/src/ptvsd/wrapper.py b/src/ptvsd/wrapper.py index 9b262b402..59c7d0583 100644 --- a/src/ptvsd/wrapper.py +++ b/src/ptvsd/wrapper.py @@ -2,7 +2,7 @@ # Licensed under the MIT License. See LICENSE in the project root # for license information. -from __future__ import print_function, absolute_import +from __future__ import print_function, absolute_import, unicode_literals import contextlib import errno @@ -46,6 +46,7 @@ from ptvsd import _util from ptvsd import multiproc from ptvsd import options +from ptvsd.compat import unicode import ptvsd.ipcjson as ipcjson # noqa import ptvsd.futures as futures # noqa import ptvsd.untangle as untangle # noqa @@ -105,14 +106,8 @@ def is_debugger_internal_thread(thread_name): return False -try: - unicode # noqa - - def needs_unicode(value): - return isinstance(value, unicode) # noqa -except Exception: - def needs_unicode(value): - return False +def path_to_unicode(s): + return s if isinstance(s, unicode) else s.decode(sys.getfilesystemencoding()) class SafeReprPresentationProvider(pydevd_extapi.StrPresentationProvider): @@ -217,7 +212,7 @@ def unquote_xml_path(s): """ if s is None: return None - return xml_unescape(unquote(str(s))) + return xml_unescape(unquote(s)) class IDMap(object): @@ -440,15 +435,14 @@ def makefile(self, *args, **kwargs): return os.fdopen(self.pipe_r) def make_packet(self, cmd_id, args): - # TODO: docstring + assert not isinstance(args, bytes) with self.lock: seq = self.seq self.seq += 1 - s = u'{}\t{}\t{}\n'.format(cmd_id, seq, args) + s = '{}\t{}\t{}\n'.format(cmd_id, seq, args) return seq, s def pydevd_notify(self, cmd_id, args): - # TODO: docstring if self.pipe_w is None: raise EOFError seq, s = self.make_packet(cmd_id, args) @@ -456,7 +450,6 @@ def pydevd_notify(self, cmd_id, args): os.write(self.pipe_w, s.encode('utf8')) def pydevd_request(self, loop, cmd_id, args): - # TODO: docstring if self.pipe_w is None: raise EOFError seq, s = self.make_packet(cmd_id, args) @@ -1467,11 +1460,14 @@ def _process_debug_options(self, opts): self.start_subprocess_notifier() # Print on all but NameError, don't suspend on any. - self.pydevd_request(pydevd_comm.CMD_PYDEVD_JSON_CONFIG, json.dumps(dict( + msg = json.dumps(dict( skip_suspend_on_breakpoint_exception=('BaseException',), skip_print_breakpoint_exception=('NameError',), multi_threads_single_notification=True, - ))) + )) + if isinstance(msg, bytes): + msg = msg.decode('utf-8') + self.pydevd_request(pydevd_comm.CMD_PYDEVD_JSON_CONFIG, msg) def _is_just_my_code_stepping_enabled(self): """Returns true if just-me-code stepping is enabled. @@ -1507,9 +1503,8 @@ def _initialize_path_maps(self, args): def _send_cmd_version_command(self): cmd = pydevd_comm.CMD_VERSION - default_os_type = 'WINDOWS' if platform.system() == 'Windows' else 'UNIX' # noqa - client_os_type = self.debug_options.get( - 'CLIENT_OS_TYPE', default_os_type) + default_os_type = 'WINDOWS' if platform.system() == 'Windows' else 'UNIX' + client_os_type = self.debug_options.get('CLIENT_OS_TYPE', default_os_type) os_id = client_os_type msg = '1.1\t{}\tID'.format(os_id) return self.pydevd_request(cmd, msg) @@ -1610,8 +1605,7 @@ def on_source(self, request, args): if source_reference == 0: self.send_error_response(request, 'Source unavailable') else: - server_filename = pydevd_file_utils.norm_file_to_server(filename) - + server_filename = path_to_unicode(pydevd_file_utils.norm_file_to_server(filename)) cmd = pydevd_comm.CMD_LOAD_SOURCE _, _, content = yield self.pydevd_request(cmd, server_filename) self.send_response(request, content=content) @@ -1788,7 +1782,7 @@ def on_variables(self, request, args): cmd = pydevd_comm.CMD_GET_FRAME else: cmd = pydevd_comm.CMD_GET_VARIABLE - cmdargs = (str(s) for s in pyd_var) + cmdargs = (unicode(s) for s in pyd_var) msg = '\t'.join(cmdargs) with (yield self.using_format(fmt)): _, _, resp_args = yield self.pydevd_request(cmd, msg) @@ -1870,7 +1864,7 @@ def _get_variable_evaluate_name(self, pyd_var_parent, var_name): def _get_index_or_key(self, text): # Dictionary resolver in pydevd provides key # in ' ()' format - result = re.match(r"(.*)\ \(([0-9]*)\)", text, + result = re.match("(.*)\ \(([0-9]*)\)", text, re.IGNORECASE | re.UNICODE) if result and len(result.groups()) == 2: try: @@ -1912,8 +1906,8 @@ def on_setVariable(self, request, args): # pydevd message format doesn't permit tabs in expressions expr = expr.replace('\t', ' ') - pyd_tid = str(pyd_var[0]) - pyd_fid = str(pyd_var[1]) + pyd_tid = unicode(pyd_var[0]) + pyd_fid = unicode(pyd_var[1]) # VSC gives us variablesReference to the parent of the variable # being set, and variable name; but pydevd wants the ID @@ -1968,7 +1962,7 @@ def on_evaluate(self, request, args): pyd_tid, pyd_fid = self.frame_map.to_pydevd(vsc_fid) cmd_args = (pyd_tid, pyd_fid, 'LOCAL', expr, '1') - msg = '\t'.join(str(s) for s in cmd_args) + msg = '\t'.join(unicode(s) for s in cmd_args) with (yield self.using_format(fmt)): _, _, resp_args = yield self.pydevd_request( pydevd_comm.CMD_EVALUATE_EXPRESSION, @@ -2053,7 +2047,7 @@ def on_setExpression(self, request, args): expr = expr.replace('\t', ' ') cmd_args = (pyd_tid, pyd_fid, 'LOCAL', expr, '1') - msg = '\t'.join(str(s) for s in cmd_args) + msg = '\t'.join(unicode(s) for s in cmd_args) with (yield self.using_format(fmt)): yield self.pydevd_request( pydevd_comm.CMD_EXEC_EXPRESSION, @@ -2183,8 +2177,6 @@ def on_setBreakpoints(self, request, args): cmd = pydevd_comm.CMD_SET_BREAK msgfmt = '{}\t{}\t{}\t{}\tNone\t{}\t{}\t{}\t{}\tALL' - if needs_unicode(path): - msgfmt = unicode(msgfmt) # noqa for src_bp in src_bps: line = src_bp['line'] vsc_bpid = self.bp_map.add(