Skip to content

Commit

Permalink
Workaround for SafariVC reporting failure while redirecting to iDEAL (#…
Browse files Browse the repository at this point in the history 
…937)

Ignores reported errors from SafariVC unless we know the error was for a `stripe.com`
domain.

SafariVC is basically a black box: give it a URL and ask it to load. It'll report via
the delegate method when the initial load finishes, and whether it was successful or not.

Prior to this commit, iDEAL (and others?) would fail and close SafariVC. However, it hadn't
actually failed, and just leaving the SafariVC open would allow the user to finish
the redirect workflow.

It's strange to me that SafariVC behaves this way. It's also a little strange how the
redirects are being handled by girogate.de. This works around the interaction between
the two, while still attempting to report legitimate errors, so the host application can
tell the user and allow them to retry.

* Add error handling to `STPRedirectContext` completion block errors in example code.
* Rename property and move comment for code review feedback
* Add changelog entry
  • Loading branch information
@danj-stripe
danj-stripe authored May 17, 2018
1 parent 6bf2c15 commit 46df046
Show file tree
Hide file tree
Showing 4 changed files with 92 additions and 53 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
## 13.0.1 2018-05-17
* Fixes bug in `STPRedirectContext` that'd close the `SFSafariViewController` during the initial redirects, but only in livemode. [#937](https://github.com/stripe/stripe-ios/pull/937)

## 13.0.0 2018-04-26
* Removes Bitcoin source support. See MIGRATING.md. [#931](https://github.com/stripe/stripe-ios/pull/931)
* Adds Masterpass support to `STPSourceParams` [#928](https://github.com/stripe/stripe-ios/pull/928)
Expand Down
52 changes: 28 additions & 24 deletions Example/Custom Integration (ObjC)/SofortExampleViewController.m
View file
Original file line number Diff line number Diff line change
Expand Up @@ -100,31 +100,35 @@ - (void)pay {
// your app delegate to forward URLs to the Stripe SDK.
// See `[Stripe handleStripeURLCallback:]`
self.redirectContext = [[STPRedirectContext alloc] initWithSource:source completion:^(NSString *sourceID, NSString *clientSecret, NSError *error) {
[[STPAPIClient sharedClient] startPollingSourceWithId:sourceID
clientSecret:clientSecret
timeout:10
completion:^(STPSource *source, NSError *error) {
[self updateUIForPaymentInProgress:NO];
if (error) {
[self.delegate exampleViewController:self didFinishWithError:error];
} else {
switch (source.status) {
case STPSourceStatusChargeable:
case STPSourceStatusConsumed:
[self.delegate exampleViewController:self didFinishWithMessage:@"Payment successfully created"];
break;
case STPSourceStatusCanceled:
[self.delegate exampleViewController:self didFinishWithMessage:@"Payment failed"];
break;
case STPSourceStatusPending:
case STPSourceStatusFailed:
case STPSourceStatusUnknown:
[self.delegate exampleViewController:self didFinishWithMessage:@"Order received"];
break;
if (error) {
[self.delegate exampleViewController:self didFinishWithError:error];
} else {
[[STPAPIClient sharedClient] startPollingSourceWithId:sourceID
clientSecret:clientSecret
timeout:10
completion:^(STPSource *source, NSError *error) {
[self updateUIForPaymentInProgress:NO];
if (error) {
[self.delegate exampleViewController:self didFinishWithError:error];
} else {
switch (source.status) {
case STPSourceStatusChargeable:
case STPSourceStatusConsumed:
[self.delegate exampleViewController:self didFinishWithMessage:@"Payment successfully created"];
break;
case STPSourceStatusCanceled:
[self.delegate exampleViewController:self didFinishWithMessage:@"Payment failed"];
break;
case STPSourceStatusPending:
case STPSourceStatusFailed:
case STPSourceStatusUnknown:
[self.delegate exampleViewController:self didFinishWithMessage:@"Order received"];
break;
}
}
}
self.redirectContext = nil;
}];
self.redirectContext = nil;
}];
}
}];
[self.redirectContext startRedirectFlowFromViewController:self];
}
Expand Down
52 changes: 28 additions & 24 deletions Example/Custom Integration (ObjC)/ThreeDSExampleViewController.m
View file
Original file line number Diff line number Diff line change
Expand Up @@ -131,31 +131,35 @@ - (void)pay {
// your app delegate to forwards URLs to the Stripe SDK.
// See `[Stripe handleStripeURLCallback:]`
self.redirectContext = [[STPRedirectContext alloc] initWithSource:source completion:^(NSString *sourceID, NSString *clientSecret, NSError *error) {
[[STPAPIClient sharedClient] startPollingSourceWithId:sourceID
clientSecret:clientSecret
timeout:10
completion:^(STPSource *source, NSError *error) {
[self updateUIForPaymentInProgress:NO];
if (error) {
[self.delegate exampleViewController:self didFinishWithError:error];
} else {
switch (source.status) {
case STPSourceStatusChargeable:
case STPSourceStatusConsumed:
[self.delegate exampleViewController:self didFinishWithMessage:@"Payment successfully created"];
break;
case STPSourceStatusCanceled:
[self.delegate exampleViewController:self didFinishWithMessage:@"Payment failed"];
break;
case STPSourceStatusPending:
case STPSourceStatusFailed:
case STPSourceStatusUnknown:
[self.delegate exampleViewController:self didFinishWithMessage:@"Order received"];
break;
if (error) {
[self.delegate exampleViewController:self didFinishWithError:error];
} else {
[[STPAPIClient sharedClient] startPollingSourceWithId:sourceID
clientSecret:clientSecret
timeout:10
completion:^(STPSource *source, NSError *error) {
[self updateUIForPaymentInProgress:NO];
if (error) {
[self.delegate exampleViewController:self didFinishWithError:error];
} else {
switch (source.status) {
case STPSourceStatusChargeable:
case STPSourceStatusConsumed:
[self.delegate exampleViewController:self didFinishWithMessage:@"Payment successfully created"];
break;
case STPSourceStatusCanceled:
[self.delegate exampleViewController:self didFinishWithMessage:@"Payment failed"];
break;
case STPSourceStatusPending:
case STPSourceStatusFailed:
case STPSourceStatusUnknown:
[self.delegate exampleViewController:self didFinishWithMessage:@"Order received"];
break;
}
}
}
self.redirectContext = nil;
}];
self.redirectContext = nil;
}];
}
}];
[self.redirectContext startRedirectFlowFromViewController:self];
}
Expand Down
38 changes: 33 additions & 5 deletions Stripe/STPRedirectContext.m
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ @interface STPRedirectContext () <SFSafariViewControllerDelegate, STPURLCallback
@property (nonatomic, strong) STPSource *source;
@property (nonatomic, strong, nullable) SFSafariViewController *safariVC;
@property (nonatomic, assign, readwrite) STPRedirectContextState state;
/// If we're on iOS 11+ and in the SafariVC flow, this tracks the latest URL loaded/redirected to during the initial load
@property (nonatomic, strong, readwrite, nullable) NSURL *lastKnownSafariVCUrl;

@property (nonatomic, assign) BOOL subscribedToURLNotifications;
@property (nonatomic, assign) BOOL subscribedToForegroundNotifications;
Expand Down Expand Up @@ -121,7 +123,8 @@ - (void)startSafariViewControllerRedirectFlowFromViewController:(UIViewControlle
if (self.state == STPRedirectContextStateNotStarted) {
_state = STPRedirectContextStateInProgress;
[self subscribeToUrlNotifications];
self.safariVC = [[SFSafariViewController alloc] initWithURL:self.source.redirect.url];
self.lastKnownSafariVCUrl = self.source.redirect.url;
self.safariVC = [[SFSafariViewController alloc] initWithURL:self.lastKnownSafariVCUrl];
self.safariVC.delegate = self;
[presentingViewController presentViewController:self.safariVC
animated:YES
Expand Down Expand Up @@ -154,14 +157,39 @@ - (void)safariViewControllerDidFinish:(__unused SFSafariViewController *)control
}

- (void)safariViewController:(__unused SFSafariViewController *)controller didCompleteInitialLoad:(BOOL)didLoadSuccessfully {
/*
SafariVC is, imo, over-eager to report errors. The way that (for example) girogate.de redirects
can cause SafariVC to report that the initial load failed, even though it completes successfully.
So, only report failures to complete the initial load if the host was a Stripe domain.
Stripe uses 302 redirects, and this should catch local connection problems as well as
server-side failures from Stripe.
*/
if (didLoadSuccessfully == NO) {
stpDispatchToMainThreadIfNecessary(^{
[self handleRedirectCompletionWithError:[NSError stp_genericConnectionError]
shouldDismissViewController:YES];
});
if (@available(iOS 11, *)) {
stpDispatchToMainThreadIfNecessary(^{
if ([self.lastKnownSafariVCUrl.host containsString:@"stripe.com"]) {
[self handleRedirectCompletionWithError:[NSError stp_genericConnectionError]
shouldDismissViewController:YES];
}
});
} else {
/*
We can only track the latest URL loaded on iOS 11, because `safariViewController:initialLoadDidRedirectToURL:`
didn't exist prior to that. This might be a spurious error, so we need to ignore it.
*/
}
}
}

- (void)safariViewController:(__unused SFSafariViewController *)controller initialLoadDidRedirectToURL:(NSURL *)URL {
stpDispatchToMainThreadIfNecessary(^{
// This is only kept up to date during the "initial load", but we only need the value in
// `safariViewController:didCompleteInitialLoad:`, so that's fine.
self.lastKnownSafariVCUrl = URL;
});
}

#pragma mark - Private methods -

- (void)handleWillForegroundNotification {
Expand Down

0 comments on commit 46df046

Please sign in to comment.