[TIMOB-18887] Android: Wrong implementation of TiResponseCache may cause occasional crashes in Lollipop

GitHub Issue	n/a
Type	Bug
Priority	Critical
Status	Closed
Resolution	Fixed
Resolution Date	2015-05-20T22:15:43.000+0000
Affected Version/s	n/a
Fix Version/s	Release 4.1.0, Release 4.0.1
Components	Android
Labels	android-5
Reporter	grebulon
Assignee	Hieu Pham
Created	2015-03-19T14:23:17.000+0000
Updated	2015-06-23T20:16:35.000+0000

Description

I was getting occasional crashes in my titanium module when downloading files using HttpUrlConnection on a Lollipop device. Tracing this to okhttp (the http client used in android 5), I posted a bug report (see: https://code.google.com/p/android/issues/detail?id=160522) There is a bug in okhttp where they don't handle null pointer correctly, which is returned from TiCacheResponse.getHeaders() or its null key's value, but the fact is that it shouldn't return null in the first place. This all comes from the way the cached header's multimap assumes that the status is in the null key at position 0 (see: http://developer.android.com/reference/java/net/URLConnection.html#getHeaderFields%28%29) For a full explanation of the cause see: https://code.google.com/p/android/issues/detail?id=160522#c5 Partial stack dump of the problem: {quote} Attempt to invoke virtual method 'boolean java.lang.String.startsWith(java.lang.String)' on a null object reference java.lang.NullPointerException: Attempt to invoke virtual method 'boolean java.lang.String.startsWith(java.lang.String)' on a null object reference at com.android.okhttp.internal.http.StatusLine.(StatusLine.java:24) at com.android.okhttp.Response$Builder.statusLine(Response.java:419) at com.android.okhttp.internal.http.JavaApiConverter.createOkResponse(JavaApiConverter.java:116) at com.android.okhttp.internal.http.ResponseCacheAdapter.get(ResponseCacheAdapter.java:53) at com.android.okhttp.internal.http.HttpEngine.sendRequest(HttpEngine.java:269) at com.android.okhttp.internal.http.HttpURLConnectionImpl.execute(HttpURLConnectionImpl.java:373) at com.android.okhttp.internal.http.HttpURLConnectionImpl.getResponse(HttpURLConnectionImpl.java:323) at com.android.okhttp.internal.http.HttpURLConnectionImpl.getResponseCode(HttpURLConnectionImpl.java:491) at com.android.okhttp.internal.http.DelegatingHttpsURLConnection.getResponseCode(DelegatingHttpsURLConnection.java:105) at com.android.okhttp.internal.http.HttpsURLConnectionImpl.getResponseCode(HttpsURLConnectionImpl.java:25) {quote}

Comments

grebulon 2015-03-22

More discussion of this issue here: https://github.com/square/okhttp/issues/1523 As for TiResponseCache, the problem starts at makeLowerCaseHeaders() where the implementation just throws away the null key-pair, instead of encoding it (say to a "null=xxx" string in the cache file).
grebulon 2015-04-13

My fix is here: https://www.dropbox.com/s/z814rg68p8qz7ew/TiResponseCache.java?dl=0 Look for hopflow comments in the code
Muhammad Ahmed Fahad 2015-04-17

Please resolve this issue soon probably with @grebulon's fix, before more devices get upgraded to Android Lollipop where the effects of this bug are more visible.
Praveen Kodakkad 2015-05-08

Please resolve this issue ASAP.
Ingo Muschenetz 2015-05-11

[~buddyguards] Thank you for the fix. Is it possible to submit it as a pull request? We can't accept it otherwise.
grebulon 2015-05-12

Ingo, my fix is a quick hack so that users can take it and quickly build it into titanium. It should be taken as a basis for a cleaner implementation.

Hieu Pham 2015-05-14

[~buddyguards] Thank you for the fix. Your fix seems pretty straightforward, so I put together a PR for it: https://github.com/appcelerator/titanium_mobile/pull/6850. Test code:

   var vidWin = Titanium.UI.createWindow({
       title : 'Video View Demo',
       backgroundColor : '#fff'
   });
    
   var videoPlayer = Titanium.Media.createVideoPlayer({
       top : 2,
       autoplay : true,
       backgroundColor : 'blue',
       height : 300,
       width : 300,
       mediaControlStyle : Titanium.Media.VIDEO_CONTROL_DEFAULT,
       scalingMode : Titanium.Media.VIDEO_SCALING_ASPECT_FIT
   });
    
   videoPlayer.url = 'http://techslides.com/demos/sample-videos/small.mp4';
   vidWin.add(videoPlayer);
   vidWin.open();

1. Run app, wait until video is finished (autoplay) 2. Hit back button (close the app) 3. Run app again, make sure video played properly

Muhammad Ahmed Fahad 2015-05-21

Can't this fix be put in the next immediate release rather than 4.1.0?
Ingo Muschenetz 2015-05-21

I have marked it as such.
Hieu Pham 2015-05-21

4.0.X backport: https://github.com/appcelerator/titanium_mobile/pull/6868
Lokesh Choudhary 2015-05-29

Verified the fix by running the test case above. Closing. Environment: Appc Studio : 4.0.1.201505282200 Ti SDK : 4.0.1.v20150529150210, 4.1.0.v20150529013634 CLI : 4.0.0 Alloy : 1.6.0 MAC Yosemite : 10.10.3 Appc npm : 4.0.0 Appc CLI : 4.0.1-rc Node: v0.10.37 Nexus 5 - Android 5.1.1

JSON Source