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

Fix #2539: Super Referrer Improvements (Clipboard) #2535

Merged
merged 16 commits into from May 25, 2020
Merged
Changes from 1 commit
Commits
File filter...
Filter file types
Jump to…
Jump to file
Failed to load files.

Always

Just for now

Fixed crash from bad referral situation.

  • Loading branch information
jhreis committed May 20, 2020
commit 834a1fcd47d7ae55cac2d571a5cf34ed83216351
@@ -196,7 +196,11 @@ public class UserReferralProgram {
/// Uses very strict matching.
/// Returns the sanitized code, or nil if no code was found
public class func sanitize(input: String?) -> String? {
guard var input = input, input.hasPrefix(self.clipboardPrefix) else { return nil }
guard
var input = input,
input.hasPrefix(self.clipboardPrefix),
input.count > self.clipboardPrefix.count
else { return nil }

// +1 to strip off `:` that proceeds the defined prefix
input.removeFirst(self.clipboardPrefix.count + 1)
This conversation was marked as resolved by jhreis

This comment has been minimized.

Copy link
@kylehickinson

kylehickinson May 11, 2020

Contributor

Understand this probably set in a spec somewhere, but maybe a comment about why + 1?

This comment has been minimized.

Copy link
@jhreis

jhreis May 11, 2020

Author Contributor

Nah, lol, yeah, will do!

This conversation was marked as resolved by Brandon-T

This comment has been minimized.

Copy link
@Brandon-T

Brandon-T May 20, 2020

Collaborator

If input is exactly the same length as clipboardPrefix, then removing count + 1 is undefined behaviour and could potentially cause a crash.

The number of elements to remove from the collection. k must be greater than or equal to zero and must not exceed the number of elements in the collection.

We can add to the guard statement that input.count > self.clipboardPrefix.count + 1
Is it possible that the ref-code after the prefix could potentially be blank? I'm not sure, but I think some people might try to manipulate it :D

This comment has been minimized.

Copy link
@jhreis

jhreis May 20, 2020

Author Contributor

Good catch, thanks @Brandon-T

This comment has been minimized.

Copy link
@jhreis

jhreis May 20, 2020

Author Contributor

I added a unit test to cover this too, thanks for pointing out.

@@ -56,6 +56,9 @@ class UserReferralProgramTests: XCTestCase {

response = UserReferralProgram.sanitize(input: "\(UserReferralProgramTests.clipboardPrefix):")
XCTAssertNil(response)

response = UserReferralProgram.sanitize(input: "\(UserReferralProgramTests.clipboardPrefix)")
XCTAssertNil(response)
}

func testNoSpacer() throws {
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.