From f23d38760981e17849da406df45d634a04144f8f Mon Sep 17 00:00:00 2001
From: Henry Le Grys <henry@legrys.info>
Date: Wed, 31 Mar 2021 22:22:00 +0100
Subject: [PATCH] Fix bad logic surrounding download resuming

An oversight in the implementation of partial download resumption led
to cross-contamination in the presence of multiple request URLs, where
an `Accept-Ranges` header from one server would cause a `Range` header
to be supplied to the next. Compounded with a bad ternary expression
in `HttpRequest` which couldn't handle non-200 success codes, this
was causing cryptic "Stream closed" errors on library downloads.
---
 .../launcher/install/HttpDownloader.java      | 31 +++++++++++++------
 .../skcraft/launcher/util/HttpRequest.java    | 13 ++++++--
 2 files changed, 33 insertions(+), 11 deletions(-)

diff --git a/launcher/src/main/java/com/skcraft/launcher/install/HttpDownloader.java b/launcher/src/main/java/com/skcraft/launcher/install/HttpDownloader.java
index 9832169..6c5aeea 100644
--- a/launcher/src/main/java/com/skcraft/launcher/install/HttpDownloader.java
+++ b/launcher/src/main/java/com/skcraft/launcher/install/HttpDownloader.java
@@ -238,7 +238,6 @@ public class HttpDownloader implements Downloader {
             int trial = 0;
             boolean first = true;
             IOException lastException = null;
-            HttpRequest.PartialDownloadInfo retryDetails = null;
 
             do {
                 for (URL url : urls) {
@@ -249,17 +248,10 @@ public class HttpDownloader implements Downloader {
                     first = false;
 
                     try {
-                        request = HttpRequest.get(url);
-                        request.setResumeInfo(retryDetails).execute().expectResponseCode(200).saveContent(file);
+                        tryDownloadFrom(url, file, null, 0);
                         return;
                     } catch (IOException e) {
                         lastException = e;
-                        log.log(Level.WARNING, "Failed to download " + url, e);
-
-                        Optional<HttpRequest.PartialDownloadInfo> byteRangeSupport = request.canRetryPartial();
-                        if (byteRangeSupport.isPresent()) {
-                            retryDetails = byteRangeSupport.get();
-                        }
                     }
                 }
             } while (++trial < tryCount);
@@ -267,6 +259,27 @@ public class HttpDownloader implements Downloader {
             throw new IOException("Failed to download from " + urls, lastException);
         }
 
+        private void tryDownloadFrom(URL url, File file, HttpRequest.PartialDownloadInfo retryDetails, int tries)
+                throws InterruptedException, IOException {
+            try {
+                request = HttpRequest.get(url);
+                request.setResumeInfo(retryDetails).execute().expectResponseCode(200).saveContent(file);
+            } catch (IOException e) {
+                log.log(Level.WARNING, "Failed to download " + url, e);
+
+                // We only want to try to resume a partial download if the request succeeded before
+                // throwing an exception halfway through. If it didn't succeed, just throw the error.
+                if (tries >= tryCount || !request.isSuccessCode()) {
+                    throw e;
+                }
+
+                Optional<HttpRequest.PartialDownloadInfo> byteRangeSupport = request.canRetryPartial();
+                if (byteRangeSupport.isPresent()) {
+                    tryDownloadFrom(url, file, byteRangeSupport.get(), tries + 1);
+                }
+            }
+        }
+
         @Override
         public double getProgress() {
             HttpRequest request = this.request;
diff --git a/launcher/src/main/java/com/skcraft/launcher/util/HttpRequest.java b/launcher/src/main/java/com/skcraft/launcher/util/HttpRequest.java
index d9da68a..3f05cd3 100644
--- a/launcher/src/main/java/com/skcraft/launcher/util/HttpRequest.java
+++ b/launcher/src/main/java/com/skcraft/launcher/util/HttpRequest.java
@@ -117,8 +117,7 @@ public class HttpRequest implements Closeable, ProgressObservable {
 
             conn = this.runRequest(url);
 
-            inputStream = conn.getResponseCode() == HttpURLConnection.HTTP_OK ?
-                    conn.getInputStream() : conn.getErrorStream();
+            inputStream = isSuccessCode() ? conn.getInputStream() : conn.getErrorStream();
 
             successful = true;
         } finally {
@@ -247,6 +246,16 @@ public class HttpRequest implements Closeable, ProgressObservable {
         return conn.getResponseCode();
     }
 
+    /**
+     * Check if the response code indicates a successful request.
+     * @return True if response code is 2xx, false otherwise.
+     * @throws IOException on I/O error getting the response code.
+     */
+    public boolean isSuccessCode() throws IOException {
+        int code = getResponseCode();
+        return code >= 200 && code < 300;
+    }
+
     /**
      * Get the input stream.
      *