gh-116931: Add fileobj parameter check for Tarfile.addfile (GH-117988)

Tarfile.addfile now throws an ValueError when the user passes
in a non-zero size tarinfo but does not provide a fileobj,
instead of writing an incomplete entry.

4 files changed, 25 insertions, 9 deletions

diff --git a/Doc/library/tarfile.rst b/Doc/library/tarfile.rst
index 2134293a..afcad9b 100644
--- a/Doc/library/tarfile.rst
+++ b/Doc/library/tarfile.rst
@@ -637,11 +637,15 @@ be finalized; only the internally used file object will be closed. See the
 
 .. method:: TarFile.addfile(tarinfo, fileobj=None)
 
-   Add the :class:`TarInfo` object *tarinfo* to the archive. If *fileobj* is given,
-   it should be a :term:`binary file`, and
-   ``tarinfo.size`` bytes are read from it and added to the archive.  You can
+   Add the :class:`TarInfo` object *tarinfo* to the archive. If *tarinfo* represents
+   a non zero-size regular file, the *fileobj* argument should be a :term:`binary file`,
+   and ``tarinfo.size`` bytes are read from it and added to the archive.  You can
    create :class:`TarInfo` objects directly, or by using :meth:`gettarinfo`.
 
+   .. versionchanged:: 3.13
+
+      *fileobj* must be given for non-zero-sized regular files.
+
 
 .. method:: TarFile.gettarinfo(name=None, arcname=None, fileobj=None)
 
diff --git a/Lib/tarfile.py b/Lib/tarfile.py
index 149b1c3..78bb10c 100755
--- a/Lib/tarfile.py
+++ b/Lib/tarfile.py
@@ -2214,13 +2214,16 @@ class TarFile(object):
             self.addfile(tarinfo)
 
     def addfile(self, tarinfo, fileobj=None):
-        """Add the TarInfo object `tarinfo' to the archive. If `fileobj' is
-           given, it should be a binary file, and tarinfo.size bytes are read
-           from it and added to the archive. You can create TarInfo objects
-           directly, or by using gettarinfo().
+        """Add the TarInfo object `tarinfo' to the archive. If `tarinfo' represents
+           a non zero-size regular file, the `fileobj' argument should be a binary file,
+           and tarinfo.size bytes are read from it and added to the archive.
+           You can create TarInfo objects directly, or by using gettarinfo().
         """
         self._check("awx")
 
+        if fileobj is None and tarinfo.isreg() and tarinfo.size != 0:
+            raise ValueError("fileobj not provided for non zero-size regular file")
+
         tarinfo = copy.copy(tarinfo)
 
         buf = tarinfo.tobuf(self.format, self.encoding, self.errors)
diff --git a/Lib/test/test_tarfile.py b/Lib/test/test_tarfile.py
index f18dcc0..c2abc7e 100644
--- a/Lib/test/test_tarfile.py
+++ b/Lib/test/test_tarfile.py
@@ -1612,6 +1612,12 @@ class WriteTest(WriteTestBase, unittest.TestCase):
                                    pax_headers={'non': 'empty'})
             self.assertFalse(f.closed)
 
+    def test_missing_fileobj(self):
+        with tarfile.open(tmpname, self.mode) as tar:
+            tarinfo = tar.gettarinfo(tarname)
+            with self.assertRaises(ValueError):
+                tar.addfile(tarinfo)
+
 
 class GzipWriteTest(GzipTest, WriteTest):
     pass
@@ -3283,7 +3289,8 @@ class NoneInfoTests_Misc(unittest.TestCase):
                 tar = tarfile.open(fileobj=bio, mode='w', format=tarformat)
                 tarinfo = tar.gettarinfo(tarname)
                 try:
-                    tar.addfile(tarinfo)
+                    with open(tarname, 'rb') as f:
+                        tar.addfile(tarinfo, f)
                 except Exception:
                     if tarformat == tarfile.USTAR_FORMAT:
                         # In the old, limited format, adding might fail for
@@ -3298,7 +3305,8 @@ class NoneInfoTests_Misc(unittest.TestCase):
                             replaced = tarinfo.replace(**{attr_name: None})
                             with self.assertRaisesRegex(ValueError,
                                                         f"{attr_name}"):
-                                tar.addfile(replaced)
+                                with open(tarname, 'rb') as f:
+                                    tar.addfile(replaced, f)
 
     def test_list(self):
         # Change some metadata to None, then compare list() output
diff --git a/Misc/NEWS.d/next/Library/2024-04-17-21-28-24.gh-issue-116931._AS09h.rst b/Misc/NEWS.d/next/Library/2024-04-17-21-28-24.gh-issue-116931._AS09h.rst
new file mode 100644
index 0000000..98df8f5
--- /dev/null
+++ b/Misc/NEWS.d/next/Library/2024-04-17-21-28-24.gh-issue-116931._AS09h.rst
@@ -0,0 +1 @@
+Add parameter *fileobj* check for :func:`tarfile.addfile()`