Opened 22 months ago

Closed 22 months ago

Last modified 20 months ago

#6023 closed defect (fixed)

Connection is not closed if response code was not 200

Reported by: asmirnov Assignee:
Priority: P3 Milestone:
Module: Libadblockplus-Android Keywords:
Cc: Blocked By:
Blocking: Platform: Android
Ready: no Confidential: no
Tester: Unknown Verified working: no
Review URL(s):

https://codereview.adblockplus.org/29603560/

Description

Environment

The connection in AndroidWebRequest (https://github.com/adblockplus/libadblockplus-android/blob/master/libadblockplus-android/src/org/adblockplus/libadblockplus/android/AndroidWebRequest.java#L193) is not closed if request fails (response code != 200), allowing possible leak of internal stream. See connection.disconnect() in positive branch of if (response.getResponseStatus() == 200) only.

The bug comes from original adblockplusandroid sources https://github.com/adblockplus/adblockplusandroid/blob/master/src/org/adblockplus/android/AndroidWebRequest.java#L127

It can be detected in StrictMode with

StrictMode.setVmPolicy(new StrictMode.VmPolicy.Builder()
      .detectAll()
      .penaltyLog()
      .penaltyDeath()
      .build());

and can be seen in the log:

11-10 14:10:07.758 23720-24920 D/WebRequest: Downloading from: https://adblockplus.org/devbuilds/libadblockplus-android/update.json?type=0&addonName=libadblockplus-android&addonVersion=1.0&application=org.chromium.chrome&applicationVersion=Developer%20Build&platform=libadblockplus&platformVersion=1.0&lastVersion=0&downloadCount=0
11-10 14:10:08.708 23720-24920 E/compat.js:68: Adblock Plus: Downloading URL https://adblockplus.org/devbuilds/libadblockplus-android/update.json?type=0 failed (synchronize_connection_error)
                                               Download address: https://adblockplus.org/devbuilds/libadblockplus-android/update.json?type=0&addonName=libadblockplus-android&addonVersion=1.0&application=org.chromium.chrome&applicationVersion=Developer%20Build&platform=libadblockplus&platformVersion=1.0&lastVersion=0&downloadCount=0
                                               Channel status: -2147467259
                                               Server response: 404
                                               
                                               [ 11-10 14:10:08.718 23720:24920 V/         ]
                                               1: reportError() at compat.js:69
                                               2: errorCallback() at downloader.js:249
                                               3: request.addEventListener.event() at downloader.js:307
                                               4: onGetDone() at compat.js:334
11-10 14:10:32.138 23720-23733 E/StrictMode: A resource was acquired at attached stack trace but never released. See java.io.Closeable for information on avoiding resource leaks.
                                             java.lang.Throwable: Explicit termination method 'close' not called
                                                 at dalvik.system.CloseGuard.open(CloseGuard.java:184)
                                                 at com.android.org.conscrypt.OpenSSLSocketImpl.startHandshake(OpenSSLSocketImpl.java:289)
                                                 at com.android.okhttp.Connection.upgradeToTls(Connection.java:1285)
                                                 at com.android.okhttp.Connection.connect(Connection.java:1197)
                                                 at com.android.okhttp.internal.http.HttpEngine.connect(HttpEngine.java:392)
                                                 at com.android.okhttp.internal.http.HttpEngine.sendRequest(HttpEngine.java:295)
                                                 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)
                                                 at org.adblockplus.libadblockplus.android.AndroidWebRequest.httpGET(AndroidWebRequest.java:131)

How to reproduce

  1. Enable strict mode (see the code above)
  2. Just wait for dev download request to happen
  3. Make sure you can see output in the log

...

Observed behaviour

Connection is not closed (and leaked).

Expected behaviour

Conection is closed, no log output.

Change History (4)

comment:1 Changed 22 months ago by asmirnov

  • Review URL(s) modified (diff)
  • Status changed from new to reviewing

comment:2 Changed 22 months ago by abpbot

A commit referencing this issue has landed:
Issue 6023 - Connection is not closed if response code was not 200

comment:3 Changed 22 months ago by asmirnov

  • Resolution set to fixed
  • Status changed from reviewing to closed

comment:4 Changed 20 months ago by abpbot

A commit referencing this issue has landed:
Issue 6023 - Connection is not closed if response code was not 200

Note: See TracTickets for help on using tickets.