Fix broken RAR seek support.

Using SEEK_CUR or SEEK_END produced the wrong results, SEEK_SET was
fine however. Note that archive_read_data() doesn't function exactly
as POSIX read. Currently, downstreams will have to check the current
file position after after call to archive_read_data() if they are
to use SEEK_CUR afterwards.

diff --git a/libarchive/archive_read_support_format_rar.c b/libarchive/archive_read_support_format_rar.c
index 7492a50bab6796964fe0837d9547bb0d3fb0c090..1bd95a3956b9d3baf9279eacc8e327b88d01ee20 100644 (file)
--- a/libarchive/archive_read_support_format_rar.c
+++ b/libarchive/archive_read_support_format_rar.c
@@ -201,6 +201,7 @@ struct lzss
 
 struct data_block_offsets
 {
+  int64_t header_size;
   int64_t start_offset;
   int64_t end_offset;
 };
@@ -241,6 +242,7 @@ struct rar
   int64_t bytes_uncopied;
   int64_t offset;
   int64_t offset_outgoing;
+  int64_t offset_seek;
   char valid;
   unsigned int unp_offset;
   unsigned int unp_buffer_size;
@@ -1025,25 +1027,49 @@ archive_read_format_rar_seek_data(struct archive_read *a, int64_t offset,
     switch (whence)
     {
       case SEEK_SET:
+        client_offset = 0;
         break;
-
       case SEEK_CUR:
-        offset += rar->offset;
+        client_offset = rar->offset_seek;
         break;
-
       case SEEK_END:
-        offset += rar->unp_size - 1;
-        break;
+        client_offset = rar->unp_size;
+    }
+    client_offset += offset;
+    if (client_offset < 0)
+    {
+      /* Can't seek past beginning of data block */
+      return -1;
+    }
+    else if (client_offset > rar->unp_size)
+    {
+      /*
+       * Set the returned offset but only seek to the end of
+       * the data block.
+       */
+      rar->offset_seek = client_offset;
+      client_offset = rar->unp_size;
     }
-    if (offset < 0)
-      offset = 0;
-    else if (offset > rar->unp_size - 1)
-      offset = rar->unp_size - 1;
 
     /* Find the appropriate offset in the client */
     i = 0;
-    client_offset = offset;
     client_offset += rar->dbo[i].start_offset;
+    if (rar->main_flags & MHD_VOLUME &&
+      client_offset < rar->dbo[rar->cursor].start_offset)
+    {
+      /* The header of the first multivolume file must be re-read */
+      ret = __archive_read_seek(a,
+        rar->dbo[i].start_offset - rar->dbo[i].header_size, SEEK_SET);
+      if (ret < (ARCHIVE_OK))
+        return ret;
+      ret = archive_read_format_rar_read_header(a, a->entry);
+      if (ret != (ARCHIVE_OK))
+      {
+        archive_set_error(&a->archive, ARCHIVE_ERRNO_MISC,
+          "Error during seek of RAR file");
+        return (ARCHIVE_FAILED);
+      }
+    }
     while (rar->main_flags & MHD_VOLUME &&
       client_offset > rar->dbo[i].end_offset)
     {
@@ -1088,7 +1114,16 @@ archive_read_format_rar_seek_data(struct archive_read *a, int64_t offset,
     rar->bytes_unconsumed = 0;
     rar->offset = 0;
 
-    return ret;
+    /*
+     * If a seek past the end of file was requested, return the requested
+     * offset.
+     */
+    if (ret == rar->unp_size && rar->offset_seek > rar->unp_size)
+      return rar->offset_seek;
+
+    /* Return the new offset */
+    rar->offset_seek = ret;
+    return rar->offset_seek;
   }
   else
   {
@@ -1409,6 +1444,7 @@ read_header(struct archive_read *a, struct archive_entry *entry,
         archive_set_error(&a->archive, ENOMEM, "Couldn't allocate memory.");
         return (ARCHIVE_FATAL);
       }
+      rar->dbo[rar->cursor].header_size = header_size;
       rar->dbo[rar->cursor].start_offset = -1;
       rar->dbo[rar->cursor].end_offset = -1;
     }
@@ -1432,6 +1468,7 @@ read_header(struct archive_read *a, struct archive_entry *entry,
     archive_set_error(&a->archive, ENOMEM, "Couldn't allocate memory.");
     return (ARCHIVE_FATAL);
   }
+  rar->dbo[0].header_size = header_size;
   rar->dbo[0].start_offset = -1;
   rar->dbo[0].end_offset = -1;
   rar->cursor = 0;
@@ -1487,6 +1524,7 @@ read_header(struct archive_read *a, struct archive_entry *entry,
 
   rar->bytes_uncopied = rar->bytes_unconsumed = 0;
   rar->lzss.position = rar->offset = 0;
+  rar->offset_seek = 0;
   rar->dictionary_size = 0;
   rar->offset_outgoing = 0;
   rar->br.cache_avail = 0;
@@ -1698,6 +1736,7 @@ read_data_stored(struct archive_read *a, const void **buff, size_t *size,
   *size = bytes_avail;
   *offset = rar->offset;
   rar->offset += bytes_avail;
+  rar->offset_seek += bytes_avail;
   rar->bytes_remaining -= bytes_avail;
   rar->bytes_unconsumed = bytes_avail;
   /* Calculate File CRC. */

diff --git a/libarchive/test/test_read_format_rar.c b/libarchive/test/test_read_format_rar.c
index 4317a1adcf25d48703833ec8e23618048f50538a..0cc7d08ac5cc6a015045fbdc8ec86bbccdd7a3b0 100644 (file)
--- a/libarchive/test/test_read_format_rar.c
+++ b/libarchive/test/test_read_format_rar.c
@@ -1166,6 +1166,7 @@ DEFINE_TEST(test_read_format_rar_multivolume_seek_data)
   };
   char buff[64];
   int file_size = 20111;
+  int64_t result;
   const char file_test_txt1[] = "d. \n</P>\n<P STYLE=\"margin-bottom: 0in\">"
                                 "<BR>\n</P>\n</BODY>\n</HTML>";
   const char file_test_txt2[] = "<!DOCTYPE HTML PUBLIC \"-//W3C//DTD HTML 4."
@@ -1220,6 +1221,55 @@ DEFINE_TEST(test_read_format_rar_multivolume_seek_data)
   assertA(sizeof(buff) == archive_read_data(a, buff, sizeof(buff)));
   assertEqualMem(buff, file_test_txt5, sizeof(file_test_txt5) - 1);
 
+  /* Use various combinations of SEEK_SET, SEEK_CUR, and SEEK_END */
+  assertEqualInt(file_size, archive_seek_data(a, 0, SEEK_END));
+  assertEqualInt(0, archive_seek_data(a, 0, SEEK_SET));
+  assertEqualInt(0, archive_seek_data(a, 0, SEEK_CUR));
+  assertEqualInt(-1, archive_seek_data(a, -10, SEEK_CUR));
+  assertEqualInt(10, archive_seek_data(a, 10, SEEK_CUR));
+  assertEqualInt(-1, archive_seek_data(a, -20, SEEK_CUR));
+  assertEqualInt(10, archive_seek_data(a, 0, SEEK_CUR));
+  assertEqualInt(file_size, archive_seek_data(a, 0, SEEK_END));
+  assertEqualInt(file_size - 20, archive_seek_data(a, -20, SEEK_END));
+  assertEqualInt(file_size + 40, archive_seek_data(a, 40, SEEK_END));
+  assertEqualInt(file_size + 40, archive_seek_data(a, 0, SEEK_CUR));
+  assertEqualInt(file_size + 40 + 20, archive_seek_data(a, 20, SEEK_CUR));
+  assertEqualInt(file_size + 40 + 20 + 20, archive_seek_data(a, 20, SEEK_CUR));
+  assertEqualInt(file_size + 20, archive_seek_data(a, 20, SEEK_END));
+  assertEqualInt(file_size - 20, archive_seek_data(a, -20, SEEK_END));
+
+  /* Seek to the end minus 64 bytes */
+  assertA(0 == archive_seek_data(a, 0, SEEK_SET));
+  assertA(file_size - (int)sizeof(buff) ==
+    archive_seek_data(a, -sizeof(buff), SEEK_END));
+  assertA(sizeof(buff) == archive_read_data(a, buff, sizeof(buff)));
+  assertEqualMem(buff, file_test_txt1, sizeof(file_test_txt1) - 1);
+
+  /* The file position should be at the end of the file here */
+  assertA(file_size == archive_seek_data(a, 0, SEEK_CUR));
+
+  /* Seek back to the beginning */
+  assertA(0 == archive_seek_data(a, -file_size, SEEK_CUR));
+  assertA(sizeof(buff) == archive_read_data(a, buff, sizeof(buff)));
+  assertEqualMem(buff, file_test_txt2, sizeof(file_test_txt2) - 1);
+
+  /* Seek to the middle of the combined data block */
+  result = archive_seek_data(a, 0, SEEK_CUR);
+  assertA(10054 == archive_seek_data(a, 10054 - result, SEEK_CUR));
+  assertA(sizeof(buff) == archive_read_data(a, buff, sizeof(buff)));
+  assertEqualMem(buff, file_test_txt3, sizeof(file_test_txt3) - 1);
+
+  /* Seek to 32 bytes before the end of the first data sub-block */
+  result = archive_seek_data(a, 0, SEEK_CUR);
+  assertA(6860 == archive_seek_data(a, 6860 - result, SEEK_CUR));
+  assertA(sizeof(buff) == archive_read_data(a, buff, sizeof(buff)));
+  assertEqualMem(buff, file_test_txt4, sizeof(file_test_txt4) - 1);
+
+  /* Seek to 32 bytes before the end of the second data sub-block */
+  assertA(13752 == archive_seek_data(a, 13752 - file_size, SEEK_END));
+  assertA(sizeof(buff) == archive_read_data(a, buff, sizeof(buff)));
+  assertEqualMem(buff, file_test_txt5, sizeof(file_test_txt5) - 1);
+
   /* Test EOF */
   assertA(1 == archive_read_next_header(a, &ae));
   assertEqualInt(1, archive_file_count(a));
@@ -1241,6 +1291,7 @@ DEFINE_TEST(test_read_format_rar_multivolume_seek_multiple_files)
   };
   char buff[64];
   int file_size = 20111;
+  int64_t result;
   const char file_test_txt1[] = "d. \n</P>\n<P STYLE=\"margin-bottom: 0in\">"
                                 "<BR>\n</P>\n</BODY>\n</HTML>";
   const char file_test_txt2[] = "<!DOCTYPE HTML PUBLIC \"-//W3C//DTD HTML 4."
@@ -1277,27 +1328,29 @@ DEFINE_TEST(test_read_format_rar_multivolume_seek_multiple_files)
 
   /* Seek to the end minus 64 bytes */
   assertA(file_size - (int)sizeof(buff) ==
-    archive_seek_data(a, file_size - sizeof(buff), SEEK_SET));
+    archive_seek_data(a, -sizeof(buff), SEEK_END));
   assertA(sizeof(buff) == archive_read_data(a, buff, sizeof(buff)));
   assertEqualMem(buff, file_test_txt1, sizeof(file_test_txt1) - 1);
 
   /* Seek back to the beginning */
-  assertA(0 == archive_seek_data(a, 0, SEEK_SET));
+  assertA(0 == archive_seek_data(a, -file_size, SEEK_END));
   assertA(sizeof(buff) == archive_read_data(a, buff, sizeof(buff)));
   assertEqualMem(buff, file_test_txt2, sizeof(file_test_txt2) - 1);
 
   /* Seek to the middle of the combined data block */
-  assertA(10054 == archive_seek_data(a, 10054, SEEK_SET));
+  result = archive_seek_data(a, 0, SEEK_CUR);
+  assertA(10054 == archive_seek_data(a, 10054 - result, SEEK_CUR));
   assertA(sizeof(buff) == archive_read_data(a, buff, sizeof(buff)));
   assertEqualMem(buff, file_test_txt3, sizeof(file_test_txt3) - 1);
 
   /* Seek to 32 bytes before the end of the first data sub-block */
-  assertA(7027 == archive_seek_data(a, 7027, SEEK_SET));
+  result = archive_seek_data(a, 0, SEEK_CUR);
+  assertA(7027 == archive_seek_data(a, 7027 - result, SEEK_CUR));
   assertA(sizeof(buff) == archive_read_data(a, buff, sizeof(buff)));
   assertEqualMem(buff, file_test_txt4, sizeof(file_test_txt4) - 1);
 
   /* Seek to 32 bytes before the end of the second data sub-block */
-  assertA(14086 == archive_seek_data(a, 14086, SEEK_SET));
+  assertA(14086 == archive_seek_data(a, 14086 - file_size, SEEK_END));
   assertA(sizeof(buff) == archive_read_data(a, buff, sizeof(buff)));
   assertEqualMem(buff, file_test_txt5, sizeof(file_test_txt5) - 1);
 
@@ -1322,22 +1375,24 @@ DEFINE_TEST(test_read_format_rar_multivolume_seek_multiple_files)
   assertEqualMem(buff, file_test_txt2, sizeof(file_test_txt2) - 1);
 
   /* Seek to the middle of the combined data block */
-  assertA(10054 == archive_seek_data(a, 10054, SEEK_SET));
+  result = archive_seek_data(a, 0, SEEK_CUR);
+  assertA(10054 == archive_seek_data(a, 10054 - result, SEEK_CUR));
   assertA(sizeof(buff) == archive_read_data(a, buff, sizeof(buff)));
   assertEqualMem(buff, file_test_txt3, sizeof(file_test_txt3) - 1);
 
   /* Seek to 32 bytes before the end of the first data sub-block */
-  assertA(969 == archive_seek_data(a, 969, SEEK_SET));
+  result = archive_seek_data(a, 0, SEEK_CUR);
+  assertA(969 == archive_seek_data(a, 969 - result, SEEK_CUR));
   assertA(sizeof(buff) == archive_read_data(a, buff, sizeof(buff)));
   assertEqualMem(buff, file_test_txt6, sizeof(file_test_txt4) - 1);
 
   /* Seek to 32 bytes before the end of the second data sub-block */
-  assertA(8029 == archive_seek_data(a, 8029, SEEK_SET));
+  assertA(8029 == archive_seek_data(a, 8029 - file_size, SEEK_END));
   assertA(sizeof(buff) == archive_read_data(a, buff, sizeof(buff)));
   assertEqualMem(buff, file_test_txt7, sizeof(file_test_txt5) - 1);
 
   /* Seek to 32 bytes before the end of the third data sub-block */
-  assertA(15089 == archive_seek_data(a, 15089, SEEK_SET));
+  assertA(15089 == archive_seek_data(a, 15089 - file_size, SEEK_END));
   assertA(sizeof(buff) == archive_read_data(a, buff, sizeof(buff)));
   assertEqualMem(buff, file_test_txt8, sizeof(file_test_txt5) - 1);