[dds/dap] Handle errors parsing/casting launch/attach arguments and provide useful errors

Fixes https://github.com/dart-lang/sdk/issues/50709.

Change-Id: I35a7593ced462ab81d52069db831e5f95a9a4ba7
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/275623
Commit-Queue: Ben Konyi <bkonyi@google.com>
Reviewed-by: Ben Konyi <bkonyi@google.com>
This commit is contained in:
Danny Tuppeny 2022-12-14 17:19:05 +00:00 committed by Ben Konyi
parent 9b79ab636c
commit ec8918c20c
7 changed files with 403 additions and 47 deletions

View file

@ -1,3 +1,6 @@
# 2.5.1
- [DAP] Supplying incorrect types of arguments in `launch`/`attach` requests will now result in a clear error message in an error response instead of terminating the adapter.
# 2.5.0
- [DAP] `variables` requests now treat lists from `dart:typed_data` (such as `Uint8List`) like standard `List` instances and return their elements instead of class fields.
- [DAP] `variables` requests now return information about the number of items in lists to allow the client to page through them.

View file

@ -102,6 +102,10 @@ class DartAttachRequestArguments extends DartCommonLaunchAttachRequestArguments
/// Either this or [vmServiceUri] must be supplied.
final String? vmServiceInfoFile;
/// A reader for protocol arguments that throws detailed exceptions if
/// arguments aren't of the correct type.
static final arg = DebugAdapterArgumentReader('attach');
DartAttachRequestArguments({
this.vmServiceUri,
this.vmServiceInfoFile,
@ -131,8 +135,8 @@ class DartAttachRequestArguments extends DartCommonLaunchAttachRequestArguments
);
DartAttachRequestArguments.fromMap(Map<String, Object?> obj)
: vmServiceUri = obj['vmServiceUri'] as String?,
vmServiceInfoFile = obj['vmServiceInfoFile'] as String?,
: vmServiceUri = arg.read<String?>(obj, 'vmServiceUri'),
vmServiceInfoFile = arg.read<String?>(obj, 'vmServiceInfoFile'),
super.fromMap(obj);
@override
@ -149,6 +153,10 @@ class DartAttachRequestArguments extends DartCommonLaunchAttachRequestArguments
/// A common base for [DartLaunchRequestArguments] and
/// [DartAttachRequestArguments] for fields that are common to both.
class DartCommonLaunchAttachRequestArguments extends RequestArguments {
/// A reader for protocol arguments that throws detailed exceptions if
/// arguments aren't of the correct type.
static final arg = DebugAdapterArgumentReader('launch/attach');
/// Optional data from the previous, restarted session.
/// The data is sent as the 'restart' attribute of the 'terminated' event.
/// The client should leave the data intact.
@ -226,21 +234,22 @@ class DartCommonLaunchAttachRequestArguments extends RequestArguments {
});
DartCommonLaunchAttachRequestArguments.fromMap(Map<String, Object?> obj)
: restart = obj['restart'],
name = obj['name'] as String?,
cwd = obj['cwd'] as String?,
env = (obj['env'] as Map<String, Object?>?)?.cast<String, String>(),
: restart = arg.read<Object?>(obj, 'restart'),
name = arg.read<String?>(obj, 'name'),
cwd = arg.read<String?>(obj, 'cwd'),
env = arg.readOptionalMap<String, String>(obj, 'env'),
additionalProjectPaths =
(obj['additionalProjectPaths'] as List?)?.cast<String>(),
debugSdkLibraries = obj['debugSdkLibraries'] as bool?,
arg.readOptionalList<String>(obj, 'additionalProjectPaths'),
debugSdkLibraries = arg.read<bool?>(obj, 'debugSdkLibraries'),
debugExternalPackageLibraries =
obj['debugExternalPackageLibraries'] as bool?,
arg.read<bool?>(obj, 'debugExternalPackageLibraries'),
evaluateGettersInDebugViews =
obj['evaluateGettersInDebugViews'] as bool?,
arg.read<bool?>(obj, 'evaluateGettersInDebugViews'),
evaluateToStringInDebugViews =
obj['evaluateToStringInDebugViews'] as bool?,
sendLogsToClient = obj['sendLogsToClient'] as bool?,
sendCustomProgressEvents = obj['sendCustomProgressEvents'] as bool?;
arg.read<bool?>(obj, 'evaluateToStringInDebugViews'),
sendLogsToClient = arg.read<bool?>(obj, 'sendLogsToClient'),
sendCustomProgressEvents =
arg.read<bool?>(obj, 'sendCustomProgressEvents');
Map<String, Object?> toJson() => {
if (restart != null) 'restart': restart,
@ -2213,6 +2222,10 @@ abstract class DartDebugAdapter<TL extends LaunchRequestArguments,
/// class.
class DartLaunchRequestArguments extends DartCommonLaunchAttachRequestArguments
implements LaunchRequestArguments {
/// A reader for protocol arguments that throws detailed exceptions if
/// arguments aren't of the correct type.
static final arg = DebugAdapterArgumentReader('launch');
/// If noDebug is true the launch request should launch the program without
/// enabling debugging.
final bool? noDebug;
@ -2304,15 +2317,16 @@ class DartLaunchRequestArguments extends DartCommonLaunchAttachRequestArguments
);
DartLaunchRequestArguments.fromMap(Map<String, Object?> obj)
: noDebug = obj['noDebug'] as bool?,
program = obj['program'] as String,
args = (obj['args'] as List?)?.cast<String>(),
toolArgs = (obj['toolArgs'] as List?)?.cast<String>(),
vmAdditionalArgs = (obj['vmAdditionalArgs'] as List?)?.cast<String>(),
vmServicePort = obj['vmServicePort'] as int?,
console = obj['console'] as String?,
customTool = obj['customTool'] as String?,
customToolReplacesArgs = obj['customToolReplacesArgs'] as int?,
: noDebug = arg.read<bool?>(obj, 'noDebug'),
program = arg.read<String>(obj, 'program'),
args = arg.readOptionalList<String>(obj, 'args'),
toolArgs = arg.readOptionalList<String>(obj, 'toolArgs'),
vmAdditionalArgs =
arg.readOptionalList<String>(obj, 'vmAdditionalArgs'),
vmServicePort = arg.read<int?>(obj, 'vmServicePort'),
console = arg.read<String?>(obj, 'console'),
customTool = arg.read<String?>(obj, 'customTool'),
customToolReplacesArgs = arg.read<int?>(obj, 'customToolReplacesArgs'),
super.fromMap(obj);
@override

View file

@ -108,31 +108,31 @@ abstract class BaseDebugAdapter<TLaunchArgs extends LaunchRequestArguments,
_RequestHandler<TArg, TResp> handler,
TArg Function(Map<String, Object?>) fromJson,
) async {
final args = request.arguments != null
? fromJson(request.arguments as Map<String, Object?>)
// arguments are only valid to be null then TArg is nullable.
: null as TArg;
// Because handlers may need to send responses before they have finished
// executing (for example, initializeRequest needs to send its response
// before sending InitializedEvent()), we pass in a function `sendResponse`
// rather than using a return value.
var sendResponseCalled = false;
void sendResponse(TResp responseBody) {
assert(!sendResponseCalled,
'sendResponse was called multiple times by ${request.command}');
sendResponseCalled = true;
final response = Response(
success: true,
requestSeq: request.seq,
seq: _sequence++,
command: request.command,
body: responseBody,
);
_channel.sendResponse(response);
}
try {
final args = request.arguments != null
? fromJson(request.arguments as Map<String, Object?>)
// arguments are only valid to be null then TArg is nullable.
: null as TArg;
// Because handlers may need to send responses before they have finished
// executing (for example, initializeRequest needs to send its response
// before sending InitializedEvent()), we pass in a function `sendResponse`
// rather than using a return value.
var sendResponseCalled = false;
void sendResponse(TResp responseBody) {
assert(!sendResponseCalled,
'sendResponse was called multiple times by ${request.command}');
sendResponseCalled = true;
final response = Response(
success: true,
requestSeq: request.seq,
seq: _sequence++,
command: request.command,
body: responseBody,
);
_channel.sendResponse(response);
}
await handler(request, args, sendResponse);
assert(sendResponseCalled,
'sendResponse was not called in ${request.command}');

View file

@ -12,3 +12,42 @@ class DebugAdapterException implements Exception {
String toString() => 'DebugAdapterException: $message';
}
/// Exception thrown when failing to read arguments supplied by the user because
/// they are not the correct type.
///
/// This is usually because a user customised their launch configuration (for
/// example in `.vscode/launch.json` for VS Code) with values that are not
/// valid, such as putting a `String` in a field intended to be a `Map`:
///
/// ```
/// // Bad.
/// "env": "foo"
///
/// // Good.
/// "env": {
/// "FLUTTER_ROOT": "foo",
/// }
/// ```
class DebugAdapterInvalidArgumentException implements DebugAdapterException {
final String requestName;
final String argumentName;
final Type expectedType;
final Type actualType;
final Object? actualValue;
DebugAdapterInvalidArgumentException({
required this.requestName,
required this.argumentName,
required this.expectedType,
required this.actualType,
required this.actualValue,
});
@override
String get message =>
'"$argumentName" argument in $requestName configuration must be a '
'$expectedType but provided value was a $actualType ($actualValue)';
String toString() => 'DebugAdapterInvalidArgumentException: $message';
}

View file

@ -4,6 +4,8 @@
import 'dart:convert';
import 'exceptions.dart';
/// A base class for (spec-generated) classes that represent the `body` of a an
/// event.
abstract class EventBody {
@ -49,3 +51,81 @@ class RawRequestArguments extends RequestArguments {
abstract class RequestArguments {
static bool canParse(Object? obj) => obj is Map<String, Object?>?;
}
/// A helper for reading arguments for DAP requests from the client.
class DebugAdapterArgumentReader {
final String request;
DebugAdapterArgumentReader(this.request);
/// Reads a value of type [T] from [field] in [obj].
T read<T>(
Map<String, Object?> obj,
String field,
) {
final value = obj[field];
if (value is! T) {
throw DebugAdapterInvalidArgumentException(
requestName: request,
argumentName: field,
expectedType: T,
actualType: value.runtimeType,
actualValue: value,
);
}
return obj[field] as T;
}
/// Reads a List of values of type [T] from [field] in [obj].
List<T> readList<T>(
Map<String, Object?> obj,
String field,
) {
final value = obj[field];
if (value is! List || !value.every((element) => element is T)) {
throw DebugAdapterInvalidArgumentException(
requestName: request,
argumentName: field,
expectedType: List<T>,
actualType: value.runtimeType,
actualValue: value,
);
}
return (obj[field] as List<Object?>).cast<T>();
}
/// Reads an optional List of values of type [T] from [field] in [obj].
List<T>? readOptionalList<T>(
Map<String, Object?> obj,
String field,
) {
return obj.containsKey(field) ? readList<T>(obj, field) : null;
}
/// Reads an optional Map of types [K],[V] from [field] in [obj].
Map<K, V>? readOptionalMap<K, V>(
Map<String, Object?> obj,
String field,
) {
return obj.containsKey(field) ? readMap<K, V>(obj, field) : null;
}
/// Reads a Map of types [K],[V] from [field] in [obj].
Map<K, V> readMap<K, V>(
Map<String, Object?> obj,
String field,
) {
final value = obj[field];
if (value is! Map ||
!value.entries.every((entry) => entry.key is K && entry.value is V)) {
throw DebugAdapterInvalidArgumentException(
requestName: request,
argumentName: field,
expectedType: Map<K, V>,
actualType: value.runtimeType,
actualValue: value,
);
}
return (obj[field] as Map<Object?, Object?>).cast<K, V>();
}
}

View file

@ -1,5 +1,5 @@
name: dds
version: 2.5.0
version: 2.5.1
description: >-
A library used to spawn the Dart Developer Service, used to communicate with
a Dart VM Service instance.

View file

@ -0,0 +1,220 @@
// Copyright (c) 2022, the Dart project authors. Please see the AUTHORS file
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.
import 'package:test/test.dart';
import 'test_support.dart';
main() {
group('validates arguments', () {
group('for Dart CLI adapter', () {
late DapTestSession dap;
setUp(() async {
dap = await DapTestSession.setUp();
});
tearDown(() => dap.tearDown());
String errorMessage(
String field,
String request,
String expectedType,
Object? actual,
) =>
'"$field" argument in $request configuration must be a $expectedType '
'but provided value was a ${actual.runtimeType} ($actual)';
group('for launchRequest', () {
Future<void> expectError(
Map<String, Object?> args,
String expectedError,
) async {
final response = await dap.client.sendRequest(
args,
overrideCommand: 'launch',
allowFailure: true,
);
expect(response.success, isFalse);
expect(response.message, expectedError);
}
test('when a non-String is supplied for a String', () async {
await expectError(
{
'program': true,
},
errorMessage('program', 'launch', 'String', true),
);
});
test('when a non-Map is supplied for a Map', () async {
await expectError(
{
'program': '',
'env': 'test',
},
errorMessage('env', 'launch/attach', 'Map<String, String>', 'test'),
);
});
test('when an invalid type is supplied for a Map value', () async {
await expectError(
{
'program': '',
'env': {'FOO': true}
},
errorMessage('env', 'launch/attach', 'Map<String, String>',
<String, dynamic>{'FOO': true}),
);
});
});
group('for attachRequest', () {
Future<void> expectError(
Map<String, Object?> args,
String expectedError,
) async {
final response = await dap.client.sendRequest(
args,
overrideCommand: 'attach',
allowFailure: true,
);
expect(response.success, isFalse);
expect(response.message, expectedError);
}
test('when a non-String is supplied for a String?', () async {
await expectError(
{
'vmServiceUri': true,
},
errorMessage('vmServiceUri', 'attach', 'String?', true),
);
});
test('when a non-Map is supplied for a Map', () async {
await expectError(
{
'program': '',
'env': 'test',
},
errorMessage('env', 'launch/attach', 'Map<String, String>', 'test'),
);
});
test('when an invalid type is supplied for a Map value', () async {
await expectError(
{
'program': '',
'env': {'FOO': true}
},
errorMessage('env', 'launch/attach', 'Map<String, String>',
<String, dynamic>{'FOO': true}),
);
});
});
});
group('for Dart Test adapter', () {
late DapTestSession dap;
setUp(() async {
dap = await DapTestSession.setUp(additionalArgs: ['--test']);
});
tearDown(() => dap.tearDown());
group('for launchRequest', () {
Future<void> expectError(
Map<String, Object?> args,
String expectedError,
) async {
final response = await dap.client.sendRequest(
args,
overrideCommand: 'launch',
allowFailure: true,
);
expect(response.success, isFalse);
expect(response.message, expectedError);
}
test('when a non-String is supplied for a String', () async {
await expectError(
{
'program': true,
},
'"program" argument in launch configuration must be a String but provided value was a bool (true)',
);
});
test('when a non-Map is supplied for a Map', () async {
await expectError(
{
'program': '',
'env': 'test',
},
'"env" argument in launch/attach configuration must be a Map<String, String> but provided value was a String (test)',
);
});
test('when an invalid type is supplied for a Map value', () async {
await expectError(
{
'program': '',
'env': {'FOO': true}
},
'"env" argument in launch/attach configuration must be a Map<String, String> but provided value was a _Map<String, dynamic> ({FOO: true})',
);
});
});
group('for attachRequest', () {
Future<void> expectError(
Map<String, Object?> args,
String expectedError,
) async {
final response = await dap.client.sendRequest(
args,
overrideCommand: 'attach',
allowFailure: true,
);
expect(response.success, isFalse);
expect(response.message, expectedError);
}
test('when a non-String is supplied for a String?', () async {
await expectError(
{
'vmServiceUri': true,
},
'"vmServiceUri" argument in attach configuration must be a String? but provided value was a bool (true)',
);
});
test('when a non-Map is supplied for a Map', () async {
await expectError(
{
'program': '',
'env': 'test',
},
'"env" argument in launch/attach configuration must be a Map<String, String> but provided value was a String (test)',
);
});
test('when an invalid type is supplied for a Map value', () async {
await expectError(
{
'program': '',
'env': {'FOO': true}
},
'"env" argument in launch/attach configuration must be a Map<String, String> but provided value was a _Map<String, dynamic> ({FOO: true})',
);
});
});
});
// These tests can be slow due to starting up the external server process.
}, timeout: Timeout.none);
}