From d9b042d9e352a2d35d77b5171fcbcfd7136d9a09 Mon Sep 17 00:00:00 2001 From: kapodamy Date: Thu, 1 Aug 2019 22:41:09 -0300 Subject: [PATCH 1/2] socket leak fix * fix socket leak in "DownloadRunnable" * in "DownloadInitializer" close the HTTP body after doing range-request checks * in "DownloadRunnableFallback" fix typo in comment * in "DownloadDialog" fix regression, using one thread for audios instead of subtitles --- .../newpipe/download/DownloadDialog.java | 2 +- .../giga/get/DownloadInitializer.java | 22 ++++++++++++++-- .../shandian/giga/get/DownloadRunnable.java | 25 +++++++------------ .../giga/get/DownloadRunnableFallback.java | 2 +- 4 files changed, 31 insertions(+), 20 deletions(-) diff --git a/app/src/main/java/org/schabi/newpipe/download/DownloadDialog.java b/app/src/main/java/org/schabi/newpipe/download/DownloadDialog.java index 56ea9366d..146223bd0 100644 --- a/app/src/main/java/org/schabi/newpipe/download/DownloadDialog.java +++ b/app/src/main/java/org/schabi/newpipe/download/DownloadDialog.java @@ -773,7 +773,6 @@ public class DownloadDialog extends DialogFragment implements RadioGroup.OnCheck // more download logic: select muxer, subtitle converter, etc. switch (radioStreamsGroup.getCheckedRadioButtonId()) { case R.id.audio_button: - threads = 1;// use unique thread for subtitles due small file size kind = 'a'; selectedStream = audioStreamsAdapter.getItem(selectedAudioIndex); @@ -808,6 +807,7 @@ public class DownloadDialog extends DialogFragment implements RadioGroup.OnCheck } break; case R.id.subtitle_button: + threads = 1;// use unique thread for subtitles due small file size kind = 's'; selectedStream = subtitleStreamsAdapter.getItem(selectedSubtitleIndex); diff --git a/app/src/main/java/us/shandian/giga/get/DownloadInitializer.java b/app/src/main/java/us/shandian/giga/get/DownloadInitializer.java index 8f1b9ba4e..901f2b54c 100644 --- a/app/src/main/java/us/shandian/giga/get/DownloadInitializer.java +++ b/app/src/main/java/us/shandian/giga/get/DownloadInitializer.java @@ -28,11 +28,21 @@ public class DownloadInitializer extends Thread { mConn = null; } + private static void safeClose(HttpURLConnection con) { + try { + con.getInputStream().close(); + } catch (Exception e) { + // nothing to do + } + } + @Override public void run() { if (mMission.current > 0) mMission.resetState(false, true, DownloadMission.ERROR_NOTHING); int retryCount = 0; + int httpCode = 204; + while (true) { try { if (mMission.blocks == null && mMission.current == 0) { @@ -43,11 +53,16 @@ public class DownloadInitializer extends Thread { for (int i = 0; i < mMission.urls.length && mMission.running; i++) { mConn = mMission.openConnection(mMission.urls[i], mId, -1, -1); mMission.establishConnection(mId, mConn); + safeClose(mConn); if (Thread.interrupted()) return; long length = Utility.getContentLength(mConn); - if (i == 0) mMission.length = length; + if (i == 0) { + httpCode = mConn.getResponseCode(); + mMission.length = length; + } + if (length > 0) finalLength += length; if (length < lowestSize) lowestSize = length; } @@ -68,13 +83,15 @@ public class DownloadInitializer extends Thread { // ask for the current resource length mConn = mMission.openConnection(mId, -1, -1); mMission.establishConnection(mId, mConn); + safeClose(mConn); if (!mMission.running || Thread.interrupted()) return; + httpCode = mConn.getResponseCode(); mMission.length = Utility.getContentLength(mConn); } - if (mMission.length == 0 || mConn.getResponseCode() == 204) { + if (mMission.length == 0 || httpCode == 204) { mMission.notifyError(DownloadMission.ERROR_HTTP_NO_CONTENT, null); return; } @@ -92,6 +109,7 @@ public class DownloadInitializer extends Thread { // Open again mConn = mMission.openConnection(mId, mMission.length - 10, mMission.length); mMission.establishConnection(mId, mConn); + safeClose(mConn); if (!mMission.running || Thread.interrupted()) return; diff --git a/app/src/main/java/us/shandian/giga/get/DownloadRunnable.java b/app/src/main/java/us/shandian/giga/get/DownloadRunnable.java index 6d1d2bfbc..8126cc7e8 100644 --- a/app/src/main/java/us/shandian/giga/get/DownloadRunnable.java +++ b/app/src/main/java/us/shandian/giga/get/DownloadRunnable.java @@ -49,7 +49,6 @@ public class DownloadRunnable extends Thread { } SharpStream f; - InputStream is = null; try { f = mMission.storage.getStream(); @@ -114,16 +113,16 @@ public class DownloadRunnable extends Thread { f.seek(mMission.offsets[mMission.current] + start); - is = mConn.getInputStream(); + try (InputStream is = mConn.getInputStream()) { + byte[] buf = new byte[DownloadMission.BUFFER_SIZE]; + int len; - byte[] buf = new byte[DownloadMission.BUFFER_SIZE]; - int len; - - while (start < end && mMission.running && (len = is.read(buf, 0, buf.length)) != -1) { - f.write(buf, 0, len); - start += len; - block.done += len; - mMission.notifyProgress(len); + while (start < end && mMission.running && (len = is.read(buf, 0, buf.length)) != -1) { + f.write(buf, 0, len); + start += len; + block.done += len; + mMission.notifyProgress(len); + } } if (DEBUG && mMission.running) { @@ -143,12 +142,6 @@ public class DownloadRunnable extends Thread { } } - try { - if (is != null) is.close(); - } catch (Exception err) { - // nothing to do - } - try { f.close(); } catch (Exception err) { diff --git a/app/src/main/java/us/shandian/giga/get/DownloadRunnableFallback.java b/app/src/main/java/us/shandian/giga/get/DownloadRunnableFallback.java index d93053881..3ed57778e 100644 --- a/app/src/main/java/us/shandian/giga/get/DownloadRunnableFallback.java +++ b/app/src/main/java/us/shandian/giga/get/DownloadRunnableFallback.java @@ -94,7 +94,7 @@ public class DownloadRunnableFallback extends Thread { mMission.notifyProgress(len); } - // if thread goes interrupted check if the last part mIs written. This avoid re-download the whole file + // if thread goes interrupted check if the last part is written. This avoid re-download the whole file done = len == -1; } catch (Exception e) { dispose(); From 2f66913813372cb3b5e0d69854b50999d23d0352 Mon Sep 17 00:00:00 2001 From: kapodamy Date: Fri, 2 Aug 2019 01:07:37 -0300 Subject: [PATCH 2/2] drop unused popup storage permission request --- .../java/org/schabi/newpipe/MainActivity.java | 12 ------- .../org/schabi/newpipe/RouterActivity.java | 9 ++--- .../newpipe/download/DownloadDialog.java | 34 ++----------------- .../fragments/detail/VideoDetailFragment.java | 5 +-- .../settings/DownloadSettingsFragment.java | 2 -- .../newpipe/settings/NewPipeSettings.java | 7 ++-- .../schabi/newpipe/util/NavigationHelper.java | 3 -- .../schabi/newpipe/util/PermissionHelper.java | 3 -- .../giga/service/DownloadManagerService.java | 17 +++++----- app/src/main/res/values-es/strings.xml | 3 +- app/src/main/res/values/strings.xml | 4 +-- 11 files changed, 21 insertions(+), 78 deletions(-) diff --git a/app/src/main/java/org/schabi/newpipe/MainActivity.java b/app/src/main/java/org/schabi/newpipe/MainActivity.java index a9f2e9622..bf538cc65 100644 --- a/app/src/main/java/org/schabi/newpipe/MainActivity.java +++ b/app/src/main/java/org/schabi/newpipe/MainActivity.java @@ -60,7 +60,6 @@ import org.schabi.newpipe.report.ErrorActivity; import org.schabi.newpipe.util.Constants; import org.schabi.newpipe.util.KioskTranslator; import org.schabi.newpipe.util.NavigationHelper; -import org.schabi.newpipe.util.PermissionHelper; import org.schabi.newpipe.util.ServiceHelper; import org.schabi.newpipe.util.StateSaver; import org.schabi.newpipe.util.ThemeHelper; @@ -422,17 +421,6 @@ public class MainActivity extends AppCompatActivity { return; } } - switch (requestCode) { - case PermissionHelper.DOWNLOADS_REQUEST_CODE: - NavigationHelper.openDownloads(this); - break; - case PermissionHelper.DOWNLOAD_DIALOG_REQUEST_CODE: - Fragment fragment = getSupportFragmentManager().findFragmentById(R.id.fragment_holder); - if (fragment instanceof VideoDetailFragment) { - ((VideoDetailFragment) fragment).openDownloadDialog(); - } - break; - } } /** diff --git a/app/src/main/java/org/schabi/newpipe/RouterActivity.java b/app/src/main/java/org/schabi/newpipe/RouterActivity.java index c7bf4c881..88f6bdb2b 100644 --- a/app/src/main/java/org/schabi/newpipe/RouterActivity.java +++ b/app/src/main/java/org/schabi/newpipe/RouterActivity.java @@ -382,10 +382,8 @@ public class RouterActivity extends AppCompatActivity { } if (selectedChoiceKey.equals(getString(R.string.download_key))) { - if (PermissionHelper.checkStoragePermissions(this, PermissionHelper.DOWNLOAD_DIALOG_REQUEST_CODE)) { - selectionIsDownload = true; - openDownloadDialog(); - } + selectionIsDownload = true; + openDownloadDialog(); return; } @@ -453,9 +451,6 @@ public class RouterActivity extends AppCompatActivity { return; } } - if (requestCode == PermissionHelper.DOWNLOAD_DIALOG_REQUEST_CODE) { - openDownloadDialog(); - } } private static class AdapterChoiceItem { diff --git a/app/src/main/java/org/schabi/newpipe/download/DownloadDialog.java b/app/src/main/java/org/schabi/newpipe/download/DownloadDialog.java index 146223bd0..ea212e1e8 100644 --- a/app/src/main/java/org/schabi/newpipe/download/DownloadDialog.java +++ b/app/src/main/java/org/schabi/newpipe/download/DownloadDialog.java @@ -47,7 +47,6 @@ import org.schabi.newpipe.report.ErrorActivity; import org.schabi.newpipe.report.UserAction; import org.schabi.newpipe.util.FilenameUtils; import org.schabi.newpipe.util.ListHelper; -import org.schabi.newpipe.util.PermissionHelper; import org.schabi.newpipe.util.SecondaryStreamHelper; import org.schabi.newpipe.util.StreamItemAdapter; import org.schabi.newpipe.util.StreamItemAdapter.StreamSizeWrapper; @@ -173,10 +172,6 @@ public class DownloadDialog extends DialogFragment implements RadioGroup.OnCheck super.onCreate(savedInstanceState); if (DEBUG) Log.d(TAG, "onCreate() called with: savedInstanceState = [" + savedInstanceState + "]"); - if (!PermissionHelper.checkStoragePermissions(getActivity(), PermissionHelper.DOWNLOAD_DIALOG_REQUEST_CODE)) { - getDialog().dismiss(); - return; - } context = getContext(); @@ -217,32 +212,6 @@ public class DownloadDialog extends DialogFragment implements RadioGroup.OnCheck okButton.setEnabled(true); context.unbindService(this); - - // check of download paths are defined - if (!askForSavePath) { - String msg = ""; - if (mainStorageVideo == null) msg += getString(R.string.download_path_title); - if (mainStorageAudio == null) - msg += getString(R.string.download_path_audio_title); - - if (!msg.isEmpty()) { - String title; - if (mainStorageVideo == null && mainStorageAudio == null) { - title = getString(R.string.general_error); - msg = getString(R.string.no_available_dir) + ":\n" + msg; - } else { - title = msg; - msg = getString(R.string.no_available_dir); - } - - new AlertDialog.Builder(context) - .setPositiveButton(android.R.string.ok, null) - .setTitle(title) - .setMessage(msg) - .create() - .show(); - } - } } @Override @@ -602,6 +571,9 @@ public class DownloadDialog extends DialogFragment implements RadioGroup.OnCheck // * save path not defined (via download settings) // * the user as checked the "ask where to download" option + if (!askForSavePath) + Toast.makeText(context, getString(R.string.no_available_dir), Toast.LENGTH_LONG).show(); + StoredFileHelper.requestSafWithFileCreation(this, REQUEST_DOWNLOAD_PATH_SAF, filename, mime); return; } diff --git a/app/src/main/java/org/schabi/newpipe/fragments/detail/VideoDetailFragment.java b/app/src/main/java/org/schabi/newpipe/fragments/detail/VideoDetailFragment.java index f62cda2dd..5202c4b2a 100644 --- a/app/src/main/java/org/schabi/newpipe/fragments/detail/VideoDetailFragment.java +++ b/app/src/main/java/org/schabi/newpipe/fragments/detail/VideoDetailFragment.java @@ -385,10 +385,7 @@ public class VideoDetailFragment } break; case R.id.detail_controls_download: - if (PermissionHelper.checkStoragePermissions(activity, - PermissionHelper.DOWNLOAD_DIALOG_REQUEST_CODE)) { - this.openDownloadDialog(); - } + this.openDownloadDialog(); break; case R.id.detail_uploader_root_layout: if (TextUtils.isEmpty(currentInfo.getUploaderUrl())) { diff --git a/app/src/main/java/org/schabi/newpipe/settings/DownloadSettingsFragment.java b/app/src/main/java/org/schabi/newpipe/settings/DownloadSettingsFragment.java index e671e4d3a..4212f4cb8 100644 --- a/app/src/main/java/org/schabi/newpipe/settings/DownloadSettingsFragment.java +++ b/app/src/main/java/org/schabi/newpipe/settings/DownloadSettingsFragment.java @@ -12,7 +12,6 @@ import android.support.annotation.Nullable; import android.support.annotation.StringRes; import android.support.v7.preference.Preference; import android.util.Log; -import android.widget.Toast; import com.nononsenseapps.filepicker.Utils; @@ -64,7 +63,6 @@ public class DownloadSettingsFragment extends BasePreferenceFragment { } if (hasInvalidPath(DOWNLOAD_PATH_VIDEO_PREFERENCE) || hasInvalidPath(DOWNLOAD_PATH_AUDIO_PREFERENCE)) { - Toast.makeText(ctx, R.string.download_pick_path, Toast.LENGTH_SHORT).show(); updatePreferencesSummary(); } diff --git a/app/src/main/java/org/schabi/newpipe/settings/NewPipeSettings.java b/app/src/main/java/org/schabi/newpipe/settings/NewPipeSettings.java index f18d90a95..a0f3b6063 100644 --- a/app/src/main/java/org/schabi/newpipe/settings/NewPipeSettings.java +++ b/app/src/main/java/org/schabi/newpipe/settings/NewPipeSettings.java @@ -22,6 +22,7 @@ package org.schabi.newpipe.settings; import android.content.Context; import android.content.SharedPreferences; +import android.os.Build; import android.os.Environment; import android.preference.PreferenceManager; import android.support.annotation.NonNull; @@ -66,8 +67,10 @@ public class NewPipeSettings { PreferenceManager.setDefaultValues(context, R.xml.video_audio_settings, true); PreferenceManager.setDefaultValues(context, R.xml.debug_settings, true); - getVideoDownloadFolder(context); - getAudioDownloadFolder(context); + if (android.os.Build.VERSION.SDK_INT < Build.VERSION_CODES.LOLLIPOP) { + getVideoDownloadFolder(context); + getAudioDownloadFolder(context); + } } private static void getVideoDownloadFolder(Context context) { diff --git a/app/src/main/java/org/schabi/newpipe/util/NavigationHelper.java b/app/src/main/java/org/schabi/newpipe/util/NavigationHelper.java index 89c4b33fe..bb176166b 100644 --- a/app/src/main/java/org/schabi/newpipe/util/NavigationHelper.java +++ b/app/src/main/java/org/schabi/newpipe/util/NavigationHelper.java @@ -446,9 +446,6 @@ public class NavigationHelper { } public static boolean openDownloads(Activity activity) { - if (!PermissionHelper.checkStoragePermissions(activity, PermissionHelper.DOWNLOADS_REQUEST_CODE)) { - return false; - } Intent intent = new Intent(activity, DownloadActivity.class); activity.startActivity(intent); return true; diff --git a/app/src/main/java/org/schabi/newpipe/util/PermissionHelper.java b/app/src/main/java/org/schabi/newpipe/util/PermissionHelper.java index 7574a9304..1367b895c 100644 --- a/app/src/main/java/org/schabi/newpipe/util/PermissionHelper.java +++ b/app/src/main/java/org/schabi/newpipe/util/PermissionHelper.java @@ -18,9 +18,6 @@ import android.widget.Toast; import org.schabi.newpipe.R; public class PermissionHelper { - public static final int DOWNLOAD_DIALOG_REQUEST_CODE = 778; - public static final int DOWNLOADS_REQUEST_CODE = 777; - public static boolean checkStoragePermissions(Activity activity, int requestCode) { if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.JELLY_BEAN) { diff --git a/app/src/main/java/us/shandian/giga/service/DownloadManagerService.java b/app/src/main/java/us/shandian/giga/service/DownloadManagerService.java index 7f3a4bde1..065e21b9d 100755 --- a/app/src/main/java/us/shandian/giga/service/DownloadManagerService.java +++ b/app/src/main/java/us/shandian/giga/service/DownloadManagerService.java @@ -1,7 +1,5 @@ package us.shandian.giga.service; -import android.Manifest; -import android.app.AlertDialog; import android.app.Notification; import android.app.NotificationManager; import android.app.PendingIntent; @@ -30,7 +28,6 @@ import android.support.annotation.Nullable; import android.support.annotation.StringRes; import android.support.v4.app.NotificationCompat; import android.support.v4.app.NotificationCompat.Builder; -import android.support.v4.content.PermissionChecker; import android.util.Log; import android.util.SparseArray; import android.widget.Toast; @@ -257,18 +254,20 @@ public class DownloadManagerService extends Service { @Override public IBinder onBind(Intent intent) { + /* int permissionCheck; -// if (android.os.Build.VERSION.SDK_INT >= android.os.Build.VERSION_CODES.JELLY_BEAN) { -// permissionCheck = PermissionChecker.checkSelfPermission(this, Manifest.permission.READ_EXTERNAL_STORAGE); -// if (permissionCheck == PermissionChecker.PERMISSION_DENIED) { -// Toast.makeText(this, "Permission denied (read)", Toast.LENGTH_SHORT).show(); -// } -// } + if (android.os.Build.VERSION.SDK_INT >= android.os.Build.VERSION_CODES.JELLY_BEAN) { + permissionCheck = PermissionChecker.checkSelfPermission(this, Manifest.permission.READ_EXTERNAL_STORAGE); + if (permissionCheck == PermissionChecker.PERMISSION_DENIED) { + Toast.makeText(this, "Permission denied (read)", Toast.LENGTH_SHORT).show(); + } + } permissionCheck = PermissionChecker.checkSelfPermission(this, Manifest.permission.WRITE_EXTERNAL_STORAGE); if (permissionCheck == PermissionChecker.PERMISSION_DENIED) { Toast.makeText(this, "Permission denied (write)", Toast.LENGTH_SHORT).show(); } + */ return mBinder; } diff --git a/app/src/main/res/values-es/strings.xml b/app/src/main/res/values-es/strings.xml index 4eef99835..f61584639 100644 --- a/app/src/main/res/values-es/strings.xml +++ b/app/src/main/res/values-es/strings.xml @@ -103,7 +103,7 @@ Toque para ver detalles Por favor espere… Copiado al portapapeles - Por favor, seleccione un directorio de descarga disponible + Por favor, defina un directorio de descarga mas tarde en ajustes No se pudo cargar la imagen La interfaz de la app dejó de funcionar Lo sucedido:\\nPetición:\\nIdioma del contenido:\\nServicio:\\nHora GMT:\\nPaquete:\\nVersión:\\nVersión del SO: @@ -458,7 +458,6 @@ abrir en modo popup Se perdió el progreso porque el archivo fue eliminado Tiempo de espera excedido - Seleccione los directorios de descarga Preguntar dónde descargar Se preguntará dónde guardar cada descarga Se preguntará dónde guardar cada descarga.\nHabilita esta opción si quieres descargar en la tarjeta SD externa diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index 847f5fc49..916a31230 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -294,7 +294,7 @@ Tap for details Please wait… Copied to clipboard - Please select an available download folder + Please define an download folder later in settings This permission is needed to\nopen in popup mode 1 item deleted. @@ -553,8 +553,6 @@ Start downloads Pause downloads - Select the downloads save path - Ask where to download You will be asked where to save each download You will be asked where to save each download.\nEnable this option if you want download to the external SD Card