From bd6798d2af5df042e2c17f1d81683bf93faa17df Mon Sep 17 00:00:00 2001 From: ByteHamster Date: Fri, 1 Apr 2022 22:56:21 +0200 Subject: Move redirect handling to DownloadService - Only redirects when feed was parsed successfully (captive portals) - Decouple http client from database --- .../service/download/AntennapodHttpClient.java | 36 ---------------------- .../core/service/download/DownloadService.java | 3 ++ .../core/service/download/Downloader.java | 2 +- .../core/service/download/HttpDownloader.java | 22 +++++++++++++ 4 files changed, 26 insertions(+), 37 deletions(-) diff --git a/core/src/main/java/de/danoeh/antennapod/core/service/download/AntennapodHttpClient.java b/core/src/main/java/de/danoeh/antennapod/core/service/download/AntennapodHttpClient.java index b26c57963..b1138eeff 100644 --- a/core/src/main/java/de/danoeh/antennapod/core/service/download/AntennapodHttpClient.java +++ b/core/src/main/java/de/danoeh/antennapod/core/service/download/AntennapodHttpClient.java @@ -6,20 +6,14 @@ import androidx.annotation.NonNull; import de.danoeh.antennapod.core.preferences.UserPreferences; import de.danoeh.antennapod.core.service.BasicAuthorizationInterceptor; import de.danoeh.antennapod.core.service.UserAgentInterceptor; -import de.danoeh.antennapod.core.storage.DBWriter; import de.danoeh.antennapod.net.ssl.SslClientSetup; import okhttp3.Cache; import okhttp3.Credentials; -import okhttp3.HttpUrl; import okhttp3.JavaNetCookieJar; import okhttp3.OkHttpClient; -import okhttp3.Request; -import okhttp3.Response; -import okhttp3.internal.http.StatusLine; import java.io.File; import java.net.CookieManager; import java.net.CookiePolicy; -import java.net.HttpURLConnection; import java.net.InetSocketAddress; import java.net.Proxy; import java.net.SocketAddress; @@ -69,36 +63,6 @@ public class AntennapodHttpClient { System.setProperty("http.maxConnections", String.valueOf(MAX_CONNECTIONS)); OkHttpClient.Builder builder = new OkHttpClient.Builder(); - - // detect 301 Moved permanently and 308 Permanent Redirect - builder.networkInterceptors().add(chain -> { - Request request = chain.request(); - Response response = chain.proceed(request); - if (response.code() == HttpURLConnection.HTTP_MOVED_PERM - || response.code() == StatusLine.HTTP_PERM_REDIRECT) { - String location = response.header("Location"); - if (location == null) { - return response; - } - if (location.startsWith("/")) { // URL is not absolute, but relative - HttpUrl url = request.url(); - location = url.scheme() + "://" + url.host() + location; - } else if (!location.toLowerCase().startsWith("http://") - && !location.toLowerCase().startsWith("https://")) { - // Reference is relative to current path - HttpUrl url = request.url(); - String path = url.encodedPath(); - String newPath = path.substring(0, path.lastIndexOf("/") + 1) + location; - location = url.scheme() + "://" + url.host() + newPath; - } - try { - DBWriter.updateFeedDownloadURL(request.url().toString(), location).get(); - } catch (Exception e) { - Log.e(TAG, Log.getStackTraceString(e)); - } - } - return response; - }); builder.interceptors().add(new BasicAuthorizationInterceptor()); builder.networkInterceptors().add(new UserAgentInterceptor()); diff --git a/core/src/main/java/de/danoeh/antennapod/core/service/download/DownloadService.java b/core/src/main/java/de/danoeh/antennapod/core/service/download/DownloadService.java index 1522f42e2..218ccba9d 100644 --- a/core/src/main/java/de/danoeh/antennapod/core/service/download/DownloadService.java +++ b/core/src/main/java/de/danoeh/antennapod/core/service/download/DownloadService.java @@ -335,6 +335,9 @@ public class DownloadService extends Service { // Was stored in the database before and not initiated manually newEpisodesNotification.showIfNeeded(DownloadService.this, task.getSavedFeed()); } + if (downloader.permanentRedirectUrl != null) { + DBWriter.updateFeedDownloadURL(request.getSource(), downloader.permanentRedirectUrl); + } } else { DBWriter.setFeedLastUpdateFailed(request.getFeedfileId(), true); saveDownloadStatus(task.getDownloadStatus()); diff --git a/core/src/main/java/de/danoeh/antennapod/core/service/download/Downloader.java b/core/src/main/java/de/danoeh/antennapod/core/service/download/Downloader.java index fe44faac9..22c4e9b87 100644 --- a/core/src/main/java/de/danoeh/antennapod/core/service/download/Downloader.java +++ b/core/src/main/java/de/danoeh/antennapod/core/service/download/Downloader.java @@ -18,8 +18,8 @@ public abstract class Downloader implements Callable { private static final String TAG = "Downloader"; private volatile boolean finished; - public volatile boolean cancelled; + public String permanentRedirectUrl = null; @NonNull final DownloadRequest request; diff --git a/core/src/main/java/de/danoeh/antennapod/core/service/download/HttpDownloader.java b/core/src/main/java/de/danoeh/antennapod/core/service/download/HttpDownloader.java index 894428f44..7945239c7 100644 --- a/core/src/main/java/de/danoeh/antennapod/core/service/download/HttpDownloader.java +++ b/core/src/main/java/de/danoeh/antennapod/core/service/download/HttpDownloader.java @@ -7,6 +7,7 @@ import android.util.Log; import de.danoeh.antennapod.core.util.NetworkUtils; import de.danoeh.antennapod.model.download.DownloadStatus; import okhttp3.CacheControl; +import okhttp3.internal.http.StatusLine; import org.apache.commons.io.IOUtils; import java.io.BufferedInputStream; @@ -19,6 +20,7 @@ import java.net.HttpURLConnection; import java.net.SocketTimeoutException; import java.net.URI; import java.net.UnknownHostException; +import java.util.ArrayList; import java.util.Collections; import java.util.Date; @@ -173,6 +175,7 @@ public class HttpDownloader extends Downloader { return; } } + checkIfRedirect(response); connection = new BufferedInputStream(responseBody.byteStream()); @@ -279,6 +282,25 @@ public class HttpDownloader extends Downloader { } } + private void checkIfRedirect(Response response) { + // detect 301 Moved permanently and 308 Permanent Redirect + ArrayList responses = new ArrayList<>(); + while (response != null) { + responses.add(response); + response = response.priorResponse(); + } + if (responses.isEmpty()) { + return; + } + Collections.reverse(responses); + int firstCode = responses.get(0).code(); + if (firstCode == HttpURLConnection.HTTP_MOVED_PERM || firstCode == StatusLine.HTTP_PERM_REDIRECT) { + String secondUrl = responses.get(1).request().url().toString(); + Log.d(TAG, "Detected permanent redirect from " + request.getSource() + " to " + secondUrl); + permanentRedirectUrl = secondUrl; + } + } + private void onSuccess() { Log.d(TAG, "Download was successful"); result.setSuccessful(); -- cgit v1.2.3 From b53cec6795ed34f2fd225d74dbc10973e16abe94 Mon Sep 17 00:00:00 2001 From: ByteHamster Date: Fri, 1 Apr 2022 23:16:17 +0200 Subject: Rework HttpDownloader for readability --- .../core/service/download/HttpDownloader.java | 138 ++++++++++----------- 1 file changed, 66 insertions(+), 72 deletions(-) diff --git a/core/src/main/java/de/danoeh/antennapod/core/service/download/HttpDownloader.java b/core/src/main/java/de/danoeh/antennapod/core/service/download/HttpDownloader.java index 7945239c7..01c135061 100644 --- a/core/src/main/java/de/danoeh/antennapod/core/service/download/HttpDownloader.java +++ b/core/src/main/java/de/danoeh/antennapod/core/service/download/HttpDownloader.java @@ -39,7 +39,6 @@ import okio.ByteString; public class HttpDownloader extends Downloader { private static final String TAG = "HttpDownloader"; - private static final int BUFFER_SIZE = 8 * 1024; public HttpDownloader(@NonNull DownloadRequest request) { @@ -57,7 +56,6 @@ public class HttpDownloader extends Downloader { return; } - OkHttpClient httpClient = AntennapodHttpClient.getHttpClient(); RandomAccessFile out = null; InputStream connection; ResponseBody responseBody = null; @@ -90,7 +88,6 @@ public class HttpDownloader extends Downloader { } } - // add range header if necessary if (fileExists && destination.length() > 0) { request.setSoFar(destination.length()); @@ -98,22 +95,7 @@ public class HttpDownloader extends Downloader { Log.d(TAG, "Adding range header: " + request.getSoFar()); } - Response response; - - try { - response = httpClient.newCall(httpReq.build()).execute(); - } catch (IOException e) { - Log.e(TAG, e.toString()); - if (e.getMessage().contains("PROTOCOL_ERROR")) { - httpClient = httpClient.newBuilder() - .protocols(Collections.singletonList(Protocol.HTTP_1_1)) - .build(); - response = httpClient.newCall(httpReq.build()).execute(); - } else { - throw e; - } - } - + Response response = newCall(httpReq); responseBody = response.body(); String contentEncodingHeader = response.header("Content-Encoding"); boolean isGzip = false; @@ -122,65 +104,26 @@ public class HttpDownloader extends Downloader { } Log.d(TAG, "Response code is " + response.code()); - if (!response.isSuccessful() && response.code() == HttpURLConnection.HTTP_NOT_MODIFIED) { Log.d(TAG, "Feed '" + request.getSource() + "' not modified since last update, Download canceled"); onCancelled(); return; - } - - if (!response.isSuccessful() || response.body() == null) { - final DownloadError error; - final String details; - if (response.code() == HttpURLConnection.HTTP_UNAUTHORIZED) { - error = DownloadError.ERROR_UNAUTHORIZED; - details = String.valueOf(response.code()); - } else if (response.code() == HttpURLConnection.HTTP_FORBIDDEN) { - error = DownloadError.ERROR_FORBIDDEN; - details = String.valueOf(response.code()); - } else if (response.code() == HttpURLConnection.HTTP_NOT_FOUND) { - error = DownloadError.ERROR_NOT_FOUND; - details = String.valueOf(response.code()); - } else { - error = DownloadError.ERROR_HTTP_DATA_ERROR; - details = String.valueOf(response.code()); - } - onFail(error, details); + } else if (!response.isSuccessful() || response.body() == null) { + callOnFailByResponseCode(response); return; - } - - if (!StorageUtils.storageAvailable()) { + } else if (!StorageUtils.storageAvailable()) { onFail(DownloadError.ERROR_DEVICE_NOT_FOUND, null); return; - } - - // fail with a file type error when the content type is text and - // the reported content length is less than 100kb (or no length is given) - if (request.getFeedfileType() == FeedMedia.FEEDFILETYPE_FEEDMEDIA) { - int contentLength = -1; - String contentLen = response.header("Content-Length"); - if (contentLen != null) { - try { - contentLength = Integer.parseInt(contentLen); - } catch (NumberFormatException e) { - e.printStackTrace(); - } - } - Log.d(TAG, "content length: " + contentLength); - String contentType = response.header("Content-Type"); - Log.d(TAG, "content type: " + contentType); - if (contentType != null && contentType.startsWith("text/") && - contentLength < 100 * 1024) { - onFail(DownloadError.ERROR_FILE_TYPE, null); - return; - } + } else if (request.getFeedfileType() == FeedMedia.FEEDFILETYPE_FEEDMEDIA + && isContentTypeTextAndSmallerThan100kb(response)) { + onFail(DownloadError.ERROR_FILE_TYPE, null); + return; } checkIfRedirect(response); connection = new BufferedInputStream(responseBody.byteStream()); String contentRangeHeader = (fileExists) ? response.header("Content-Range") : null; - if (fileExists && response.code() == HttpURLConnection.HTTP_PARTIAL && !TextUtils.isEmpty(contentRangeHeader)) { String start = contentRangeHeader.substring("bytes ".length(), @@ -211,7 +154,6 @@ public class HttpDownloader extends Downloader { long freeSpace = StorageUtils.getFreeSpaceAvailable(); Log.d(TAG, "Free space is " + freeSpace); - if (request.getSize() != DownloadStatus.SIZE_UNKNOWN && request.getSize() > freeSpace) { onFail(DownloadError.ERROR_NOT_ENOUGH_SPACE, null); return; @@ -233,10 +175,10 @@ public class HttpDownloader extends Downloader { } else { // check if size specified in the response header is the same as the size of the // written file. This check cannot be made if compression was used - if (!isGzip && request.getSize() != DownloadStatus.SIZE_UNKNOWN && - request.getSoFar() != request.getSize()) { - onFail(DownloadError.ERROR_IO_WRONG_SIZE, "Download completed but size: " + - request.getSoFar() + " does not equal expected size " + request.getSize()); + if (!isGzip && request.getSize() != DownloadStatus.SIZE_UNKNOWN + && request.getSoFar() != request.getSize()) { + onFail(DownloadError.ERROR_IO_WRONG_SIZE, "Download completed but size: " + + request.getSoFar() + " does not equal expected size " + request.getSize()); return; } else if (request.getSize() > 0 && request.getSoFar() == 0) { onFail(DownloadError.ERROR_IO_ERROR, "Download completed, but nothing was read"); @@ -282,6 +224,59 @@ public class HttpDownloader extends Downloader { } } + private Response newCall(Request.Builder httpReq) throws IOException { + OkHttpClient httpClient = AntennapodHttpClient.getHttpClient(); + try { + return httpClient.newCall(httpReq.build()).execute(); + } catch (IOException e) { + Log.e(TAG, e.toString()); + if (e.getMessage() != null && e.getMessage().contains("PROTOCOL_ERROR")) { + // Apparently some servers announce they support SPDY but then actually don't. + httpClient = httpClient.newBuilder() + .protocols(Collections.singletonList(Protocol.HTTP_1_1)) + .build(); + return httpClient.newCall(httpReq.build()).execute(); + } else { + throw e; + } + } + } + + private boolean isContentTypeTextAndSmallerThan100kb(Response response) { + int contentLength = -1; + String contentLen = response.header("Content-Length"); + if (contentLen != null) { + try { + contentLength = Integer.parseInt(contentLen); + } catch (NumberFormatException e) { + e.printStackTrace(); + } + } + Log.d(TAG, "content length: " + contentLength); + String contentType = response.header("Content-Type"); + Log.d(TAG, "content type: " + contentType); + return contentType != null && contentType.startsWith("text/") && contentLength < 100 * 1024; + } + + private void callOnFailByResponseCode(Response response) { + final DownloadError error; + final String details; + if (response.code() == HttpURLConnection.HTTP_UNAUTHORIZED) { + error = DownloadError.ERROR_UNAUTHORIZED; + details = String.valueOf(response.code()); + } else if (response.code() == HttpURLConnection.HTTP_FORBIDDEN) { + error = DownloadError.ERROR_FORBIDDEN; + details = String.valueOf(response.code()); + } else if (response.code() == HttpURLConnection.HTTP_NOT_FOUND) { + error = DownloadError.ERROR_NOT_FOUND; + details = String.valueOf(response.code()); + } else { + error = DownloadError.ERROR_HTTP_DATA_ERROR; + details = String.valueOf(response.code()); + } + onFail(error, details); + } + private void checkIfRedirect(Response response) { // detect 301 Moved permanently and 308 Permanent Redirect ArrayList responses = new ArrayList<>(); @@ -307,8 +302,7 @@ public class HttpDownloader extends Downloader { } private void onFail(DownloadError reason, String reasonDetailed) { - Log.d(TAG, "onFail() called with: " + "reason = [" + reason + "], " + - "reasonDetailed = [" + reasonDetailed + "]"); + Log.d(TAG, "onFail() called with: " + "reason = [" + reason + "], reasonDetailed = [" + reasonDetailed + "]"); result.setFailed(reason, reasonDetailed); if (request.isDeleteOnFailure()) { cleanup(); -- cgit v1.2.3