Make the debugger more resilient against invalid DAP messages. Fixes https://github.com/microsoft/ptvsd/issues/2050 (#10)

This commit is contained in:
Fabio Zadrozny 2020-01-29 08:12:31 -03:00 committed by GitHub
parent a1e4eb3c5e
commit 4652f1042e
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
5 changed files with 1611 additions and 357 deletions

View file

@ -40,6 +40,11 @@ class _OrderedSet(object):
self._contents_as_set.add(x)
self._contents.append(x)
def discard(self, x):
if x in self._contents_as_set:
self._contents_as_set.remove(x)
self._contents.remove(x)
def copy(self):
return _OrderedSet(self._contents)
@ -135,6 +140,8 @@ def create_classes_to_generate_structure(json_schema_data):
if isinstance(description, (list, tuple)):
description = '\n'.join(description)
if name == 'ModulesRequest': # Hack to accept modules request without arguments (ptvsd: 2050).
required.discard('arguments')
class_to_generatees[name] = dict(
name=name,
properties=properties,

View file

@ -14,7 +14,7 @@
"properties": {
"seq": {
"type": "integer",
"description": "Sequence number."
"description": "Sequence number (also known as message ID). For protocol messages of type 'request' this ID can be used to cancel the request."
},
"type": {
"type": "string",
@ -84,7 +84,7 @@
},
"success": {
"type": "boolean",
"description": "Outcome of the request."
"description": "Outcome of the request.\nIf true, the request was successful and the 'body' attribute may contain the result of the request.\nIf the value is false, the attribute 'message' contains the error in short form and the 'body' may contain additional information (see 'ErrorResponse.body.error')."
},
"command": {
"type": "string",
@ -92,7 +92,11 @@
},
"message": {
"type": "string",
"description": "Contains error message if success == false."
"description": "Contains the raw error in short form if 'success' is false.\nThis raw error might be interpreted by the frontend and is not shown in the UI.\nSome predefined values exist.",
"_enum": [ "cancelled" ],
"enumDescriptions": [
"request was cancelled."
]
},
"body": {
"type": [ "array", "boolean", "integer", "null", "number" , "object", "string" ],
@ -122,6 +126,40 @@
}]
},
"CancelRequest": {
"allOf": [ { "$ref": "#/definitions/Request" }, {
"type": "object",
"description": "The 'cancel' request is used by the frontend to indicate that it is no longer interested in the result produced by a specific request issued earlier.\nThis request has a hint characteristic: a debug adapter can only be expected to make a 'best effort' in honouring this request but there are no guarantees.\nThe 'cancel' request may return an error if it could not cancel an operation but a frontend should refrain from presenting this error to end users.\nA frontend client should only call this request if the capability 'supportsCancelRequest' is true.\nThe request that got canceled still needs to send a response back.\nThis can either be a normal result ('success' attribute true) or an error response ('success' attribute false and the 'message' set to 'cancelled').\nReturning partial results from a cancelled request is possible but please note that a frontend client has no generic way for detecting that a response is partial or not.",
"properties": {
"command": {
"type": "string",
"enum": [ "cancel" ]
},
"arguments": {
"$ref": "#/definitions/CancelArguments"
}
},
"required": [ "command" ]
}]
},
"CancelArguments": {
"type": "object",
"description": "Arguments for 'cancel' request.",
"properties": {
"requestId": {
"type": "integer",
"description": "The ID (attribute 'seq') of the request to cancel."
}
},
"required": [ "id" ]
},
"CancelResponse": {
"allOf": [ { "$ref": "#/definitions/Response" }, {
"type": "object",
"description": "Response to 'cancel' request. This is just an acknowledgement, so no body field is required."
}]
},
"InitializedEvent": {
"allOf": [ { "$ref": "#/definitions/Event" }, {
"type": "object",
@ -846,6 +884,73 @@
}]
},
"BreakpointLocationsRequest": {
"allOf": [ { "$ref": "#/definitions/Request" }, {
"type": "object",
"description": "The 'breakpointLocations' request returns all possible locations for source breakpoints in a given range.",
"properties": {
"command": {
"type": "string",
"enum": [ "breakpointLocations" ]
},
"arguments": {
"$ref": "#/definitions/BreakpointLocationsArguments"
}
},
"required": [ "command" ]
}]
},
"BreakpointLocationsArguments": {
"type": "object",
"description": "Arguments for 'breakpointLocations' request.",
"properties": {
"source": {
"$ref": "#/definitions/Source",
"description": "The source location of the breakpoints; either 'source.path' or 'source.reference' must be specified."
},
"line": {
"type": "integer",
"description": "Start line of range to search possible breakpoint locations in. If only the line is specified, the request returns all possible locations in that line."
},
"column": {
"type": "integer",
"description": "Optional start column of range to search possible breakpoint locations in. If no start column is given, the first column in the start line is assumed."
},
"endLine": {
"type": "integer",
"description": "Optional end line of range to search possible breakpoint locations in. If no end line is given, then the end line is assumed to be the start line."
},
"endColumn": {
"type": "integer",
"description": "Optional end column of range to search possible breakpoint locations in. If no end column is given, then it is assumed to be in the last column of the end line."
}
},
"required": [ "source", "line" ]
},
"BreakpointLocationsResponse": {
"allOf": [ { "$ref": "#/definitions/Response" }, {
"type": "object",
"description": "Response to 'breakpointLocations' request.\nContains possible locations for source breakpoints.",
"properties": {
"body": {
"type": "object",
"properties": {
"breakpoints": {
"type": "array",
"items": {
"$ref": "#/definitions/BreakpointLocation"
},
"description": "Sorted set of possible breakpoint locations."
}
},
"required": [ "breakpoints" ]
}
},
"required": [ "body" ]
}]
},
"SetBreakpointsRequest": {
"allOf": [ { "$ref": "#/definitions/Request" }, {
"type": "object",
@ -2601,6 +2706,14 @@
"supportsDisassembleRequest": {
"type": "boolean",
"description": "The debug adapter supports the 'disassemble' request."
},
"supportsCancelRequest": {
"type": "boolean",
"description": "The debug adapter supports the 'cancel' request."
},
"supportsBreakpointLocationsRequest": {
"type": "boolean",
"description": "The debug adapter supports the 'breakpointLocations' request."
}
}
},
@ -3016,6 +3129,30 @@
}
},
"BreakpointLocation": {
"type": "object",
"description": "Properties of a breakpoint location returned from the 'breakpointLocations' request.",
"properties": {
"line": {
"type": "integer",
"description": "Start line of breakpoint location."
},
"column": {
"type": "integer",
"description": "Optional start column of breakpoint location."
},
"endLine": {
"type": "integer",
"description": "Optional end line of breakpoint location if the location covers a range."
},
"endColumn": {
"type": "integer",
"description": "Optional end column of breakpoint location if the location covers a range."
}
},
"required": [ "line" ]
},
"SourceBreakpoint": {
"type": "object",
"description": "Properties of a breakpoint or logpoint passed to the setBreakpoints request.",

View file

@ -15,7 +15,7 @@ from _pydevd_bundle._debug_adapter.pydevd_schema import (
ProcessEvent, Scope, ScopesResponseBody, SetExpressionResponseBody,
SetVariableResponseBody, SourceBreakpoint, SourceResponseBody,
VariablesResponseBody, SetBreakpointsResponseBody, Response,
Capabilities, PydevdAuthorizeRequest)
Capabilities, PydevdAuthorizeRequest, Request)
from _pydevd_bundle.pydevd_api import PyDevdAPI
from _pydevd_bundle.pydevd_breakpoints import get_exception_class
from _pydevd_bundle.pydevd_comm_constants import (
@ -130,14 +130,25 @@ class PyDevJsonCommandProcessor(object):
DEBUG = False
try:
if isinstance(json_contents, bytes):
json_contents = json_contents.decode('utf-8')
request = self.from_json(json_contents, update_ids_from_dap=True)
except KeyError as e:
request = self.from_json(json_contents, update_ids_from_dap=False)
except Exception as e:
try:
loaded_json = json.loads(json_contents)
request = Request(loaded_json.get('command', '<unknown>'), loaded_json['seq'])
except:
# There's not much we can do in this case...
pydev_log.exception('Error loading json: %s', json_contents)
return
error_msg = str(e)
if error_msg.startswith("'") and error_msg.endswith("'"):
error_msg = error_msg[1:-1]
# This means a failure updating ids from the DAP (the client sent a key we didn't send).
# This means a failure processing the request (but we were able to load the seq,
# so, answer with a failure response).
def on_request(py_db, request):
error_response = {
'type': 'response',

View file

@ -80,8 +80,9 @@ class JsonFacade(object):
msg = self.writer.wait_for_message(accept_json_message, unquote_msg=False, expect_xml=False)
return from_json(msg)
def wait_for_response(self, request):
response_class = pydevd_base_schema.get_response_class(request)
def wait_for_response(self, request, response_class=None):
if response_class is None:
response_class = pydevd_base_schema.get_response_class(request)
def accept_message(response):
if isinstance(request, dict):
@ -3610,6 +3611,50 @@ def test_debug_options(case_setup, val):
writer.finished_ok = True
def test_send_invalid_messages(case_setup):
with case_setup.test_file('_debugger_case_local_variables.py') as writer:
json_facade = JsonFacade(writer)
writer.write_add_breakpoint(writer.get_line_index_with_content('Break 2 here'))
json_facade.write_make_initial_run()
stopped_event = json_facade.wait_for_json_message(StoppedEvent)
thread_id = stopped_event.body.threadId
json_facade.write_request(
pydevd_schema.StackTraceRequest(pydevd_schema.StackTraceArguments(threadId=thread_id)))
# : :type response: ModulesResponse
# : :type modules_response_body: ModulesResponseBody
# *** Check that we accept an invalid modules request (i.e.: without arguments).
response = json_facade.wait_for_response(json_facade.write_request(
{'type': 'request', 'command': 'modules'}))
modules_response_body = response.body
assert len(modules_response_body.modules) == 1
module = next(iter(modules_response_body.modules))
assert module['name'] == '__main__'
assert module['path'].endswith('_debugger_case_local_variables.py')
# *** Check that we don't fail on request without command.
request = json_facade.write_request({'type': 'request'})
response = json_facade.wait_for_response(request, Response)
assert not response.success
assert response.command == '<unknown>'
# *** Check that we don't crash if we can't decode message.
json_facade.writer.write_with_content_len('invalid json here')
# *** Check that we get a failure from a completions without arguments.
response = json_facade.wait_for_response(json_facade.write_request(
{'type': 'request', 'command': 'completions'}))
assert not response.success
json_facade.write_continue()
writer.finished_ok = True
def test_send_json_message(case_setup):
with case_setup.test_file('_debugger_case_custom_message.py') as writer: