[Python-checkins] bpo-18819: tarfile: only set device fields for device files (GH-18080)

[William Chargin]

https://github.com/python/cpython/commit/674935b8caf33e47c78f1b8e197b1b77a04992d2
commit: 674935b8caf33e47c78f1b8e197b1b77a04992d2
branch: master
author: William Chargin <wchargin at gmail.com>
committer: GitHub <noreply at github.com>
date: 2020-02-12T11:56:02-08:00
summary:

bpo-18819: tarfile: only set device fields for device files (GH-18080)

The GNU docs describe the `devmajor` and `devminor` fields of the tar
header struct only in the context of character and block special files,
suggesting that in other cases they are not populated. Typical utilities
behave accordingly; this patch teaches `tarfile` to do the same.

files:
A Misc/NEWS.d/next/Library/2020-01-20-10-06-19.bpo-18819.H4qsoS.rst
M Lib/tarfile.py
M Lib/test/test_tarfile.py
M Misc/ACKS

diff --git a/Lib/tarfile.py b/Lib/tarfile.py
index 90a2c95b315b3..e2b60532f693d 100755
--- a/Lib/tarfile.py
+++ b/Lib/tarfile.py
@@ -930,6 +930,14 @@ def _create_header(info, format, encoding, errors):
         """Return a header block. info is a dictionary with file
            information, format must be one of the *_FORMAT constants.
         """
+        has_device_fields = info.get("type") in (CHRTYPE, BLKTYPE)
+        if has_device_fields:
+            devmajor = itn(info.get("devmajor", 0), 8, format)
+            devminor = itn(info.get("devminor", 0), 8, format)
+        else:
+            devmajor = stn("", 8, encoding, errors)
+            devminor = stn("", 8, encoding, errors)
+
         parts = [
             stn(info.get("name", ""), 100, encoding, errors),
             itn(info.get("mode", 0) & 0o7777, 8, format),
@@ -943,8 +951,8 @@ def _create_header(info, format, encoding, errors):
             info.get("magic", POSIX_MAGIC),
             stn(info.get("uname", ""), 32, encoding, errors),
             stn(info.get("gname", ""), 32, encoding, errors),
-            itn(info.get("devmajor", 0), 8, format),
-            itn(info.get("devminor", 0), 8, format),
+            devmajor,
+            devminor,
             stn(info.get("prefix", ""), 155, encoding, errors)
         ]
 
diff --git a/Lib/test/test_tarfile.py b/Lib/test/test_tarfile.py
index 6a901089611cd..cae96802ded67 100644
--- a/Lib/test/test_tarfile.py
+++ b/Lib/test/test_tarfile.py
@@ -1549,6 +1549,52 @@ def test_longnamelink_1025(self):
                    ("longlnk/" * 127) + "longlink_")
 
 
+class DeviceHeaderTest(WriteTestBase, unittest.TestCase):
+
+    prefix = "w:"
+
+    def test_headers_written_only_for_device_files(self):
+        # Regression test for bpo-18819.
+        tempdir = os.path.join(TEMPDIR, "device_header_test")
+        os.mkdir(tempdir)
+        try:
+            tar = tarfile.open(tmpname, self.mode)
+            try:
+                input_blk = tarfile.TarInfo(name="my_block_device")
+                input_reg = tarfile.TarInfo(name="my_regular_file")
+                input_blk.type = tarfile.BLKTYPE
+                input_reg.type = tarfile.REGTYPE
+                tar.addfile(input_blk)
+                tar.addfile(input_reg)
+            finally:
+                tar.close()
+
+            # devmajor and devminor should be *interpreted* as 0 in both...
+            tar = tarfile.open(tmpname, "r")
+            try:
+                output_blk = tar.getmember("my_block_device")
+                output_reg = tar.getmember("my_regular_file")
+            finally:
+                tar.close()
+            self.assertEqual(output_blk.devmajor, 0)
+            self.assertEqual(output_blk.devminor, 0)
+            self.assertEqual(output_reg.devmajor, 0)
+            self.assertEqual(output_reg.devminor, 0)
+
+            # ...but the fields should not actually be set on regular files:
+            with open(tmpname, "rb") as infile:
+                buf = infile.read()
+            buf_blk = buf[output_blk.offset:output_blk.offset_data]
+            buf_reg = buf[output_reg.offset:output_reg.offset_data]
+            # See `struct posixheader` in GNU docs for byte offsets:
+            # <https://www.gnu.org/software/tar/manual/html_node/Standard.html>
+            device_headers = slice(329, 329 + 16)
+            self.assertEqual(buf_blk[device_headers], b"0000000\0" * 2)
+            self.assertEqual(buf_reg[device_headers], b"\0" * 16)
+        finally:
+            support.rmtree(tempdir)
+
+
 class CreateTest(WriteTestBase, unittest.TestCase):
 
     prefix = "x:"
diff --git a/Misc/ACKS b/Misc/ACKS
index 5a779833e68be..933402069b4fd 100644
--- a/Misc/ACKS
+++ b/Misc/ACKS
@@ -286,6 +286,7 @@ Brad Chapman
 Greg Chapman
 Mitch Chapman
 Matt Chaput
+William Chargin
 Yogesh Chaudhari
 David Chaum
 Nicolas Chauvat
diff --git a/Misc/NEWS.d/next/Library/2020-01-20-10-06-19.bpo-18819.H4qsoS.rst b/Misc/NEWS.d/next/Library/2020-01-20-10-06-19.bpo-18819.H4qsoS.rst
new file mode 100644
index 0000000000000..e9f111ad62e28
--- /dev/null
+++ b/Misc/NEWS.d/next/Library/2020-01-20-10-06-19.bpo-18819.H4qsoS.rst
@@ -0,0 +1,3 @@
+Omit ``devmajor`` and ``devminor`` fields for non-device files in
+:mod:`tarfile` archives, enabling bit-for-bit compatibility with GNU
+``tar(1)``.