Sentry extension erroring when email present in the URL when passed through the sanitiser

[vlazdra]

Platform

Dart

Obfuscation

Disabled

Debug Info

Disabled

Doctor

[✓] Flutter (Channel stable, 3.7.12, on macOS 13.1 22C65 darwin-arm64, locale en-RS)
• Flutter version 3.7.12 on channel stable at /Users/vlazdra/flutter
• Upstream repository https://github.com/flutter/flutter.git
• Framework revision 4d9e56e694 (9 days ago), 2023-04-17 21:47:46 -0400
• Engine revision 1a65d409c7
• Dart version 2.19.6
• DevTools version 2.20.1

[✓] Android toolchain - develop for Android devices (Android SDK version 33.0.2)
• Android SDK at /Users/vlazdra/Library/Android/sdk
• Platform android-33, build-tools 33.0.2
• ANDROID_HOME = /Users/vlazdra/Library/Android/sdk
• Java binary at: /Applications/Android Studio.app/Contents/jbr/Contents/Home/bin/java
• Java version OpenJDK Runtime Environment (build 17.0.6+0-17.0.6b802.4-9586694)
• All Android licenses accepted.

[✓] Xcode - develop for iOS and macOS (Xcode 14.3)
• Xcode at /Applications/Xcode.app/Contents/Developer
• Build 14E222b
• CocoaPods version 1.12.1

[✓] Chrome - develop for the web
• CHROME_EXECUTABLE = /Applications/Brave Browser.app/Contents/MacOS/Brave Browser

[✓] Android Studio (version 2022.2)
• Android Studio at /Applications/Android Studio.app/Contents
• Flutter plugin can be installed from:
🔨 https://plugins.jetbrains.com/plugin/9212-flutter
• Dart plugin can be installed from:
🔨 https://plugins.jetbrains.com/plugin/6351-dart
• Java version OpenJDK Runtime Environment (build 17.0.6+0-17.0.6b802.4-9586694)

[✓] IntelliJ IDEA Community Edition (version 2023.1)
• IntelliJ at /Applications/IntelliJ IDEA CE.app
• Flutter plugin version 73.0.4
• Dart plugin version 231.8109.91

[✓] VS Code (version 1.77.3)
• VS Code at /Applications/Visual Studio Code.app/Contents
• Flutter extension version 3.40.0

[✓] Connected device (3 available)
• sdk gphone64 arm64 (mobile) • emulator-5554 • android-arm64 • Android 13 (API 33) (emulator)
• macOS (desktop) • macos • darwin-arm64 • macOS 13.1 22C65 darwin-arm64
• Chrome (web) • chrome • web-javascript • Brave Browser 112.1.50.121

[✓] HTTP Host Availability
• All required HTTP hosts are available

Version

7.5.0

Steps to Reproduce

It might be reproducable if the URL contains an email address, not 100% sure that it will do.
I receive an unknown Dio error with this message: FormatException: Invalid character (at character 9) https://[Filtered]@EXAMPLE.COM

Removing the Dio extension: dioClient.addSentry(); "fixes" the issue.

Expected Result

The parsing is successful.
Again, not sure if the parsing part actually fails, but I do see that Dio returning an Unknown error because of it.

Actual Result

The URL that is being process looks like this:
https://staging.server.com/api/v4/auth/password/reset/[email protected]

Possible regression of this PR: #1327

Are you willing to submit a PR?

No

[marandaneto]

@vlazdra Thanks for reporting.
Can you give more details about when this happens and which features you have enabled?
Is it adding breadcrumbs?
Is it capturing client errors?
Is it when measuring performance? (spans)

Please share your SDK init code snippet and the code snippet of when setting up dio, thanks.

[marandaneto]

The result here is wrong:

sentry-dart/dart/lib/src/utils/http_sanitizer.dart

Lines 63 to 75 in c4f1621

	static String _urlWithAuthRemoved(String url) {
	final userInfoMatch = _authRegExp.firstMatch(url);
	if (userInfoMatch != null && userInfoMatch.groupCount == 3) {
	final userInfoString = userInfoMatch.group(2) ?? '';
	final replacementString = userInfoString.contains(":")
	? "[Filtered]:[Filtered]@"
	: "[Filtered]@";
	return '${userInfoMatch.group(1) ?? ''}$replacementString${userInfoMatch.group(3) ?? ''}';
	} else {
	return url;
	}
	}
	}

The output is https://[Filtered]@example.com for urlDetails?.urlOrFallback but @example.com is part of the email and not the URL or userInfo.
It's not throwing FormatException tho.

[marandaneto]

@denrase can you check this? probably the same problem here.

[vlazdra]

Hey @marandaneto! 👋

This is the setup of the Sentry SDK:

  await SentryFlutter.init((options) {
    options.dsn = 'dsn value';
    // Set tracesSampleRate to 1.0 to capture 100% of transactions for performance monitoring.
    // We recommend adjusting this value in production.
    options.tracesSampleRate = 1.0;
    options.attachScreenshot = true;
  });

Regarding the Dio setup, it's pretty basic, I'm adding 5 different Interceptors but when I do enable the Sentry extensions that when I get the FormatException. Even if I remove all of my custom Interceptors and just leave the basic Dio setup and the Sentry extensions, it still errors with FormatException.

_dioClient.addSentry();

The worst things is that I'm unable to catch where exactly the issue is ocuring. The closest I thought where something might go wrong is with the new change.

Also, I'm not using Breadcrumbs directly at this stage, and nor am I using the Transaction option at this stage as well as Performance tracing.

Version of Sentry: 7.5.0 (both dart and flutter packages)
Version of Dio: 5.1.1

[marandaneto]

@vlazdra right, it's probably a different URL then, maybe similar to https://staging.server.com/api/v4/auth/password/reset/[email protected] but not exactly, which fails during parsing, is there any way to be 100% sure of its format? via crumbs, debugging, etc?
We'll check if there's a need to at least wrap the whole thing in a try/catch, if not yet.

[vlazdra]

@marandaneto I think I got it. At least where it fails to parse and throws a FormatException.

In the end it's a completely different place than what was reported, as per usual when it comes to these type of issues. 😄

This line is the problem:

sentry-dart/dio/lib/src/breadcrumb_client_adapter.dart

Line 64 in c4f1621

url: Uri.parse(urlDetails.urlOrFallback),

[vlazdra]

It was this change that was introduced a month ago. I just came around to update the dependencies in my project.

24f71aa#diff-88768c3d418bb22a5112992f1828b4a2a9ffcca36f9080b9b5127e962f6880d8R85

[marandaneto]

@vlazdra Right, that makes sense.
#1414
This fixes the issue with the error, another PR has to follow up to fix the formatting.

[vlazdra]

Closing the issue since the PR (#1414) has been merged.
Thansk @marandaneto 🙌

[marandaneto]

@krystofwoldrich raise a new issue and assign it to @denrase , related to #1412 (comment)
The format exception is fixed but not the formatting, the output is still wrong, thanks.