fix: correct getting content length method and add unit test

the length in Content-Range header is the whole length of the file instead of the current connection, so correct it by calculating the difference value between start range and end range, add unit tests for this method in the meantime
lingochamp · Mar 11, 2018 · d8f6cf1 · d8f6cf1
1 parent 2a5b81a
commit d8f6cf1
Show file tree

Hide file tree

Showing 3 changed files with 60 additions and 8 deletions.
diff --git a/library/src/main/java/com/liulishuo/filedownloader/download/DownloadLaunchRunnable.java b/library/src/main/java/com/liulishuo/filedownloader/download/DownloadLaunchRunnable.java
@@ -537,7 +537,7 @@ private void handleTrialConnectResult(Map<String, List<String>> requestHeader,
 
         redirectedUrl = connectTask.getFinalRedirectedUrl();
         if (acceptPartial || onlyFromBeginning) {
-            final long totalLength = FileDownloadUtils.findInstanceLengthForTrial(id, connection);
+            final long totalLength = FileDownloadUtils.findInstanceLengthForTrial(connection);
 
             // update model
             String fileName = null;

diff --git a/library/src/main/java/com/liulishuo/filedownloader/util/FileDownloadUtils.java b/library/src/main/java/com/liulishuo/filedownloader/util/FileDownloadUtils.java
@@ -555,9 +555,9 @@ public static boolean isAcceptRange(int responseCode, FileDownloadConnection con
     // because of we using one of two HEAD method to request or using range:0-0 to trial connection
     // only if connection api not support, so we test content-range first and then test
     // content-length.
-    public static long findInstanceLengthForTrial(int id, FileDownloadConnection connection) {
+    public static long findInstanceLengthForTrial(FileDownloadConnection connection) {
         long length = findInstanceLengthFromContentRange(connection);
-        if (length < 0) length = findContentLength(id, connection);
+        if (length < 0) length = findInstanceLengthFromContentLength(connection);
         if (length < 0) length = TOTAL_VALUE_IN_CHUNKED_RESOURCE;
         // the response of HEAD method is not very canonical sometimes(it depends on server
         // implementation)
@@ -571,14 +571,24 @@ public static long findInstanceLengthForTrial(int id, FileDownloadConnection con
     }
 
     public static long findInstanceLengthFromContentRange(FileDownloadConnection connection) {
-        return parseContentRangeFoInstanceLength(
-                connection.getResponseHeaderField("Content-Range"));
+        return parseContentRangeFoInstanceLength(getContentRangeHeader(connection));
+    }
+
+    private static String getContentRangeHeader(FileDownloadConnection connection) {
+        return  connection.getResponseHeaderField("Content-Range");
+    }
+
+    public static long findInstanceLengthFromContentLength(FileDownloadConnection connection) {
+        return convertContentLengthString(connection.getResponseHeaderField("Content-Length"));
     }
 
     public static long findContentLength(final int id, FileDownloadConnection connection) {
-        long contentLength = FileDownloadUtils
-                .convertContentLengthString(connection.getResponseHeaderField("Content-Length"));
-        if (contentLength < 0) contentLength = findInstanceLengthFromContentRange(connection);
+        long contentLength = findInstanceLengthFromContentLength(connection);
+        if (contentLength < 0) {
+            final String contentRange = getContentRangeHeader(connection);
+            contentLength = parseContentLengthFromContentRange(contentRange);
+        }
+
         final String transferEncoding = connection.getResponseHeaderField("Transfer-Encoding");
 
         if (contentLength < 0) {
@@ -610,6 +620,24 @@ public static long findContentLength(final int id, FileDownloadConnection connec
         return contentLength;
     }
 
+    public static long parseContentLengthFromContentRange(String contentRange) {
+        if (contentRange == null || contentRange.length() == 0) return -1;
+        final String pattern = "bytes (\\d+)-(\\d+)/\\d+";
+        try {
+            final Pattern r = Pattern.compile(pattern);
+            final Matcher m = r.matcher(contentRange);
+            if (m.find()) {
+                final long rangeStart = Long.parseLong(m.group(1));
+                final long rangeEnd = Long.parseLong(m.group(2));
+                return rangeEnd - rangeStart + 1;
+            }
+        }catch (Exception e) {
+            FileDownloadLog.e(FileDownloadUtils.class, e, "parse content length" +
+                    " from content range error");
+        }
+        return -1;
+    }
+
     public static String findFilename(FileDownloadConnection connection, String url) {
         String filename = FileDownloadUtils.parseContentDisposition(connection.
                 getResponseHeaderField("Content-Disposition"));

diff --git a/library/src/test/java/com/liulishuo/filedownloader/util/FileDownloadUtilsTest.java b/library/src/test/java/com/liulishuo/filedownloader/util/FileDownloadUtilsTest.java
@@ -35,4 +35,28 @@ public void parseContentDisposition() {
         assertThat(filename).isEqualTo("genome.jpeg");
     }
 
+    @Test
+    public void parseContentLengthFromContentRange_withNullContentRange() {
+        long length = FileDownloadUtils.parseContentLengthFromContentRange(null);
+        assertThat(length).isEqualTo(-1);
+    }
+
+    @Test
+    public void parseContentLengthFromContentRange_withEmptyContentRange() {
+        long length = FileDownloadUtils.parseContentLengthFromContentRange("");
+        assertThat(length).isEqualTo(-1);
+    }
+
+    @Test
+    public void parseContentLengthFromContentRange_withStartToEndRange() {
+        long length = FileDownloadUtils.parseContentLengthFromContentRange("bytes 25086300-37629450/37629451");
+        assertThat(length).isEqualTo(12543151);
+    }
+
+    @Test
+    public void parseContentLengthFromContentRange_withUnavailableContentRange() {
+        long length = FileDownloadUtils.parseContentLengthFromContentRange("bytes 0-/37629451");
+        assertThat(length).isEqualTo(-1);
+    }
+
 }