Merge branch 'rewrite-fix' into 'master'

Fix TIFFRewriteDirectory discarding directories after the rewritten one

See merge request libtiff/libtiff!264

2 files changed, 104 insertions, 15 deletions

diff --git a/libtiff/tif_dirwrite.c b/libtiff/tif_dirwrite.c
index 067ee410..0e1b204f 100644
--- a/libtiff/tif_dirwrite.c
+++ b/libtiff/tif_dirwrite.c
@@ -337,6 +337,8 @@ TIFFRewriteDirectory( TIFF *tif )
 						return (0);
 					}
 					tif->tif_diroff=0;
+					/* Force a full-traversal to reach the zeroed pointer */
+					tif->tif_lastdiroff=0;
 					break;
 				}
 				nextdir=nextnextdir;
@@ -403,6 +405,8 @@ TIFFRewriteDirectory( TIFF *tif )
 						return (0);
 					}
 					tif->tif_diroff=0;
+					/* Force a full-traversal to reach the zeroed pointer */
+					tif->tif_lastdiroff=0;
 					break;
 				}
 				nextdir=nextnextdir;
diff --git a/test/test_directory.c b/test/test_directory.c
index cd3987ec..6c4389aa 100644
--- a/test/test_directory.c
+++ b/test/test_directory.c
@@ -5,14 +5,14 @@
  * to an open file.
  *
  * Currently, there is an optimization where the TIFF data structure stores the offset of the last written directory in
- * order to avoid having to transverse the entire directory list each time a new one is added. The offset is not stored
+ * order to avoid having to traverse the entire directory list each time a new one is added. The offset is not stored
  * in the file itself, only in the in-memory data structure. This means we still go through the entire list the first
  * time a directory is appended to a newly-opened file, and the shortcut is taken for subsequent directory writes.
  *
  * In order to test the correctness of the optimization, the `test_lastdir_offset` function writes 10 directories to two
  * different files. For the first file we use the optimization, by simply calling TIFFWriteDirectory() repeatedly on an
  * open TIFF handle. For the second file, we avoid the optimization by closing the file after each call to
- * TIFFWriteDirectory(), which means the next directory write will transverse the entire list.
+ * TIFFWriteDirectory(), which means the next directory write will traverse the entire list.
  *
  * Finally, the two files are compared to check that the number of directories written is the same, and that their
  * offsets match. The test is then repeated using BigTIFF files.
@@ -39,7 +39,7 @@ const uint16_t photometric = PHOTOMETRIC_RGB;
 const uint16_t rows_per_strip = 1;
 const uint16_t planarconfig = PLANARCONFIG_CONTIG;
 
-int write_directory_to_open_file(TIFF *tif) {
+int write_data_to_current_directory(TIFF *tif) {
     unsigned char buf[SPP] = {0, 127, 255};
 
     if (!tif) {
@@ -81,11 +81,6 @@ int write_directory_to_open_file(TIFF *tif) {
         return 1;
     }
 
-    if (!TIFFWriteDirectory(tif)) {
-        fprintf(stderr, "TIFFWriteDirectory() failed.\n");
-        return 1;
-    }
-
     return 0;
 }
 
@@ -96,11 +91,19 @@ int write_directory_to_closed_file(const char *filename, bool is_big_tiff) {
         fprintf(stderr, "Can't create/open %s\n", filename);
         return 1;
     }
-    if (write_directory_to_open_file(tif)) {
-        fprintf(stderr, "Can't add new directory.\n");
+
+    if (write_data_to_current_directory(tif)) {
+        fprintf(stderr, "Can't write data to current directory.\n");
         TIFFClose(tif);
         return 1;
     }
+
+    if (!TIFFWriteDirectory(tif)) {
+        fprintf(stderr, "TIFFWriteDirectory() failed.\n");
+        TIFFClose(tif);
+        return 1;
+    }
+
     TIFFClose(tif);
     return 0;
 }
@@ -143,6 +146,74 @@ int get_dir_offsets(const char *filename, uint64_t *offsets) {
     return 0;
 }
 
+/* Checks that rewriting a directory does not break the directory linked list
+ *
+ * This could happen because TIFFRewriteDirectory relies on the traversal of the directory linked list in order to
+ * move the rewritten directory to the end of the file. This means the `lastdir_offset` optimization should be skipped,
+ * otherwise the linked list will be broken at the point where it connected to the rewritten directory, resulting in the
+ * loss of the directories that come after it.
+*/
+int test_rewrite_lastdir_offset(bool is_big_tiff) {
+    const char *filename = "test_directory_rewrite.tif";
+    int i, count;
+    TIFF *tif;
+
+/* Create a file and write N_DIRECTORIES (10) directories to it */
+    tif = TIFFOpen(filename, is_big_tiff ? "w8" : "w");
+    if (!tif) {
+        fprintf(stderr, "Can't create %s\n", filename);
+        return 1;
+    }
+    for (i = 0; i < N_DIRECTORIES; i++) {
+        if (write_data_to_current_directory(tif)) {
+            fprintf(stderr, "Can't write data to current directory in %s\n", filename);
+            goto failure;
+        }
+        if (!TIFFWriteDirectory(tif)) {
+            fprintf(stderr, "Can't write directory to %s\n", filename);
+            goto failure;
+        }
+    }
+
+    /* Without closing it, go to the fifth directory */
+    TIFFSetDirectory(tif, 4);
+
+    /* Rewrite the fifth directory by calling TIFFRewriteDirectory */
+    if (write_data_to_current_directory(tif)) {
+        fprintf(stderr, "Can't write data to fifth directory in %s\n", filename);
+        goto failure;
+    }
+    if (!TIFFRewriteDirectory(tif)) {
+        fprintf(stderr, "Can't rewrite fifth directory to %s\n", filename);
+        goto failure;
+    }
+
+    TIFFClose(tif);
+    tif = NULL;
+
+    /* Check that the file has the expected number of directories*/
+    if (count_directories(filename, &count)) {
+        fprintf(stderr, "Error counting directories in file %s.\n", filename);
+        goto failure;
+    }
+    if (count != N_DIRECTORIES) {
+        fprintf(stderr, "Unexpected number of directories in %s. Expected %i, found %i.\n", filename,
+                N_DIRECTORIES, count);
+        goto failure;
+    }
+
+    unlink(filename);
+    return 0;
+
+    failure:
+    if (tif) {
+        TIFFClose(tif);
+        tif = NULL;
+    }
+    unlink(filename);
+    return 1;
+}
+
 /* Compares multi-directory files written with and without the lastdir optimization */
 int test_lastdir_offset(bool is_big_tiff) {
     const char *filename_optimized = "test_directory_optimized.tif";
@@ -159,7 +230,11 @@ int test_lastdir_offset(bool is_big_tiff) {
         return 1;
     }
     for (i = 0; i < N_DIRECTORIES; i++) {
-        if (write_directory_to_open_file(tif)) {
+        if (write_data_to_current_directory(tif)) {
+            fprintf(stderr, "Can't write data to current directory in %s\n", filename_optimized);
+            goto failure;
+        }
+        if (!TIFFWriteDirectory(tif)) {
             fprintf(stderr, "Can't write directory to %s\n", filename_optimized);
             goto failure;
         }
@@ -206,7 +281,8 @@ int test_lastdir_offset(bool is_big_tiff) {
     }
     for (i = 0; i < N_DIRECTORIES; i++) {
         if (offsets_optimized[i] != offsets_non_optimized[i]) {
-            fprintf(stderr, "Unexpected directory offset for directory %i, expected offset %"PRIu64" but got %"PRIu64".\n",
+            fprintf(stderr,
+                    "Unexpected directory offset for directory %i, expected offset %"PRIu64" but got %"PRIu64".\n",
                     i,
                     offsets_non_optimized[i], offsets_optimized[i]);
             goto failure;
@@ -230,12 +306,21 @@ int test_lastdir_offset(bool is_big_tiff) {
 
 int main() {
     if (test_lastdir_offset(false)) {
-        fprintf(stderr, "Failed during non-BigTIFF test.\n");
+        fprintf(stderr, "Failed during non-BigTIFF WriteDirectory test.\n");
         return 1;
     }
-
     if (test_lastdir_offset(true)) {
-        fprintf(stderr, "Failed during BigTIFF test.\n");
+        fprintf(stderr, "Failed during BigTIFF WriteDirectory test.\n");
+        return 1;
+    }
+
+
+    if (test_rewrite_lastdir_offset(false)) {
+        fprintf(stderr, "Failed during non-BigTIFF RewriteDirectory test.\n");
+        return 1;
+    }
+    if (test_rewrite_lastdir_offset(true)) {
+        fprintf(stderr, "Failed during BigTIFF RewriteDirectory test.\n");
         return 1;
     }
     return 0;