-
Notifications
You must be signed in to change notification settings - Fork 54
Sanitize email user attribute before sending to checkout API #462
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
base: develop
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,29 @@ | ||||||||||||||||
| // | ||||||||||||||||
| // Email.swift | ||||||||||||||||
| // SuperwallKit | ||||||||||||||||
| // | ||||||||||||||||
|
|
||||||||||||||||
| import Foundation | ||||||||||||||||
|
|
||||||||||||||||
| /// A validated email address. | ||||||||||||||||
| /// | ||||||||||||||||
| /// The failable initializer rejects any string that does not match the | ||||||||||||||||
| /// pattern expected by the checkout API (`^[a-zA-Z0-9._%+-]+@[a-zA-Z0-9.-]+\.[a-zA-Z]{2,}$`). | ||||||||||||||||
| /// Holding an `Email` instance proves the value was validated — downstream | ||||||||||||||||
| /// code never needs to re-check. | ||||||||||||||||
| struct Email: Equatable, Sendable { | ||||||||||||||||
| let rawValue: String | ||||||||||||||||
|
|
||||||||||||||||
| private static let regex = try! NSRegularExpression( | ||||||||||||||||
| pattern: #"^[a-zA-Z0-9._%+-]+@[a-zA-Z0-9.-]+\.[a-zA-Z]{2,}$"# | ||||||||||||||||
| ) | ||||||||||||||||
|
Comment on lines
+17
to
+19
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The pattern is a compile-time literal and will never throw, so the force-try is safe. Adding a short comment (or a
Suggested change
Prompt To Fix With AIThis is a comment left during a code review.
Path: Sources/SuperwallKit/Identity/Email.swift
Line: 17-19
Comment:
**Consider adding a comment or explicit assertion for `try!`**
The pattern is a compile-time literal and will never throw, so the force-try is safe. Adding a short comment (or a `preconditionFailure` wrapper) makes that intent explicit and prevents future maintainers from worrying about it.
```suggestion
// Pattern is a validated literal — initialization is guaranteed to succeed.
private static let regex = try! NSRegularExpression( // swiftlint:disable:this force_try
pattern: #"^[a-zA-Z0-9._%+-]+@[a-zA-Z0-9.-]+\.[a-zA-Z]{2,}$"#
)
```
How can I resolve this? If you propose a fix, please make it concise.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time! |
||||||||||||||||
|
|
||||||||||||||||
| /// Returns `nil` when `rawValue` is not a syntactically valid email address. | ||||||||||||||||
| init?(_ rawValue: String) { | ||||||||||||||||
| let range = NSRange(rawValue.startIndex..., in: rawValue) | ||||||||||||||||
| guard Self.regex.firstMatch(in: rawValue, range: range) != nil else { | ||||||||||||||||
| return nil | ||||||||||||||||
| } | ||||||||||||||||
| self.rawValue = rawValue | ||||||||||||||||
| } | ||||||||||||||||
| } | ||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,131 @@ | ||
| // | ||
| // EmailTests.swift | ||
| // SuperwallKit | ||
| // | ||
|
|
||
| import Testing | ||
| @testable import SuperwallKit | ||
|
|
||
| @Suite("Email") | ||
| struct EmailTests { | ||
|
|
||
| // MARK: - Valid emails | ||
|
|
||
| @Test("accepts a simple email address") | ||
| func `simple email`() { | ||
| #expect(Email("user@example.com") != nil) | ||
| } | ||
|
|
||
| @Test("accepts email with dots in local part") | ||
| func `dotted local part`() { | ||
| #expect(Email("first.last@domain.co") != nil) | ||
| } | ||
|
|
||
| @Test("accepts email with plus tag") | ||
| func `plus tag`() { | ||
| #expect(Email("user+tag@example.com") != nil) | ||
| } | ||
|
|
||
| @Test("accepts email with subdomain") | ||
| func `subdomain`() { | ||
| #expect(Email("user@mail.example.co.uk") != nil) | ||
| } | ||
|
|
||
| @Test("accepts email with numbers") | ||
| func `numbers`() { | ||
| #expect(Email("user123@domain456.com") != nil) | ||
| } | ||
|
|
||
| @Test("accepts email with hyphen in domain") | ||
| func `hyphenated domain`() { | ||
| #expect(Email("user@my-domain.com") != nil) | ||
| } | ||
|
|
||
| @Test("accepts email with underscore and percent") | ||
| func `underscore and percent`() { | ||
| #expect(Email("user_%name@domain.org") != nil) | ||
| } | ||
|
|
||
| @Test("preserves the raw value on success") | ||
| func `preserves raw value`() { | ||
| let address = "hello@world.com" | ||
| let email = Email(address) | ||
| #expect(email?.rawValue == address) | ||
| } | ||
|
|
||
| // MARK: - Invalid emails | ||
|
|
||
| @Test( | ||
| "rejects strings that are not valid email addresses", | ||
| arguments: [ | ||
| "none", | ||
| "", | ||
| "userexample.com", | ||
| "user@", | ||
| "@example.com", | ||
| "user@domain", | ||
| "user@domain.a", | ||
| "user @example.com", | ||
| "not an email", | ||
| "null", | ||
| "N/A", | ||
| ] | ||
| ) | ||
| func `rejects invalid value`(value: String) { | ||
| #expect(Email(value) == nil) | ||
| } | ||
|
|
||
| // MARK: - Equatable | ||
|
|
||
| @Test("two emails with the same address are equal") | ||
| func `equal emails`() { | ||
| #expect(Email("a@b.com") == Email("a@b.com")) | ||
| } | ||
|
|
||
| @Test("two emails with different addresses are not equal") | ||
| func `different emails`() { | ||
| #expect(Email("a@b.com") != Email("x@y.com")) | ||
| } | ||
| } | ||
|
|
||
| // MARK: - sanitizeAttribute | ||
|
|
||
| @Suite("Superwall.sanitizeAttribute") | ||
| struct SanitizeAttributeTests { | ||
|
|
||
| @Test("passes a valid email through unchanged") | ||
| func `valid email passes through`() { | ||
| let result = Superwall.sanitizeAttribute(key: "email", value: "user@example.com") | ||
| #expect(result as? String == "user@example.com") | ||
| } | ||
|
|
||
| @Test("replaces the placeholder 'none' with nil for the email key") | ||
| func `none placeholder becomes nil`() { | ||
| let result = Superwall.sanitizeAttribute(key: "email", value: "none") | ||
| #expect(result == nil) | ||
| } | ||
|
|
||
| @Test("replaces an empty string with nil for the email key") | ||
| func `empty string becomes nil`() { | ||
| let result = Superwall.sanitizeAttribute(key: "email", value: "") | ||
| #expect(result == nil) | ||
| } | ||
|
|
||
| @Test("does not sanitize non-email keys") | ||
| func `non email key untouched`() { | ||
| let result = Superwall.sanitizeAttribute(key: "name", value: "none") | ||
| #expect(result as? String == "none") | ||
| } | ||
|
|
||
| @Test("does not sanitize non-string values on the email key") | ||
| func `non string value untouched`() { | ||
| let result = Superwall.sanitizeAttribute(key: "email", value: 42) | ||
| #expect(result as? Int == 42) | ||
| } | ||
|
|
||
| @Test("passes nil through unchanged") | ||
| func `nil value passes through`() { | ||
| let result = Superwall.sanitizeAttribute(key: "email", value: nil) | ||
| #expect(result == nil) | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
$anchor may match before a trailing newlineICU regular expressions (used by
NSRegularExpression) treat$as matching at the end of the string or just before a final\ncharacter, even without.anchorsMatchLines. This means"user@example.com\n"would pass validation and be stored with the trailing newline, potentially causing a server rejection. Use\zto strictly anchor at the end of the string in all cases.Prompt To Fix With AI