touren / twitter-kit-ios

Twitter Kit is a native SDK to include Twitter content inside mobile apps.
Apache License 2.0
110 stars 80 forks source link

Many crash in TWTRAPIClient #29

Open yasuradodo opened 3 years ago

yasuradodo commented 3 years ago

I cannot reproduced the issue, but I got +5k crash reports in this past few hours from our app.

Seems the crash is happened inside of TWTRAPIClient.

- (NSURLRequest *)URLRequestWithMethod:(NSString *)method URLString:(NSString *)URLString parameters:(nullable NSDictionary *)parameters error:(NSError **)error;

My code

private let client: TWTRAPIClient

func friendIds() -> Promise<[String]> {
    Promise { resolver in
        let request = client.urlRequest( // <-- Crashed here
            withMethod: "GET",
            urlString: "https://api.twitter.com/1.1/friends/ids.json",
            parameters: ["stringify_ids": "1"],
            error: nil
        )
        client.sendTwitterRequest(request) { (_, data, error) in
            guard let data = data else {
                let error = error.flatMap {
                    TwitterError(error: $0 as NSError)
                } ?? TwitterError.illegalStateError
                resolver.reject(error)
                return
            }
            do {
                let response = try self.decoder.decode(FriendIdsResponse.self, from: data)
                resolver.fulfill(response.ids)
            } catch {
                resolver.reject(TwitterError(error: error as NSError))
            }
        }
    }
}

log

Does anyone have same issue?

inket commented 3 years ago

We have the same issue. 1K crashes in one hour. I will be tracking this post and adding any updates from our side 👍

inket commented 3 years ago

My findings:

TWTRGuestAuthRequestSigner.m#L31

[mutableRequest setValue:session.guestToken forHTTPHeaderField:TWTRGuestTokenHeaderField];

^ From the crash report we can tell that session.guestToken is a NSNumber when it should be an NSString.

Chasing the guestToken through many files, we get to its source: It's retrieved from Twitter's servers which provide a JSON response that's deserialized here:

TWTRAuthenticationProvider.m#L38

json = [NSJSONSerialization JSONObjectWithData:data options:0 error:&jsonError];

Meaning Twitter changed the token it returns from string to number.

The recommended fix would be something like this: TWTRGuestSession.m#L47

- NSString *guestToken = sessionDictionary[TWTRGuestAuthOAuthTokenKey];
+ NSString *guestToken = nil;
+ id rawToken = sessionDictionary[TWTRGuestAuthOAuthTokenKey];
+ if ([rawToken isKindOfClass:NSNumber.class]) {
+     guestToken = [(NSNumber *)rawToken stringValue];
+ } else {
+     guestToken = (NSString *)rawToken;
+ }

(assuming Twitter's API accepts their token as a string in later requests)

We have not tried/confirmed the fix yet since we cannot reproduce the crash. If you're able to reproduce it, please give it a try.

wvabrinskas commented 3 years ago

I was able to reproduce this using a jailbroken device and modifying (NSURLRequest *)signedURLRequest:(NSURLRequest *)URLRequest session:(TWTRGuestSession *)session at runtime:

%hook TWTRGuestAuthRequestSigner

+(NSURLRequest *)signedURLRequest:(NSURLRequest *)URLRequest session:(TWTRGuestSession *)session {
    NSLog(@"request");
    NSMutableURLRequest *mutableRequest = [URLRequest mutableCopy];

    \\here is where I replaced the guestToken with an NSNumber 
    [mutableRequest setValue:[NSNumber numberWithInt: 0123123] forHTTPHeaderField:TWTRGuestTokenHeaderField];

    NSString *appToken = [NSString stringWithFormat:@"Bearer %@", session.accessToken];
    [mutableRequest setValue:appToken forHTTPHeaderField:TWTRAuthorizationHeaderField];
    return mutableRequest;  
}
%end
124154038-14c4ee80-da63-11eb-9e36-1bb4b8620f24

I was able to replicate the crash in Xcode. It is exactly the same or at least very similar as I've been seeing in my logs.

It is in fact due to guestToken not being of type NSString since NSNumber doesn't have a length method.

inket commented 3 years ago

Twitter seems to have fixed their mistake since our crashes have basically stopped. We still have zero acknowledgment of the issue from Twitter though.

yasuradodo commented 3 years ago

Twitter seems to have fixed their mistake since our crashes have basically stopped. We still have zero acknowledgment of the issue from Twitter though.

Same in my project. It's stoped about 10 hours ago.