Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do not leak extensions of external classes #1576

Merged
merged 10 commits into from
Sep 4, 2023
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • 🚫 The changelog entry seems to be part of an already released section ## 8.0.0.
    Consider moving the entry to the ## Unreleased section, please.


## Unreleased

Expand Down
21 changes: 0 additions & 21 deletions dart/lib/src/client_reports/discard_reason.dart
Original file line number Diff line number Diff line change
Expand Up @@ -12,24 +12,3 @@ enum DiscardReason {
cacheOverflow,
rateLimitBackoff,
}

extension OutcomeExtension on DiscardReason {
denrase marked this conversation as resolved.
Show resolved Hide resolved
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';
}
}
}
48 changes: 46 additions & 2 deletions dart/lib/src/client_reports/discarded_event.dart
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,53 @@

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:

Check warning on line 53 in dart/lib/src/client_reports/discarded_event.dart

View check run for this annotation

Codecov / codecov/patch

dart/lib/src/client_reports/discarded_event.dart#L53

Added line #L53 was not covered by tests
return 'session';
case DataCategory.transaction:

Check warning on line 55 in dart/lib/src/client_reports/discarded_event.dart

View check run for this annotation

Codecov / codecov/patch

dart/lib/src/client_reports/discarded_event.dart#L55

Added line #L55 was not covered by tests
return 'transaction';
case DataCategory.attachment:

Check warning on line 57 in dart/lib/src/client_reports/discarded_event.dart

View check run for this annotation

Codecov / codecov/patch

dart/lib/src/client_reports/discarded_event.dart#L57

Added line #L57 was not covered by tests
return 'attachment';
case DataCategory.security:

Check warning on line 59 in dart/lib/src/client_reports/discarded_event.dart

View check run for this annotation

Codecov / codecov/patch

dart/lib/src/client_reports/discarded_event.dart#L59

Added line #L59 was not covered by tests
return 'security';
case DataCategory.unknown:

Check warning on line 61 in dart/lib/src/client_reports/discarded_event.dart

View check run for this annotation

Codecov / codecov/patch

dart/lib/src/client_reports/discarded_event.dart#L61

Added line #L61 was not covered by tests
return 'unknown';
}
}
}
4 changes: 2 additions & 2 deletions dart/lib/src/http_client/failed_request_client.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -246,7 +246,7 @@ class FailedRequestClient extends BaseClient {
}

extension _ListX on List<SentryStatusCode> {
bool containsStatusCode(int? statusCode) {
bool _containsStatusCode(int? statusCode) {
denrase marked this conversation as resolved.
Show resolved Hide resolved
if (statusCode == null) {
return false;
}
Expand Down
9 changes: 5 additions & 4 deletions dart/lib/src/protocol/sentry_request.dart
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -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;

Expand Down
10 changes: 6 additions & 4 deletions dart/lib/src/protocol/sentry_response.dart
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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) {
Expand Down
43 changes: 0 additions & 43 deletions dart/lib/src/transport/data_category.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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';
}
}
}
25 changes: 24 additions & 1 deletion dart/lib/src/transport/rate_limit_parser.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}
Expand All @@ -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;
}
}
3 changes: 2 additions & 1 deletion dart/lib/src/utils/http_sanitizer.dart
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ class HttpSanitizer {
}
}

extension UriPath on Uri {
extension _UriPath on Uri {
String _urlWithRedactedAuth() {
var buffer = '';
if (scheme.isNotEmpty) {
Expand All @@ -78,6 +78,7 @@ extension UriPath on Uri {
}
}

@internal
extension SanitizedSentryRequest on SentryRequest {
SentryRequest sanitized() {
final urlDetails = HttpSanitizer.sanitizeUrl(url) ?? UrlDetails();
Expand Down
3 changes: 3 additions & 0 deletions dart/lib/src/utils/isolate_utils.dart
Original file line number Diff line number Diff line change
@@ -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();
8 changes: 0 additions & 8 deletions dart/lib/src/utils/iterable_extension.dart

This file was deleted.

17 changes: 17 additions & 0 deletions dart/lib/src/utils/iterable_utils.dart
Original file line number Diff line number Diff line change
@@ -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;
}
}
7 changes: 5 additions & 2 deletions dart/test/sentry_client_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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);
});
Expand Down
2 changes: 1 addition & 1 deletion dart/test/utils/http_sanitizer_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ void main() {
});
}

extension StringExtension on String {
extension _StringExtension on String {
String _capitalize() {
return "${this[0].toUpperCase()}${substring(1).toLowerCase()}";
}
Expand Down
4 changes: 2 additions & 2 deletions dio/lib/src/failed_request_interceptor.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
2 changes: 1 addition & 1 deletion flutter/lib/src/view_hierarchy/sentry_tree_walker.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
10 changes: 7 additions & 3 deletions flutter/lib/src/widget_utils.dart
Original file line number Diff line number Diff line change
@@ -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) {
Expand Down
Loading