Merge pull request #5826 from ByteHamster/move-redirect-handling

Rewrite feed redirect handling

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());
 

core/src/main/java/de/danoeh/antennapod/core/service/download/DownloadService.java

@@ -339,6 +339,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());

core/src/main/java/de/danoeh/antennapod/core/service/download/Downloader.java

@@ -18,8 +18,8 @@ public abstract class Downloader implements Callable<Downloader> {
     private static final String TAG = "Downloader";
 
     private volatile boolean finished;
-
     public volatile boolean cancelled;
+    public String permanentRedirectUrl = null;
 
     @NonNull
     final DownloadRequest request;

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;
 
@@ -37,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) {
@@ -55,7 +56,6 @@ public class HttpDownloader extends Downloader {
             return;
         }
 
-        OkHttpClient httpClient = AntennapodHttpClient.getHttpClient();
         RandomAccessFile out = null;
         InputStream connection;
         ResponseBody responseBody = null;
@@ -88,7 +88,6 @@ public class HttpDownloader extends Downloader {
                 }
             }
 
-
             // add range header if necessary
             if (fileExists && destination.length() > 0) {
                 request.setSoFar(destination.length());
@@ -96,22 +95,7 @@ public class HttpDownloader extends Downloader {
                 Log.d(TAG, "Adding range header: " + request.getSoFar());
             }
 
-            Response response;
+            Response response = newCall(httpReq);
-
-            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;
-                }
-            }
-
             responseBody = response.body();
             String contentEncodingHeader = response.header("Content-Encoding");
             boolean isGzip = false;
@@ -120,64 +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;
-            }
+            } else if (!response.isSuccessful() || response.body() == null) {
-
+                callOnFailByResponseCode(response);
-            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);
                 return;
-            }
+            } else if (!StorageUtils.storageAvailable()) {
-
-            if (!StorageUtils.storageAvailable()) {
                 onFail(DownloadError.ERROR_DEVICE_NOT_FOUND, null);
                 return;
+            } else if (request.getFeedfileType() == FeedMedia.FEEDFILETYPE_FEEDMEDIA
+                    && isContentTypeTextAndSmallerThan100kb(response)) {
+                onFail(DownloadError.ERROR_FILE_TYPE, null);
+                return;
             }
-
+            checkIfRedirect(response);
-            // 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;
-                }
-            }
 
             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(),
@@ -208,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;
@@ -230,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 &&
+                if (!isGzip && request.getSize() != DownloadStatus.SIZE_UNKNOWN
-                        request.getSoFar() != request.getSize()) {
+                        && request.getSoFar() != request.getSize()) {
-                    onFail(DownloadError.ERROR_IO_WRONG_SIZE, "Download completed but size: " +
+                    onFail(DownloadError.ERROR_IO_WRONG_SIZE, "Download completed but size: "
-                            request.getSoFar() + " does not equal expected size " + request.getSize());
+                            + 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");
@@ -279,14 +224,85 @@ 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<Response> 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();
     }
 
     private void onFail(DownloadError reason, String reasonDetailed) {
-        Log.d(TAG, "onFail() called with: " + "reason = [" + reason + "], " +
+        Log.d(TAG, "onFail() called with: " + "reason = [" + reason + "], reasonDetailed = [" + reasonDetailed + "]");
-                "reasonDetailed = [" + reasonDetailed + "]");
         result.setFailed(reason, reasonDetailed);
         if (request.isDeleteOnFailure()) {
             cleanup();