Ability to catch interceptor IOException #3774
Comments
When a website in a ReactNative WebView sets a cookie with an illegal character, this cookie will automatically be added to any request to the same domain. This happens through: BridgeInterceptor.java (l.84) ReactCookieJarContainer.java (l.44) JavaNetCookieJar.java (l.59) ForwardingCookieHandler.java (l.57) ForwardingCookieHandler.java (l.168) CookieManager.java (l.39) The BridgeInterceptor.java then tries to set a Cookie header, which validates both keys and values, and then crashes. okhttp3.6.0 Headers.java (l.320) I would have expected that the IllegalArgumentException would have been caught by JavaMethodRapper.java (l.376), because that's where the NetworkingModule is triggered to build the okhttp request, but my guess is that this is not caught, because IllegalArgumentException is not an IOException. Interceptor can only throw IOExceptions.
|
Consider implementing your own CookieJar that sanitizes cookies first? No way to recover from these IllegalArgumentExceptions unfortunately. |
|
As you can see above, I already have done this, sadly it had to be done in a react-native fork. I don't work at facebook, so I don't know if this is the best place to do it. All I can do is a pull request and hope for the best. However, this seems like an issue that could affect any app using okhttp in combination with a WebView. Sure, only when an illegal cookie is set in a WebView, but you can't control them all! And once the cookie is persisted, it will crash until it has expired! It makes sense you can't catch an exception thrown on a runnable, but maybe there should be less extreme validation on headers that are set by WebViews? An easier and safer solution might be, that request validation is done on newCall.enqueue() instead of inside the intercept. Does that make sense? |
|
It all makes sense. Unfortunately React Native makes things that should be easy not easy, and it’s quite difficult for OkHttp to accommodate. I think the React Native mainainers should accept your pull request. If they don’t, please let me know. |
|
It was a difficult decision if I should open an issue here or at facebook, I 100% agree that okhttp can't and shouldn't accommodate to something that could've been prevented by normal use of the library. My React Native solution with the CookieJar works, but that's just the solution to the problem that made me open this issue. In my eyes I'm not suggesting a solution to my problem. It's a feature request that would be helpful for all types of projects. The more I think about it, the more it makes sense, a request should be validated when you create it, not somewhere in the future when it's triggered. |
|
I agree with the sentiment but we can't actually implement that reliably. Interceptors can short-circuit, change hosts, or do other things that would break eager validation. |
|
Thanks for the explanation, makes total sense! It's the explanation I needed to close the ticket ;) |
|
Hi @swankjesse, I did a PR, but the guys at Facebook are not really picking it up. What do you think of my solution? Thanks |
I have a feature request / request for help.
The feature:
When you enqueue a call, an interceptor can throw an Exception when trying to create a request, I can't find a way to catch this exception. This only seems possible in synchronous calls.
The reason why I need it:
Right now it seems like the BridgeInterceptor is gathering cookies from my WebView and forwarding these when needed. However, some websites set special characters in their cookies, like ó.
When these cookies are added to a new request, Headers.java throws an IllegalArgumentException which crashes the app.
I'm happy to help and send a PR if you can send me some pointers!
The text was updated successfully, but these errors were encountered: