Do not leak extensions of external classes (#1576)

denrase · web-flow · commit c59de474e74a · 2023-09-04T09:52:32.000+02:00
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -9,6 +9,7 @@
 - Refrain from overwriting the span status for unfinished spans ([#1577](https://github.com/getsentry/sentry-dart/pull/1577))
   - Older self-hosted sentry instances will drop transactions containing unfinished spans.
     - This change was introduced in [relay/#1690](https://github.com/getsentry/relay/pull/1690) and released with [22.12.0](https://github.com/getsentry/relay/releases/tag/22.12.0)
+- Do not leak extensions of external classes ([#1576](https://github.com/getsentry/sentry-dart/pull/1576))
 
 ## Unreleased
   
diff --git a/dart/lib/src/client_reports/discard_reason.dart b/dart/lib/src/client_reports/discard_reason.dart
@@ -12,24 +12,3 @@ enum DiscardReason {
   cacheOverflow,
   rateLimitBackoff,
 }
-
-extension OutcomeExtension on DiscardReason {
-  String toStringValue() {
-    switch (this) {
-      case DiscardReason.beforeSend:
-        return 'before_send';
-      case DiscardReason.eventProcessor:
-        return 'event_processor';
-      case DiscardReason.sampleRate:
-        return 'sample_rate';
-      case DiscardReason.networkError:
-        return 'network_error';
-      case DiscardReason.queueOverflow:
-        return 'queue_overflow';
-      case DiscardReason.cacheOverflow:
-        return 'cache_overflow';
-      case DiscardReason.rateLimitBackoff:
-        return 'ratelimit_backoff';
-    }
-  }
-}
diff --git a/dart/lib/src/client_reports/discarded_event.dart b/dart/lib/src/client_reports/discarded_event.dart
@@ -13,9 +13,53 @@ class DiscardedEvent {
 
   Map<String, dynamic> toJson() {
     return {
-      'reason': reason.toStringValue(),
-      'category': category.toStringValue(),
+      'reason': reason._toStringValue(),
+      'category': category._toStringValue(),
       'quantity': quantity,
     };
   }
 }
+
+extension _OutcomeExtension on DiscardReason {
+  String _toStringValue() {
+    switch (this) {
+      case DiscardReason.beforeSend:
+        return 'before_send';
+      case DiscardReason.eventProcessor:
+        return 'event_processor';
+      case DiscardReason.sampleRate:
+        return 'sample_rate';
+      case DiscardReason.networkError:
+        return 'network_error';
+      case DiscardReason.queueOverflow:
+        return 'queue_overflow';
+      case DiscardReason.cacheOverflow:
+        return 'cache_overflow';
+      case DiscardReason.rateLimitBackoff:
+        return 'ratelimit_backoff';
+    }
+  }
+}
+
+extension _DataCategoryExtension on DataCategory {
+  String _toStringValue() {
+    switch (this) {
+      case DataCategory.all:
+        return '__all__';
+      case DataCategory.dataCategoryDefault:
+        return 'default';
+      case DataCategory.error:
+        return 'error';
+      case DataCategory.session:
+        return 'session';
+      case DataCategory.transaction:
+        return 'transaction';
+      case DataCategory.attachment:
+        return 'attachment';
+      case DataCategory.security:
+        return 'security';
+      case DataCategory.unknown:
+        return 'unknown';
+    }
+  }
+}
diff --git a/dart/lib/src/http_client/failed_request_client.dart b/dart/lib/src/http_client/failed_request_client.dart
@@ -135,7 +135,7 @@ class FailedRequestClient extends BaseClient {
 
     // Only check `failedRequestStatusCodes` & `failedRequestTargets` if no exception was thrown.
     if (exception == null) {
-      if (!failedRequestStatusCodes.containsStatusCode(statusCode)) {
+      if (!failedRequestStatusCodes._containsStatusCode(statusCode)) {
         return;
       }
       if (!containsTargetOrMatchesRegExp(
@@ -246,7 +246,7 @@ class FailedRequestClient extends BaseClient {
 }
 
 extension _ListX on List<SentryStatusCode> {
-  bool containsStatusCode(int? statusCode) {
+  bool _containsStatusCode(int? statusCode) {
     if (statusCode == null) {
       return false;
     }
diff --git a/dart/lib/src/protocol/sentry_request.dart b/dart/lib/src/protocol/sentry_request.dart
@@ -1,6 +1,6 @@
 import 'package:meta/meta.dart';
 
-import '../utils/iterable_extension.dart';
+import '../utils/iterable_utils.dart';
 import '../utils/http_sanitizer.dart';
 
 /// The Request interface contains information on a HTTP request related to the event.
@@ -85,9 +85,10 @@ class SentryRequest {
         _headers = headers != null ? Map.from(headers) : null,
         // Look for a 'Set-Cookie' header (case insensitive) if not given.
         cookies = cookies ??
-            headers?.entries
-                .firstWhereOrNull((e) => e.key.toLowerCase() == 'cookie')
-                ?.value,
+            IterableUtils.firstWhereOrNull(
+              headers?.entries,
+              (MapEntry<String, String> e) => e.key.toLowerCase() == 'cookie',
+            )?.value,
         _env = env != null ? Map.from(env) : null,
         _other = other != null ? Map.from(other) : null;
 
diff --git a/dart/lib/src/protocol/sentry_response.dart b/dart/lib/src/protocol/sentry_response.dart
@@ -1,6 +1,6 @@
 import 'package:meta/meta.dart';
 import 'contexts.dart';
-import '../utils/iterable_extension.dart';
+import '../utils/iterable_utils.dart';
 
 /// The response interface contains information on a HTTP request related to the event.
 @immutable
@@ -53,9 +53,11 @@ class SentryResponse {
         _headers = headers != null ? Map.from(headers) : null,
         // Look for a 'Set-Cookie' header (case insensitive) if not given.
         cookies = cookies ??
-            headers?.entries
-                .firstWhereOrNull((e) => e.key.toLowerCase() == 'set-cookie')
-                ?.value;
+            IterableUtils.firstWhereOrNull(
+              headers?.entries,
+              (MapEntry<String, String> e) =>
+                  e.key.toLowerCase() == 'set-cookie',
+            )?.value;
 
   /// Deserializes a [SentryResponse] from JSON [Map].
   factory SentryResponse.fromJson(Map<String, dynamic> json) {
diff --git a/dart/lib/src/transport/data_category.dart b/dart/lib/src/transport/data_category.dart
@@ -9,46 +9,3 @@ enum DataCategory {
   security,
   unknown
 }
-
-extension DataCategoryExtension on DataCategory {
-  static DataCategory fromStringValue(String stringValue) {
-    switch (stringValue) {
-      case '__all__':
-        return DataCategory.all;
-      case 'default':
-        return DataCategory.dataCategoryDefault;
-      case 'error':
-        return DataCategory.error;
-      case 'session':
-        return DataCategory.session;
-      case 'transaction':
-        return DataCategory.transaction;
-      case 'attachment':
-        return DataCategory.attachment;
-      case 'security':
-        return DataCategory.security;
-    }
-    return DataCategory.unknown;
-  }
-
-  String toStringValue() {
-    switch (this) {
-      case DataCategory.all:
-        return '__all__';
-      case DataCategory.dataCategoryDefault:
-        return 'default';
-      case DataCategory.error:
-        return 'error';
-      case DataCategory.session:
-        return 'session';
-      case DataCategory.transaction:
-        return 'transaction';
-      case DataCategory.attachment:
-        return 'attachment';
-      case DataCategory.security:
-        return 'security';
-      case DataCategory.unknown:
-        return 'unknown';
-    }
-  }
-}
diff --git a/dart/lib/src/transport/rate_limit_parser.dart b/dart/lib/src/transport/rate_limit_parser.dart
@@ -29,7 +29,8 @@ class RateLimitParser {
       if (allCategories.isNotEmpty) {
         final categoryValues = allCategories.split(';');
         for (final categoryValue in categoryValues) {
-          final category = DataCategoryExtension.fromStringValue(categoryValue);
+          final category =
+              _DataCategoryExtension._fromStringValue(categoryValue);
           if (category != DataCategory.unknown) {
             rateLimits.add(RateLimit(category, duration));
           }
@@ -56,3 +57,25 @@ class RateLimitParser {
     }
   }
 }
+
+extension _DataCategoryExtension on DataCategory {
+  static DataCategory _fromStringValue(String stringValue) {
+    switch (stringValue) {
+      case '__all__':
+        return DataCategory.all;
+      case 'default':
+        return DataCategory.dataCategoryDefault;
+      case 'error':
+        return DataCategory.error;
+      case 'session':
+        return DataCategory.session;
+      case 'transaction':
+        return DataCategory.transaction;
+      case 'attachment':
+        return DataCategory.attachment;
+      case 'security':
+        return DataCategory.security;
+    }
+    return DataCategory.unknown;
+  }
+}
diff --git a/dart/lib/src/utils/http_sanitizer.dart b/dart/lib/src/utils/http_sanitizer.dart
@@ -60,7 +60,7 @@ class HttpSanitizer {
   }
 }
 
-extension UriPath on Uri {
+extension _UriPath on Uri {
   String _urlWithRedactedAuth() {
     var buffer = '';
     if (scheme.isNotEmpty) {
@@ -78,6 +78,7 @@ extension UriPath on Uri {
   }
 }
 
+@internal
 extension SanitizedSentryRequest on SentryRequest {
   SentryRequest sanitized() {
     final urlDetails = HttpSanitizer.sanitizeUrl(url) ?? UrlDetails();
diff --git a/dart/lib/src/utils/isolate_utils.dart b/dart/lib/src/utils/isolate_utils.dart
@@ -1,4 +1,7 @@
+import 'package:meta/meta.dart';
+
 import '_io_get_isolate_name.dart'
     if (dart.library.html) '_web_get_isolate_name.dart' as isolate_getter;
 
+@internal
 String? getIsolateName() => isolate_getter.getIsolateName();
diff --git a/dart/lib/src/utils/iterable_extension.dart b/dart/lib/src/utils/iterable_extension.dart
diff --git a/dart/lib/src/utils/iterable_utils.dart b/dart/lib/src/utils/iterable_utils.dart
@@ -0,0 +1,17 @@
+import 'package:meta/meta.dart';
+
+@internal
+class IterableUtils {
+  static T? firstWhereOrNull<T>(
+    Iterable<T>? iterable,
+    bool Function(T item) test,
+  ) {
+    if (iterable == null) {
+      return null;
+    }
+    for (var item in iterable) {
+      if (test(item)) return item;
+    }
+    return null;
+  }
+}
diff --git a/dart/test/sentry_client_test.dart b/dart/test/sentry_client_test.dart
@@ -12,6 +12,7 @@ import 'package:sentry/src/sentry_item_type.dart';
 import 'package:sentry/src/sentry_stack_trace_factory.dart';
 import 'package:sentry/src/sentry_tracer.dart';
 import 'package:sentry/src/transport/data_category.dart';
+import 'package:sentry/src/utils/iterable_utils.dart';
 import 'package:test/test.dart';
 
 import 'mocks.dart';
@@ -1426,8 +1427,10 @@ void main() {
       await sut.captureEvent(fakeEvent, hint: hint);
 
       final capturedEnvelope = (fixture.transport).envelopes.first;
-      final attachmentItem = capturedEnvelope.items.firstWhereOrNull(
-          (element) => element.header.type == SentryItemType.attachment);
+      final attachmentItem = IterableUtils.firstWhereOrNull(
+        capturedEnvelope.items,
+        (SentryEnvelopeItem e) => e.header.type == SentryItemType.attachment,
+      );
       expect(attachmentItem?.header.attachmentType,
           SentryAttachment.typeAttachmentDefault);
     });
diff --git a/dart/test/utils/http_sanitizer_test.dart b/dart/test/utils/http_sanitizer_test.dart
@@ -182,7 +182,7 @@ void main() {
   });
 }
 
-extension StringExtension on String {
+extension _StringExtension on String {
   String _capitalize() {
     return "${this[0].toUpperCase()}${substring(1).toLowerCase()}";
   }
diff --git a/dio/lib/src/failed_request_interceptor.dart b/dio/lib/src/failed_request_interceptor.dart
@@ -27,7 +27,7 @@ class FailedRequestInterceptor extends Interceptor {
     final captureFailedRequests = _hub.options.captureFailedRequests;
 
     final containsStatusCode =
-        _failedRequestStatusCodes.containsStatusCode(err.response?.statusCode);
+        _failedRequestStatusCodes._containsStatusCode(err.response?.statusCode);
     final containsRequestTarget = containsTargetOrMatchesRegExp(
       _failedRequestTargets,
       err.requestOptions.path,
@@ -46,7 +46,7 @@ class FailedRequestInterceptor extends Interceptor {
 }
 
 extension _ListX on List<SentryStatusCode> {
-  bool containsStatusCode(int? statusCode) {
+  bool _containsStatusCode(int? statusCode) {
     if (statusCode == null) {
       return false;
     }
diff --git a/flutter/lib/src/user_interaction/sentry_user_interaction_widget.dart b/flutter/lib/src/user_interaction/sentry_user_interaction_widget.dart
@@ -313,7 +313,8 @@ class _SentryUserInteractionWidgetState
 
   void _onTappedAt(Offset position) {
     final tappedWidget = _getElementAt(position);
-    final keyValue = tappedWidget?.element.widget.key?.toStringValue();
+    final keyValue =
+        WidgetUtils.toStringValue(tappedWidget?.element.widget.key);
     if (tappedWidget == null || keyValue == null) {
       return;
     }
diff --git a/flutter/lib/src/view_hierarchy/sentry_tree_walker.dart b/flutter/lib/src/view_hierarchy/sentry_tree_walker.dart
@@ -281,7 +281,7 @@ class _TreeWalker {
     return SentryViewHierarchyElement(
       element.widget.runtimeType.toString(),
       depth: element.depth,
-      identifier: element.widget.key?.toStringValue(),
+      identifier: WidgetUtils.toStringValue(element.widget.key),
       width: width,
       height: height,
       x: x,
diff --git a/flutter/lib/src/widget_utils.dart b/flutter/lib/src/widget_utils.dart
@@ -1,8 +1,12 @@
 import 'package:flutter/widgets.dart';
+import 'package:meta/meta.dart';
 
-extension WidgetExtension on Key {
-  String? toStringValue() {
-    final key = this;
+@internal
+class WidgetUtils {
+  static String? toStringValue(Key? key) {
+    if (key == null) {
+      return null;
+    }
     if (key is ValueKey<String>) {
       return key.value;
     } else if (key is ValueKey) {

Original file line number	Diff line number	Diff line change
`@@ -135,7 +135,7 @@ class FailedRequestClient extends BaseClient {`
`135`	`135`
`136`	`136`	// Only check `failedRequestStatusCodes` & `failedRequestTargets` if no exception was thrown.
`137`	`137`	`if (exception == null) {`
`138`		`- if (!failedRequestStatusCodes.containsStatusCode(statusCode)) {`
	`138`	`+ if (!failedRequestStatusCodes._containsStatusCode(statusCode)) {`
`139`	`139`	`return;`
`140`	`140`	`}`
`141`	`141`	`if (!containsTargetOrMatchesRegExp(`
`@@ -246,7 +246,7 @@ class FailedRequestClient extends BaseClient {`
`246`	`246`	`}`
`247`	`247`
`248`	`248`	`extension _ListX on List<SentryStatusCode> {`
`249`		`- bool containsStatusCode(int? statusCode) {`
	`249`	`+ bool _containsStatusCode(int? statusCode) {`
`250`	`250`	`if (statusCode == null) {`
`251`	`251`	`return false;`
`252`	`252`	`}`
Original file line number	Diff line number	Diff line change
`@@ -60,7 +60,7 @@ class HttpSanitizer {`
`60`	`60`	`}`
`61`	`61`	`}`
`62`	`62`
`63`		`-extension UriPath on Uri {`
	`63`	`+extension _UriPath on Uri {`
`64`	`64`	`String _urlWithRedactedAuth() {`
`65`	`65`	`var buffer = '';`
`66`	`66`	`if (scheme.isNotEmpty) {`
`@@ -78,6 +78,7 @@ extension UriPath on Uri {`
`78`	`78`	`}`
`79`	`79`	`}`
`80`	`80`
	`81`	`+@internal`
`81`	`82`	`extension SanitizedSentryRequest on SentryRequest {`
`82`	`83`	`SentryRequest sanitized() {`
`83`	`84`	`final urlDetails = HttpSanitizer.sanitizeUrl(url) ?? UrlDetails();`