diff --git a/CHANGELOG.md b/CHANGELOG.md index 12ae447ec3..11e20ff96c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ - The HTTP instrumentation uses the span created for the outgoing request in the sentry-trace header, fixing the parent-child relationship between client and server ([#4264](https://github.com/getsentry/sentry-dotnet/pull/4264)) - InvalidOperationException sending attachments on Android with LLVM enabled ([#4276](https://github.com/getsentry/sentry-dotnet/pull/4276)) +- When CaptureFeedback methods are called with invalid email addresses, the email address will be removed and, if Debug mode is enabled, a warning will be logged. This is done to avoid losing the Feedback altogether (Sentry would reject Feedback that has an invalid email address) ([#4284](https://github.com/getsentry/sentry-dotnet/pull/4284)) ### Dependencies diff --git a/src/Sentry/Internal/EmailValidator.cs b/src/Sentry/Internal/EmailValidator.cs new file mode 100644 index 0000000000..482543822d --- /dev/null +++ b/src/Sentry/Internal/EmailValidator.cs @@ -0,0 +1,29 @@ +using System.Text.RegularExpressions; + +namespace Sentry.Internal; + +/// +/// Helper class for email validation. +/// +internal static partial class EmailValidator +{ + private const string EmailPattern = @"^[^@\s]+@[^@\s]+\.[^@\s]+$"; + +#if NET9_0_OR_GREATER + [GeneratedRegex(EmailPattern)] + private static partial Regex Email { get; } +#elif NET8_0 + [GeneratedRegex(EmailPattern)] + private static partial Regex EmailRegex(); + private static readonly Regex Email = EmailRegex(); +#else + private static readonly Regex Email = new(EmailPattern, RegexOptions.Compiled); +#endif + + /// + /// Validates an email address. + /// + /// The email address to validate. + /// True if the email is valid, false otherwise. + public static bool IsValidEmail(string email) => Email.IsMatch(email); +} diff --git a/src/Sentry/Internal/Hub.cs b/src/Sentry/Internal/Hub.cs index 2084e20295..ff9d5de43a 100644 --- a/src/Sentry/Internal/Hub.cs +++ b/src/Sentry/Internal/Hub.cs @@ -573,6 +573,12 @@ public void CaptureFeedback(SentryFeedback feedback, Scope? scope = null, Sentry try { + if (!string.IsNullOrWhiteSpace(feedback.ContactEmail) && !EmailValidator.IsValidEmail(feedback.ContactEmail)) + { + _options.LogWarning("Feedback email scrubbed due to invalid email format: '{0}'", feedback.ContactEmail); + feedback.ContactEmail = null; + } + scope ??= CurrentScope; CurrentClient.CaptureFeedback(feedback, scope, hint); } @@ -620,6 +626,16 @@ public void CaptureUserFeedback(UserFeedback userFeedback) try { + if (!string.IsNullOrWhiteSpace(userFeedback.Email) && !EmailValidator.IsValidEmail(userFeedback.Email)) + { + _options.LogWarning("Feedback email scrubbed due to invalid email format: '{0}'", userFeedback.Email); + userFeedback = new UserFeedback( + userFeedback.EventId, + userFeedback.Name, + null, // Scrubbed email + userFeedback.Comments); + } + CurrentClient.CaptureUserFeedback(userFeedback); } catch (Exception e) diff --git a/test/Sentry.Tests/HubTests.cs b/test/Sentry.Tests/HubTests.cs index 89633c0752..864a92dd2d 100644 --- a/test/Sentry.Tests/HubTests.cs +++ b/test/Sentry.Tests/HubTests.cs @@ -1,5 +1,6 @@ using System.IO.Abstractions.TestingHelpers; using Sentry.Internal.Http; +using Sentry.Protocol; using Sentry.Tests.Internals; namespace Sentry.Tests; @@ -1742,7 +1743,7 @@ public void CaptureUserFeedback_HubEnabled(bool enabled) hub.Dispose(); } - var feedback = new UserFeedback(SentryId.Create(), "foo", "bar", "baz"); + var feedback = new UserFeedback(SentryId.Create(), "foo", "bar@example.com", "baz"); // Act hub.CaptureUserFeedback(feedback); @@ -1890,6 +1891,103 @@ await transport.Received(1) } private static Scope GetCurrentScope(Hub hub) => hub.ScopeManager.GetCurrent().Key; + + [Theory] + [InlineData(null)] + [InlineData("")] + [InlineData(" ")] + [InlineData("test@example.com")] + [InlineData("user.name@domain.com")] + [InlineData("user+tag@example.com")] + public void CaptureFeedback_ValidEmail_FeedbackRegistered(string email) + { + // Arrange + var hub = _fixture.GetSut(); + var feedback = new SentryFeedback("Test feedback", email); + + // Act + hub.CaptureFeedback(feedback); + + // Assert + _fixture.Client.Received(1).CaptureFeedback(Arg.Any(), Arg.Any(), Arg.Any()); + } + + [Theory] + [InlineData("invalid-email")] + [InlineData("missing@domain")] + [InlineData("@missing-local.com")] + [InlineData("spaces in@email.com")] + public void CaptureFeedback_InvalidEmail_FeedbackDropped(string email) + { + // Arrange + _fixture.Options.Debug = true; + _fixture.Options.DiagnosticLogger = Substitute.For(); + _fixture.Options.DiagnosticLogger!.IsEnabled(Arg.Any()).Returns(true); + var hub = _fixture.GetSut(); + var feedback = new SentryFeedback("Test feedback", email); + + // Act + hub.CaptureFeedback(feedback); + + // Assert + _fixture.Options.DiagnosticLogger.Received(1).Log( + SentryLevel.Warning, + Arg.Is(s => s.Contains("invalid email format")), + null, + Arg.Any()); + _fixture.Client.Received(1).CaptureFeedback(Arg.Is(f => f.ContactEmail.IsNull()), + Arg.Any(), Arg.Any()); + } + + [Theory] + [InlineData(null)] + [InlineData("")] + [InlineData(" ")] + [InlineData("test@example.com")] + [InlineData("user.name@domain.com")] + [InlineData("user+tag@example.com")] + public void CaptureUserFeedback_ValidEmail_FeedbackRegistered(string email) + { +#pragma warning disable CS0618 // Type or member is obsolete + // Arrange + var hub = _fixture.GetSut(); + var feedback = new UserFeedback(SentryId.Create(), "Test name", email, "Test comment"); + + // Act + hub.CaptureUserFeedback(feedback); + + // Assert + _fixture.Client.Received(1).CaptureUserFeedback(Arg.Any()); +#pragma warning restore CS0618 // Type or member is obsolete + } + + [Theory] + [InlineData("invalid-email")] + [InlineData("missing@domain")] + [InlineData("@missing-local.com")] + [InlineData("spaces in@email.com")] + public void CaptureUserFeedback_InvalidEmail_FeedbackDropped(string email) + { +#pragma warning disable CS0618 // Type or member is obsolete + // Arrange + _fixture.Options.Debug = true; + _fixture.Options.DiagnosticLogger = Substitute.For(); + _fixture.Options.DiagnosticLogger!.IsEnabled(Arg.Any()).Returns(true); + var hub = _fixture.GetSut(); + var feedback = new UserFeedback(SentryId.Create(), "Test name", email, "Test comment"); + + // Act + hub.CaptureUserFeedback(feedback); + + // Assert + _fixture.Options.DiagnosticLogger.Received(1).Log( + SentryLevel.Warning, + Arg.Is(s => s.Contains("invalid email format")), + null, + Arg.Any()); + _fixture.Client.Received(1).CaptureUserFeedback(Arg.Is(f => f.Email.IsNull())); +#pragma warning restore CS0618 // Type or member is obsolete + } } #if NET6_0_OR_GREATER