Do not close provided file handles with libtiff

hugovk · Oct 11, 2024 · 7edf952 · 7edf952
1 parent 3d9c05c
commit 7edf952
Show file tree

Hide file tree

Showing 2 changed files with 23 additions and 10 deletions.
diff --git a/src/PIL/TiffImagePlugin.py b/src/PIL/TiffImagePlugin.py
@@ -1864,7 +1864,7 @@ def _save(im: Image.Image, fp: IO[bytes], filename: str | bytes) -> None:
         if hasattr(fp, "fileno"):
             try:
                 fp.seek(0)
-                _fp = os.dup(fp.fileno())
+                _fp = fp.fileno()
             except io.UnsupportedOperation:
                 pass
 
@@ -1943,11 +1943,6 @@ def _save(im: Image.Image, fp: IO[bytes], filename: str | bytes) -> None:
                 fp.write(data)
             if errcode:
                 break
-        if _fp:
-            try:
-                os.close(_fp)
-            except OSError:
-                pass
         if errcode < 0:
             msg = f"encoder error {errcode} when writing image file"
             raise OSError(msg)

diff --git a/src/libImaging/TiffDecode.c b/src/libImaging/TiffDecode.c
@@ -780,7 +780,7 @@ ImagingLibTiffDecode(
 decode_err:
     // TIFFClose in libtiff calls tif_closeproc and TIFFCleanup
     if (clientstate->fp) {
-        // Pillow will manage the closing of the file rather than libtiff
+        // Python will manage the closing of the file rather than libtiff
         // So only call TIFFCleanup
         TIFFCleanup(tiff);
     } else {
@@ -1008,7 +1008,17 @@ ImagingLibTiffEncode(Imaging im, ImagingCodecState state, UINT8 *buffer, int byt
                 ) == -1) {
                 TRACE(("Encode Error, row %d\n", state->y));
                 state->errcode = IMAGING_CODEC_BROKEN;
-                TIFFClose(tiff);
+
+                // TIFFClose in libtiff calls tif_closeproc and TIFFCleanup
+                if (clientstate->fp) {
+                    // Python will manage the closing of the file rather than libtiff
+                    // So only call TIFFCleanup
+                    TIFFCleanup(tiff);
+                } else {
+                    // When tif_closeproc refers to our custom _tiffCloseProc though,
+                    // that is fine, as it does not close the file
+                    TIFFClose(tiff);
+                }
                 if (!clientstate->fp) {
                     free(clientstate->data);
                 }
@@ -1025,14 +1035,22 @@ ImagingLibTiffEncode(Imaging im, ImagingCodecState state, UINT8 *buffer, int byt
                 TRACE(("Error flushing the tiff"));
                 // likely reason is memory.
                 state->errcode = IMAGING_CODEC_MEMORY;
-                TIFFClose(tiff);
+                if (clientstate->fp) {
+                    TIFFCleanup(tiff);
+                } else {
+                    TIFFClose(tiff);
+                }
                 if (!clientstate->fp) {
                     free(clientstate->data);
                 }
                 return -1;
             }
             TRACE(("Closing \n"));
-            TIFFClose(tiff);
+            if (clientstate->fp) {
+                TIFFCleanup(tiff);
+            } else {
+                TIFFClose(tiff);
+            }
             // reset the clientstate metadata to use it to read out the buffer.
             clientstate->loc = 0;
             clientstate->size = clientstate->eof;  // redundant?