[ VM / Service ] Omit private fields from service responses by default

Service responses and events previously could include "private"
properties, which have names starting with "_". This change removes
these properties from service objects unless explicitly requested via a
private parameter.

See go/smaller-dart-vm-service-responses for response size reduction
data.

TEST=Existing service suite

Change-Id: Ia65b14872e798eaa843f7d180c57721b82371d0b
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/221143
Commit-Queue: Ben Konyi <bkonyi@google.com>
Reviewed-by: Ryan Macnak <rmacnak@google.com>

pkg/dds/lib/src/client.dart

@@ -97,7 +97,13 @@ class DartDevelopmentServiceClient {
   void _registerJsonRpcMethods() {
     _clientPeer.registerMethod('streamListen', (parameters) async {
       final streamId = parameters['streamId'].asString;
-      await dds.streamManager.streamListen(this, streamId);
+      final includePrivates =
+          parameters['_includePrivateMembers'].asBoolOr(false);
+      await dds.streamManager.streamListen(
+        this,
+        streamId,
+        includePrivates: includePrivates,
+      );
       return RPCResponses.success;
     });
 

pkg/dds/lib/src/stream_manager.dart

@@ -146,16 +146,19 @@ class StreamManager {
   /// `streamListen` request for `stream` to the VM service.
   Future<void> streamListen(
     DartDevelopmentServiceClient? client,
-    String stream,
-  ) async {
+    String stream, {
+    bool? includePrivates,
+  }) async {
     await _mutex.runGuarded(
       () async {
         assert(stream.isNotEmpty);
+        bool streamNewlySubscribed = false;
         if (!streamListeners.containsKey(stream)) {
           // Initialize the list of clients for the new stream before we do
           // anything else to ensure multiple clients registering for the same
           // stream in quick succession doesn't result in multiple streamListen
           // requests being sent to the VM service.
+          streamNewlySubscribed = true;
           streamListeners[stream] = <DartDevelopmentServiceClient>[];
           if ((stream == kDebugStream && client == null) ||
               stream != kDebugStream) {
@@ -164,12 +167,17 @@ class StreamManager {
             final result =
                 await dds.vmServiceClient.sendRequest('streamListen', {
               'streamId': stream,
+              if (includePrivates != null)
+                '_includePrivateMembers': includePrivates,
             });
             assert(result['type'] == 'Success');
           }
         }
         if (streamListeners[stream]!.contains(client)) {
           throw kStreamAlreadySubscribedException;
+        } else if (!streamNewlySubscribed && includePrivates != null) {
+          await dds.vmServiceClient.sendRequest('_setStreamIncludePrivateMembers',
+              {'streamId': stream, 'includePrivateMembers': includePrivates});
         }
         if (client != null) {
           streamListeners[stream]!.add(client);

runtime/lib/vmservice.cc

@@ -128,10 +128,13 @@ DEFINE_NATIVE_ENTRY(VMService_OnServerAddressChange, 0, 1) {
   return Object::null();
 }
 
-DEFINE_NATIVE_ENTRY(VMService_ListenStream, 0, 1) {
+DEFINE_NATIVE_ENTRY(VMService_ListenStream, 0, 2) {
 #ifndef PRODUCT
   GET_NON_NULL_NATIVE_ARGUMENT(String, stream_id, arguments->NativeArgAt(0));
-  bool result = Service::ListenStream(stream_id.ToCString());
+  GET_NON_NULL_NATIVE_ARGUMENT(Bool, include_privates,
+                               arguments->NativeArgAt(1));
+  bool result =
+      Service::ListenStream(stream_id.ToCString(), include_privates.value());
   return Bool::Get(result).ptr();
 #else
   return Object::null();

runtime/observatory/lib/service_common.dart

@@ -149,7 +149,11 @@ abstract class CommonWebSocketVM extends VM {
       return new Future.error(exception);
     }
     String serial = (_requestSerial++).toString();
-    var request = new _WebSocketRequest(method, params);
+    var request = new _WebSocketRequest(method, <String, dynamic>{
+      ...params,
+      // Include internal response data.
+      '_includePrivateMembers': true,
+    });
     if ((_webSocket != null) && _webSocket!.isOpen) {
       // Already connected, send request immediately.
       _sendRequest(serial, request);

runtime/observatory_2/lib/service_common.dart

@@ -149,7 +149,11 @@ abstract class CommonWebSocketVM extends VM {
       return new Future.error(exception);
     }
     String serial = (_requestSerial++).toString();
-    var request = new _WebSocketRequest(method, params);
+    var request = new _WebSocketRequest(method, <String, dynamic>{
+      ...params,
+      // Include internal response data.
+      '_includePrivateMembers': true,
+    });
     if ((_webSocket != null) && _webSocket.isOpen) {
       // Already connected, send request immediately.
       _sendRequest(serial, request);

runtime/vm/bootstrap_natives.h

@@ -362,7 +362,7 @@ namespace dart {
   V(VMService_OnStart, 0)                                                      \
   V(VMService_OnExit, 0)                                                       \
   V(VMService_OnServerAddressChange, 1)                                        \
-  V(VMService_ListenStream, 1)                                                 \
+  V(VMService_ListenStream, 2)                                                 \
   V(VMService_CancelStream, 1)                                                 \
   V(VMService_RequestAssets, 0)                                                \
   V(VMService_DecodeAssets, 1)                                                 \

runtime/vm/json_stream.cc

@@ -37,7 +37,9 @@ JSONStream::JSONStream(intptr_t buf_size)
       param_values_(NULL),
       num_params_(0),
       offset_(0),
-      count_(-1) {
+      count_(-1),
+      include_private_members_(true),
+      ignore_object_depth_(0) {
   ObjectIdRing* ring = NULL;
   Isolate* isolate = Isolate::Current();
   if (isolate != NULL) {
@@ -90,6 +92,10 @@ void JSONStream::Setup(Zone* zone,
                  "request %s\n",
                  Dart::UptimeMillis(), main_port, isolate_name, method_);
   }
+  const char* kIncludePrivateMembersKey = "_includePrivateMembers";
+  if (HasParam(kIncludePrivateMembersKey)) {
+    include_private_members_ = ParamIs(kIncludePrivateMembersKey, "true");
+  }
   buffer()->Printf("{\"jsonrpc\":\"2.0\", \"result\":");
 }
 
@@ -368,49 +374,61 @@ void JSONStream::PrintServiceId(const Object& o) {
   PrintProperty("id", id_zone_->GetServiceId(o));
 }
 
+#define PRIVATE_NAME_CHECK()                                                   \
+  if (!IsAllowableKey(name) || ignore_object_depth_ > 0) return
+
 void JSONStream::PrintProperty(const char* name, const ServiceEvent* event) {
+  PRIVATE_NAME_CHECK();
   PrintPropertyName(name);
   PrintValue(event);
 }
 
 void JSONStream::PrintProperty(const char* name, Breakpoint* bpt) {
+  PRIVATE_NAME_CHECK();
   PrintPropertyName(name);
   PrintValue(bpt);
 }
 
 void JSONStream::PrintProperty(const char* name, TokenPosition tp) {
+  PRIVATE_NAME_CHECK();
   PrintPropertyName(name);
   PrintValue(tp);
 }
 
 void JSONStream::PrintProperty(const char* name, Metric* metric) {
+  PRIVATE_NAME_CHECK();
   PrintPropertyName(name);
   PrintValue(metric);
 }
 
 void JSONStream::PrintProperty(const char* name, MessageQueue* queue) {
+  PRIVATE_NAME_CHECK();
   PrintPropertyName(name);
   PrintValue(queue);
 }
 
 void JSONStream::PrintProperty(const char* name, Isolate* isolate) {
+  PRIVATE_NAME_CHECK();
   PrintPropertyName(name);
   PrintValue(isolate);
 }
 
 void JSONStream::PrintProperty(const char* name,
                                const TimelineEvent* timeline_event) {
+  PRIVATE_NAME_CHECK();
   PrintPropertyName(name);
   PrintValue(timeline_event);
 }
 
 void JSONStream::PrintProperty(const char* name,
                                const TimelineEventBlock* timeline_event_block) {
+  PRIVATE_NAME_CHECK();
   PrintPropertyName(name);
   PrintValue(timeline_event_block);
 }
 
 void JSONStream::PrintfProperty(const char* name, const char* format, ...) {
+  PRIVATE_NAME_CHECK();
   va_list args;
   va_start(args, format);
   writer_.VPrintfProperty(name, format, args);
@@ -462,11 +480,13 @@ void JSONStream::SetParams(const char** param_keys,
 }
 
 void JSONStream::PrintProperty(const char* name, const Object& o, bool ref) {
+  PRIVATE_NAME_CHECK();
   PrintPropertyName(name);
   PrintValue(o, ref);
 }
 
 void JSONStream::PrintPropertyVM(const char* name, bool ref) {
+  PRIVATE_NAME_CHECK();
   PrintPropertyName(name);
   PrintValueVM(ref);
 }

runtime/vm/json_stream.h

@@ -9,6 +9,7 @@
 #include "platform/allocation.h"
 #include "platform/text_buffer.h"
 #include "vm/json_writer.h"
+#include "vm/os.h"
 #include "vm/service.h"
 #include "vm/token_position.h"
 
@@ -102,6 +103,18 @@ class JSONStream : ValueObject {
 
   void set_reply_port(Dart_Port port);
 
+  bool include_private_members() const { return include_private_members_; }
+  void set_include_private_members(bool include_private_members) {
+    include_private_members_ = include_private_members;
+  }
+
+  bool IsAllowableKey(const char* key) {
+    if (include_private_members_) {
+      return true;
+    }
+    return *key != '_';
+  }
+
   void SetParams(const char** param_keys,
                  const char** param_values,
                  intptr_t num_params);
@@ -167,15 +180,41 @@ class JSONStream : ValueObject {
   void PostNullReply(Dart_Port port);
 
   void OpenObject(const char* property_name = NULL) {
+    if (ignore_object_depth_ > 0 ||
+        (property_name != nullptr && !IsAllowableKey(property_name))) {
+      ignore_object_depth_++;
+      return;
+    }
     writer_.OpenObject(property_name);
   }
-  void CloseObject() { writer_.CloseObject(); }
-  void UncloseObject() { writer_.UncloseObject(); }
+  void CloseObject() {
+    if (ignore_object_depth_ > 0) {
+      ignore_object_depth_--;
+      return;
+    }
+    writer_.CloseObject();
+  }
+  void UncloseObject() {
+    // This should be updated to handle unclosing a private object if we need
+    // to handle that case, which we don't currently.
+    writer_.UncloseObject();
+  }
 
   void OpenArray(const char* property_name = NULL) {
+    if (ignore_object_depth_ > 0 ||
+        (property_name != nullptr && !IsAllowableKey(property_name))) {
+      ignore_object_depth_++;
+      return;
+    }
     writer_.OpenArray(property_name);
   }
-  void CloseArray() { writer_.CloseArray(); }
+  void CloseArray() {
+    if (ignore_object_depth_ > 0) {
+      ignore_object_depth_--;
+      return;
+    }
+    writer_.CloseArray();
+  }
 
   void PrintValueNull() { writer_.PrintValueNull(); }
   void PrintValueBool(bool b) { writer_.PrintValueBool(b); }
@@ -211,46 +250,67 @@ class JSONStream : ValueObject {
 
   void PrintServiceId(const Object& o);
 
+#define PRIVATE_NAME_CHECK()                                                   \
+  if (!IsAllowableKey(name) || ignore_object_depth_ > 0) {                     \
+    return;                                                                    \
+  }
+
   void PrintPropertyBool(const char* name, bool b) {
+    PRIVATE_NAME_CHECK();
     writer_.PrintPropertyBool(name, b);
   }
   void PrintProperty(const char* name, intptr_t i) {
+    PRIVATE_NAME_CHECK();
     writer_.PrintProperty(name, i);
   }
   void PrintProperty64(const char* name, int64_t i) {
+    PRIVATE_NAME_CHECK();
     writer_.PrintProperty64(name, i);
   }
   void PrintPropertyTimeMillis(const char* name, int64_t millis) {
+    PRIVATE_NAME_CHECK();
     writer_.PrintProperty64(name, millis);
   }
   void PrintPropertyTimeMicros(const char* name, int64_t micros) {
+    PRIVATE_NAME_CHECK();
     writer_.PrintProperty64(name, micros);
   }
   void PrintProperty(const char* name, double d) {
+    PRIVATE_NAME_CHECK();
     writer_.PrintProperty(name, d);
   }
   void PrintPropertyBase64(const char* name,
                            const uint8_t* bytes,
                            intptr_t length) {
+    PRIVATE_NAME_CHECK();
     writer_.PrintPropertyBase64(name, bytes, length);
   }
   void PrintProperty(const char* name, const char* s) {
+    PRIVATE_NAME_CHECK();
     writer_.PrintProperty(name, s);
   }
   bool PrintPropertyStr(const char* name,
                         const String& s,
                         intptr_t offset,
                         intptr_t count) {
+    if (!IsAllowableKey(name)) {
+      return false;
+    }
     return writer_.PrintPropertyStr(name, s, offset, count);
   }
   void PrintPropertyNoEscape(const char* name, const char* s) {
+    PRIVATE_NAME_CHECK();
     writer_.PrintPropertyNoEscape(name, s);
   }
   void PrintfProperty(const char* name, const char* format, ...)
       PRINTF_ATTRIBUTE(3, 4);
   void VPrintfProperty(const char* name, const char* format, va_list args) {
+    PRIVATE_NAME_CHECK();
     writer_.VPrintfProperty(name, format, args);
   }
+
+#undef PRIVATE_NAME_CHECK
+
   void PrintProperty(const char* name, const Object& o, bool ref = true);
 
   void PrintProperty(const char* name, const ServiceEvent* event);
@@ -285,7 +345,8 @@ class JSONStream : ValueObject {
   intptr_t offset_;
   intptr_t count_;
   int64_t setup_time_micros_;
-
+  bool include_private_members_;
+  intptr_t ignore_object_depth_;
   friend class JSONObject;
   friend class JSONArray;
   friend class TimelineEvent;

runtime/vm/service.cc

@@ -367,7 +367,7 @@ static StreamInfo* const streams_[] = {
     &Service::timeline_stream, &Service::profiler_stream,
 };
 
-bool Service::ListenStream(const char* stream_id) {
+bool Service::ListenStream(const char* stream_id, bool include_private_members) {
   if (FLAG_trace_service) {
     OS::PrintErr("vm-service: starting stream '%s'\n", stream_id);
   }
@@ -375,6 +375,7 @@ bool Service::ListenStream(const char* stream_id) {
   for (intptr_t i = 0; i < num_streams; i++) {
     if (strcmp(stream_id, streams_[i]->id()) == 0) {
       streams_[i]->set_enabled(true);
+      streams_[i]->set_include_private_members(include_private_members);
       return true;
     }
   }
@@ -1164,7 +1165,7 @@ static void ReportPauseOnConsole(ServiceEvent* event) {
 }
 
 void Service::HandleEvent(ServiceEvent* event, bool enter_safepoint) {
-  if (event->stream_info() != NULL && !event->stream_info()->enabled()) {
+  if (event->stream_info() != nullptr && !event->stream_info()->enabled()) {
     if (FLAG_warn_on_pause_with_no_debugger && event->IsPause()) {
       // If we are about to pause a running program which has no
       // debugger connected, tell the user about it.
@@ -1172,7 +1173,7 @@ void Service::HandleEvent(ServiceEvent* event, bool enter_safepoint) {
     }
     // Ignore events when no one is listening to the event stream.
     return;
-  } else if (event->stream_info() != NULL &&
+  } else if (event->stream_info() != nullptr &&
              FLAG_warn_on_pause_with_no_debugger && event->IsPause()) {
     ReportPauseOnConsole(event);
   }
@@ -1180,8 +1181,11 @@ void Service::HandleEvent(ServiceEvent* event, bool enter_safepoint) {
     return;
   }
   JSONStream js;
+  if (event->stream_info() != nullptr) {
+    js.set_include_private_members(event->stream_info()->include_private_members());
+  }
   const char* stream_id = event->stream_id();
-  ASSERT(stream_id != NULL);
+  ASSERT(stream_id != nullptr);
   {
     JSONObject jsobj(&js);
     jsobj.AddProperty("jsonrpc", "2.0");
@@ -1495,6 +1499,30 @@ static void PrintSentinel(JSONStream* js, SentinelType sentinel_type) {
   }
 }
 
+static const MethodParameter* const set_stream_include_private_members_params[] = {
+    NO_ISOLATE_PARAMETER,
+    new BoolParameter("includePrivateMembers", true),
+    nullptr,
+};
+
+static void SetStreamIncludePrivateMembers(Thread* thread, JSONStream* js) {
+  const char* stream_id = js->LookupParam("streamId");
+  if (stream_id == nullptr) {
+    PrintMissingParamError(js, "streamId");
+    return;
+  }
+  bool include_private_members =
+      BoolParameter::Parse(js->LookupParam("includePrivateMembers"), false);
+  intptr_t num_streams = sizeof(streams_) / sizeof(streams_[0]);
+  for (intptr_t i = 0; i < num_streams; i++) {
+    if (strcmp(stream_id, streams_[i]->id()) == 0) {
+      streams_[i]->set_include_private_members(include_private_members);
+      break;
+    }
+  }
+  PrintSuccess(js);
+}
+
 static void ActOnIsolateGroup(JSONStream* js,
                               std::function<void(IsolateGroup*)> visitor) {
   const String& prefix =
@@ -5626,6 +5654,8 @@ static const ServiceMethodDescriptor service_methods_[] = {
     set_library_debuggable_params },
   { "setName", SetName,
     set_name_params },
+  { "_setStreamIncludePrivateMembers", SetStreamIncludePrivateMembers,
+    set_stream_include_private_members_params },
   { "setTraceClassAllocation", SetTraceClassAllocation,
     set_trace_class_allocation_params },
   { "setVMName", SetVMName,

runtime/vm/service.h

@@ -68,16 +68,21 @@ class RingServiceIdZone : public ServiceIdZone {
 
 class StreamInfo {
  public:
-  explicit StreamInfo(const char* id) : id_(id), enabled_(false) {}
+  explicit StreamInfo(const char* id)
+      : id_(id), enabled_(false), include_private_members_(false) {}
 
   const char* id() const { return id_; }
 
   void set_enabled(bool value) { enabled_ = value; }
   bool enabled() const { return enabled_; }
 
+  void set_include_private_members(bool value) { include_private_members_ = value; }
+  bool include_private_members() const { return include_private_members_; }
+
  private:
   const char* id_;
   bool enabled_;
+  bool include_private_members_;
 };
 
 class Service : public AllStatic {
@@ -168,7 +173,7 @@ class Service : public AllStatic {
   static StreamInfo timeline_stream;
   static StreamInfo profiler_stream;
 
-  static bool ListenStream(const char* stream_id);
+  static bool ListenStream(const char* stream_id, bool include_privates);
   static void CancelStream(const char* stream_id);
 
   static ObjectPtr RequestAssets();

sdk/lib/vmservice/vmservice.dart

@@ -522,7 +522,9 @@ class VMService extends MessageRouter {
       return encodeRpcError(message, kStreamAlreadySubscribed);
     }
     if (!_isAnyClientSubscribed(streamId)) {
-      if (!serviceStreams.contains(streamId) && !_vmListenStream(streamId)) {
+      final includePrivates = message.params['_includePrivateMembers'] == true;
+      if (!serviceStreams.contains(streamId) &&
+          !_vmListenStream(streamId, includePrivates)) {
         return encodeRpcError(message, kInvalidParams,
             details: "streamListen: invalid 'streamId' parameter: ${streamId}");
       }
@@ -750,7 +752,7 @@ external void _onServerAddressChange(String? address);
 
 /// Subscribe to a service stream.
 @pragma("vm:external-name", "VMService_ListenStream")
-external bool _vmListenStream(String streamId);
+external bool _vmListenStream(String streamId, bool include_privates);
 
 /// Cancel a subscription to a service stream.
 @pragma("vm:external-name", "VMService_CancelStream")