From 58c1b96165e525808280502581700909d6371715 Mon Sep 17 00:00:00 2001 From: Tao Bao Date: Wed, 20 May 2015 09:32:18 -0700 Subject: [PATCH] Fix the permission setting in common.ZipWriteStr() When passing a ZipInfo instance to common.ZipWriteStr(), the external_attr attribute should not be overwritten unless specified. We didn't have the issue previously because we were calling ZipFile.writestr() directly until [1] merged. [1] commit 2ed665a033c587b276b1615516e5354e2ace47cd. Bug: http://b/21309935 Change-Id: I8c0190362c60d7d78965ecfe5e484f8398ddc5f2 (cherry picked from commit 97734654099431bd6c5bd2eeb5d34af0e2dc03e7) --- tools/releasetools/common.py | 9 ++++++-- tools/releasetools/test_common.py | 48 +++++++++++++++++++++++++++++++++++---- 2 files changed, 50 insertions(+), 7 deletions(-) diff --git a/tools/releasetools/common.py b/tools/releasetools/common.py index 8ab98e724..4f8549158 100644 --- a/tools/releasetools/common.py +++ b/tools/releasetools/common.py @@ -862,7 +862,7 @@ def ZipWrite(zip_file, filename, arcname=None, perms=0o644, zipfile.ZIP64_LIMIT = saved_zip64_limit -def ZipWriteStr(zip_file, zinfo_or_arcname, data, perms=0o644, +def ZipWriteStr(zip_file, zinfo_or_arcname, data, perms=None, compress_type=None): """Wrap zipfile.writestr() function to work around the zip64 limit. @@ -881,6 +881,8 @@ def ZipWriteStr(zip_file, zinfo_or_arcname, data, perms=0o644, if not isinstance(zinfo_or_arcname, zipfile.ZipInfo): zinfo = zipfile.ZipInfo(filename=zinfo_or_arcname) zinfo.compress_type = zip_file.compression + if perms is None: + perms = 0o644 else: zinfo = zinfo_or_arcname @@ -888,8 +890,11 @@ def ZipWriteStr(zip_file, zinfo_or_arcname, data, perms=0o644, if compress_type is not None: zinfo.compress_type = compress_type + # If perms is given, it has a priority. + if perms is not None: + zinfo.external_attr = perms << 16 + # Use a fixed timestamp so the output is repeatable. - zinfo.external_attr = perms << 16 zinfo.date_time = (2009, 1, 1, 0, 0, 0) zip_file.writestr(zinfo, data) diff --git a/tools/releasetools/test_common.py b/tools/releasetools/test_common.py index f28934d25..a8613465c 100644 --- a/tools/releasetools/test_common.py +++ b/tools/releasetools/test_common.py @@ -121,15 +121,18 @@ class CommonZipTest(unittest.TestCase): time.sleep(5) # Make sure the atime/mtime will change measurably. if not isinstance(zinfo_or_arcname, zipfile.ZipInfo): - zinfo = zipfile.ZipInfo(filename=zinfo_or_arcname) + arcname = zinfo_or_arcname + expected_mode = extra_args.get("perms", 0o644) else: - zinfo = zinfo_or_arcname - arcname = zinfo.filename + arcname = zinfo_or_arcname.filename + expected_mode = extra_args.get("perms", + zinfo_or_arcname.external_attr >> 16) - common.ZipWriteStr(zip_file, zinfo, contents, **extra_args) + common.ZipWriteStr(zip_file, zinfo_or_arcname, contents, **extra_args) common.ZipClose(zip_file) self._verify(zip_file, zip_file_name, arcname, contents, + expected_mode=expected_mode, expected_compress_type=expected_compress_type) finally: os.remove(zip_file_name) @@ -228,9 +231,10 @@ class CommonZipTest(unittest.TestCase): random_string = os.urandom(1024) # Passing arcname self._test_ZipWriteStr("foo", random_string, { + "perms": 0o700, "compress_type": zipfile.ZIP_DEFLATED, }) - self._test_ZipWriteStr("foo", random_string, { + self._test_ZipWriteStr("bar", random_string, { "compress_type": zipfile.ZIP_STORED, }) @@ -240,6 +244,7 @@ class CommonZipTest(unittest.TestCase): "compress_type": zipfile.ZIP_DEFLATED, }) self._test_ZipWriteStr(zinfo, random_string, { + "perms": 0o600, "compress_type": zipfile.ZIP_STORED, }) @@ -257,3 +262,36 @@ class CommonZipTest(unittest.TestCase): self._test_reset_ZIP64_LIMIT(self._test_ZipWriteStr, "foo", "") zinfo = zipfile.ZipInfo(filename="foo") self._test_reset_ZIP64_LIMIT(self._test_ZipWriteStr, zinfo, "") + + def test_bug21309935(self): + zip_file = tempfile.NamedTemporaryFile(delete=False) + zip_file_name = zip_file.name + zip_file.close() + + try: + random_string = os.urandom(1024) + zip_file = zipfile.ZipFile(zip_file_name, "w") + # Default perms should be 0o644 when passing the filename. + common.ZipWriteStr(zip_file, "foo", random_string) + # Honor the specified perms. + common.ZipWriteStr(zip_file, "bar", random_string, perms=0o755) + # The perms in zinfo should be untouched. + zinfo = zipfile.ZipInfo(filename="baz") + zinfo.external_attr = 0o740 << 16 + common.ZipWriteStr(zip_file, zinfo, random_string) + # Explicitly specified perms has the priority. + zinfo = zipfile.ZipInfo(filename="qux") + zinfo.external_attr = 0o700 << 16 + common.ZipWriteStr(zip_file, zinfo, random_string, perms=0o400) + common.ZipClose(zip_file) + + self._verify(zip_file, zip_file_name, "foo", random_string, + expected_mode=0o644) + self._verify(zip_file, zip_file_name, "bar", random_string, + expected_mode=0o755) + self._verify(zip_file, zip_file_name, "baz", random_string, + expected_mode=0o740) + self._verify(zip_file, zip_file_name, "qux", random_string, + expected_mode=0o400) + finally: + os.remove(zip_file_name) -- 2.11.0