Instabug / Instabug-iOS

In-app feedback and bug reporting tool for apps.
https://instabug.com/
Other
289 stars 67 forks source link

Enabling network logging seems to break cookie storage #209

Closed lgeromegnace closed 6 years ago

lgeromegnace commented 7 years ago

Hi all,

My team and I would like to benefit the network logging feature which is very useful. Unfortunately we have had the following issue:

Steps to reproduce the problem

1) Create a URLSessionConfiguration.ephemeral object => let sessionConfig = URLSessionConfiguration.ephemeral 2) Set a customized HTTPCookieStorage object to configuration => sessionConfig.httpCookieStorage = EphemeralCookieStorage() 3) call Instabug.enableLogging(for: sessionConfig) method in order to enable logging

Expected behavior

Network logging is enable and cookies are stored and sent in request's headers

Actual behavior

Network logging is enable BUT cookies are NOT stored and NOT sent in request's headers resulting server errors. Actually it looks like our customized cookie storage class is never used once network logging is enable.

For information, customized HTTPCookieStorage are used here to address this issue: http://www.openradar.me/19852444.

But I assumed that default or ephemeral session configuration should not change the network logging behavior?

SDK version

7.3.12

iOS Version

iOS 10, 11

Device

Simulator, iPhone 6, 6SP, 7

Kmohamed commented 7 years ago

@lgeromegnace Thanks for reporting this issue I will trying to fix it as soon as possible.

lgeromegnace commented 7 years ago

Hi @Kmohamed I just tried this via carthage git "https://github.com/Instabug/Instabug-iOS.git" "fix/HTTPCookiesStorage_NetworkLogger"

But I get the following error: *** Skipped building Instabug-iOS due to the error: Dependency "Instabug-iOS" has no shared framework schemes for any of the platforms: iOS

Did I miss something with Carthage?

BTW I don't see anymore your comment referencing the fix branch... is it still available for test?

Regards

Kmohamed commented 7 years ago

@lgeromegnace I am sorry I think you are using cocoapods. could you give it a try manually ? Instabug.zip

Kmohamed commented 7 years ago

Hello @lgeromegnace did you got a chance to give this custom build a try ?

lgeromegnace commented 7 years ago

Hi, @Kmohamed sorry for the delay I have been interrupted by another topic yesterday. I am on it right now! I come back to you ASAP.

Kmohamed commented 7 years ago

No worries, waiting your feedback.

lgeromegnace commented 7 years ago

So I still have the same issue: none of the custom cookie storage methods are called once I enable network logging.

BUT I have to revise some point in this issue: cookies ARE stored and SENT in request. The problem is that they are not stored with our custom cookie storage. And we need this to handle multiple account (multiple http session) logged at the same time. Otherwise we ended up with wrong cookies sent to the server.

Kmohamed commented 7 years ago

@lgeromegnace let me tell you how network logging works and then you can tell me how this breaks your custom cookie storage. In order to intercept application requests we insert our custom URLProtocol in NSURLSessionConfiguration to catch requests.then we create a copy from your NSURLSessionConfiguration with all attributes including (HTTPCookiesStorage) and perform request on this Session and then return callback to your clients.

lgeromegnace commented 7 years ago

Ok, in theory every thing should work with the description you provided. Nevertheless when we print some debug log to check if our custom class is used, nothing is printed when network logging is enable and on the other hand when it is disable we can see the debug logs.

Note that we use old fashion debug print here instead of breakpoint as a convenience since our network module is implemented in a separate framework.

Is it possible that you intercept the print made by the copied cookie storage? If so that would have led me on the wrong track

lgeromegnace commented 7 years ago

BTW, I was wondering if there is a chance that while intercepting application request that there is a mismatch for the copied NSURLSessionConfiguration object used if the request target is the same?

To give you more context, in our app the user can log into 2 accounts. So there are 2 NSURLSessionConfiguration passed to Instabug for enabling network log but both will request the same server API. I don't know how you store these objects on your side but is there a risk that it is the latest NSURLSessionConfiguration object registered that will be used?

Kmohamed commented 7 years ago

@lgeromegnace maybe this is reason because If you call it twice I used the latest one. So the latest NSURLSessionConfiguration passed will overwrite the first one. But what about making an API that will set NSHTTPCookiesStorage and I will use it in the copied SessionConfiguration ?

lgeromegnace commented 7 years ago

Ok @Kmohamed, but just to be sure I understand: when you said it override the first one, it is only if it targets the same server? Or are you telling me that even if I enable network log for different API, it will always be the last one.

Example: App has to perform requests for servers: A, B, C (with 2 http sessions) with different URLSessionConfiguration. App enable network log for each one consecutively: Instabug.enableLogging(for: sessionConfigA) Instabug.enableLogging(for: sessionConfigB) Instabug.enableLogging(for: sessionConfigC1) Instabug.enableLogging(for: sessionConfigC2)

1) Instabug log requests with sessionConfigC2 for all request performed by the app OR 2) Instabug logs request with sessionConfigA, sessionConfigB, sessionConfigC2?

If it's 2) I can at least try to call enableLogging method each time necessary but if it's 1) don't you think the network logging is a little bit limited? It would be nice to be able to log network call for different APIs.

Kmohamed commented 7 years ago

@lgeromegnace unfortunately the current version of network logging will log requests from sessionConfigC2 for all request performed by the app. But I think your use case is valid so I will make sure to communicate this with the product team. Another point I would like to be sure that the custom build for a single NSURLSessionConfiguration will works fine with your custom HTTPCookiesStorage ? could you give it a try for only a single NSURLSessionConfiguration because I doubt that it will work ( it's not implementing copy protocol) so we will have to add another API in order not to break your custom HTTPCookiesStorage.

lgeromegnace commented 7 years ago

Ok @Kmohamed

But I think your use case is valid so I will make sure to communicate this with the product team.

Yes! thanks that would be great I think. Is there a way to receive updates for this?

Another point I would like to be sure that the custom build for a single NSURLSessionConfiguration will works fine with your custom HTTPCookiesStorage ?

You were right, calling enableLogging only for one account and only one server (i.e one http session) works. Our custom cookie storage is used as it should be.

lgeromegnace commented 6 years ago

Hi @Kmohamed,

Do you have any news on this issue?

Does your fix has been added in a new release of the framework? Does the product team has considered the use case where we want to enable logs for several http sessions for the same server?

Regards,

Kmohamed commented 6 years ago

@lgeromegnace I am very sorry for not replying too quickly First: the fix will be released soon and I will inform you once it is released. Second: we investigated a lot in the issue of multiple session configuration and found that we can not handle it from our side. because we can not be sure that a specific request is related to specific sessionConfiguration. so it will be hard to fix it. I am very sorry for that. 😔

Kmohamed commented 6 years ago

@lgeromegnace I wanna inform you that we released the fix could you update to the latest version ?

miroslavkovac commented 5 years ago

@Kmohamed have you ever found a solution for how to keep track of which session configuration should be used for which request? Could it be that Apple uses some private API for this?

Would be interesting to see how they solved it in _NSURLHTTPProtocol, _NSURLDataProtocol, NSAboutURLProtocol etc.

Kmohamed commented 5 years ago

@miroslavkovac I did not find any solution to keep track of which session configuration should be used for which request. maybe Apple uses some private APIs