From f92f7f046a07b79ce768692c7500cc74f733aa55 Mon Sep 17 00:00:00 2001 From: Kelvin Zhang Date: Fri, 14 Apr 2023 21:32:54 +0000 Subject: [PATCH] Revert "Remove all ZIP64LIMIT hack" This reverts commit 37a4290909717577731bc53c8f4ccd72c4d4d5d5. Reason for revert: b/278156419 Change-Id: I67ea667619a9623be849d911993010ef0f0bfd88 --- tools/releasetools/add_img_to_target_files.py | 15 +-- tools/releasetools/apex_utils.py | 2 +- .../check_ota_package_signature.py | 4 +- tools/releasetools/common.py | 31 +++++- tools/releasetools/non_ab_ota.py | 4 +- tools/releasetools/ota_from_target_files.py | 10 +- tools/releasetools/ota_utils.py | 2 +- tools/releasetools/sign_target_files_apks.py | 6 +- tools/releasetools/test_common.py | 94 ++++++++++--------- 9 files changed, 101 insertions(+), 67 deletions(-) diff --git a/tools/releasetools/add_img_to_target_files.py b/tools/releasetools/add_img_to_target_files.py index d308a55dad..465e1a8f9e 100644 --- a/tools/releasetools/add_img_to_target_files.py +++ b/tools/releasetools/add_img_to_target_files.py @@ -842,13 +842,14 @@ def ReplaceUpdatedFiles(zip_filename, files_list): SYSTEM/ after rebuilding recovery. """ common.ZipDelete(zip_filename, files_list) - with zipfile.ZipFile(zip_filename, "a", + output_zip = zipfile.ZipFile(zip_filename, "a", compression=zipfile.ZIP_DEFLATED, - allowZip64=True) as output_zip: - for item in files_list: - file_path = os.path.join(OPTIONS.input_tmp, item) - assert os.path.exists(file_path) - common.ZipWrite(output_zip, file_path, arcname=item) + allowZip64=True) + for item in files_list: + file_path = os.path.join(OPTIONS.input_tmp, item) + assert os.path.exists(file_path) + common.ZipWrite(output_zip, file_path, arcname=item) + common.ZipClose(output_zip) def HasPartition(partition_name): @@ -1175,7 +1176,7 @@ def AddImagesToTargetFiles(filename): AddVbmetaDigest(output_zip) if output_zip: - output_zip.close() + common.ZipClose(output_zip) if OPTIONS.replace_updated_files_list: ReplaceUpdatedFiles(output_zip.filename, OPTIONS.replace_updated_files_list) diff --git a/tools/releasetools/apex_utils.py b/tools/releasetools/apex_utils.py index 22992e8b1b..d7b0ba259b 100644 --- a/tools/releasetools/apex_utils.py +++ b/tools/releasetools/apex_utils.py @@ -415,7 +415,7 @@ def SignUncompressedApex(avbtool, apex_file, payload_key, container_key, apex_zip = zipfile.ZipFile(apex_file, 'a', allowZip64=True) common.ZipWrite(apex_zip, payload_file, arcname=APEX_PAYLOAD_IMAGE) common.ZipWrite(apex_zip, payload_public_key, arcname=APEX_PUBKEY) - apex_zip.close() + common.ZipClose(apex_zip) # 3. Sign the APEX container with container_key. signed_apex = common.MakeTempFile(prefix='apex-container-', suffix='.apex') diff --git a/tools/releasetools/check_ota_package_signature.py b/tools/releasetools/check_ota_package_signature.py index 97957be28f..b395c196d0 100755 --- a/tools/releasetools/check_ota_package_signature.py +++ b/tools/releasetools/check_ota_package_signature.py @@ -142,7 +142,7 @@ def VerifyAbOtaPayload(cert, package): """Verifies the payload and metadata signatures in an A/B OTA payload.""" package_zip = zipfile.ZipFile(package, 'r', allowZip64=True) if 'payload.bin' not in package_zip.namelist(): - package_zip.close() + common.ZipClose(package_zip) return print('Verifying A/B OTA payload signatures...') @@ -160,7 +160,7 @@ def VerifyAbOtaPayload(cert, package): '--in_file=' + payload_file, '--public_key=' + pubkey] common.RunAndCheckOutput(cmd) - package_zip.close() + common.ZipClose(package_zip) # Verified successfully upon reaching here. print('\nPayload signatures VERIFIED\n\n') diff --git a/tools/releasetools/common.py b/tools/releasetools/common.py index 2f05d4415a..715802faae 100644 --- a/tools/releasetools/common.py +++ b/tools/releasetools/common.py @@ -2797,6 +2797,18 @@ class PasswordManager(object): def ZipWrite(zip_file, filename, arcname=None, perms=0o644, compress_type=None): + # http://b/18015246 + # Python 2.7's zipfile implementation wrongly thinks that zip64 is required + # for files larger than 2GiB. We can work around this by adjusting their + # limit. Note that `zipfile.writestr()` will not work for strings larger than + # 2GiB. The Python interpreter sometimes rejects strings that large (though + # it isn't clear to me exactly what circumstances cause this). + # `zipfile.write()` must be used directly to work around this. + # + # This mess can be avoided if we port to python3. + saved_zip64_limit = zipfile.ZIP64_LIMIT + zipfile.ZIP64_LIMIT = (1 << 32) - 1 + if compress_type is None: compress_type = zip_file.compression if arcname is None: @@ -2822,13 +2834,14 @@ def ZipWrite(zip_file, filename, arcname=None, perms=0o644, finally: os.chmod(filename, saved_stat.st_mode) os.utime(filename, (saved_stat.st_atime, saved_stat.st_mtime)) + zipfile.ZIP64_LIMIT = saved_zip64_limit def ZipWriteStr(zip_file, zinfo_or_arcname, data, perms=None, compress_type=None): """Wrap zipfile.writestr() function to work around the zip64 limit. - Python's zip implementation won't allow writing a string + Even with the ZIP64_LIMIT workaround, it won't allow writing a string longer than 2GiB. It gives 'OverflowError: size does not fit in an int' when calling crc32(bytes). @@ -2837,6 +2850,9 @@ def ZipWriteStr(zip_file, zinfo_or_arcname, data, perms=None, when we know the string won't be too long. """ + saved_zip64_limit = zipfile.ZIP64_LIMIT + zipfile.ZIP64_LIMIT = (1 << 32) - 1 + if not isinstance(zinfo_or_arcname, zipfile.ZipInfo): zinfo = zipfile.ZipInfo(filename=zinfo_or_arcname) zinfo.compress_type = zip_file.compression @@ -2869,6 +2885,7 @@ def ZipWriteStr(zip_file, zinfo_or_arcname, data, perms=None, zinfo.date_time = (2009, 1, 1, 0, 0, 0) zip_file.writestr(zinfo, data) + zipfile.ZIP64_LIMIT = saved_zip64_limit def ZipDelete(zip_filename, entries, force=False): @@ -2902,6 +2919,18 @@ def ZipDelete(zip_filename, entries, force=False): os.replace(new_zipfile, zip_filename) +def ZipClose(zip_file): + # http://b/18015246 + # zipfile also refers to ZIP64_LIMIT during close() when it writes out the + # central directory. + saved_zip64_limit = zipfile.ZIP64_LIMIT + zipfile.ZIP64_LIMIT = (1 << 32) - 1 + + zip_file.close() + + zipfile.ZIP64_LIMIT = saved_zip64_limit + + class DeviceSpecificParams(object): module = None diff --git a/tools/releasetools/non_ab_ota.py b/tools/releasetools/non_ab_ota.py index ac85aa4f1f..6c927ec8c7 100644 --- a/tools/releasetools/non_ab_ota.py +++ b/tools/releasetools/non_ab_ota.py @@ -277,7 +277,7 @@ endif; # We haven't written the metadata entry, which will be done in # FinalizeMetadata. - output_zip.close() + common.ZipClose(output_zip) needed_property_files = ( NonAbOtaPropertyFiles(), @@ -531,7 +531,7 @@ endif; # We haven't written the metadata entry yet, which will be handled in # FinalizeMetadata(). - output_zip.close() + common.ZipClose(output_zip) # Sign the generated zip package unless no_signing is specified. needed_property_files = ( diff --git a/tools/releasetools/ota_from_target_files.py b/tools/releasetools/ota_from_target_files.py index 60e95add6b..9d5c67d14b 100755 --- a/tools/releasetools/ota_from_target_files.py +++ b/tools/releasetools/ota_from_target_files.py @@ -487,7 +487,7 @@ def GetTargetFilesZipForSecondaryImages(input_file, skip_postinstall=False): else: common.ZipWrite(target_zip, unzipped_file, arcname=info.filename) - target_zip.close() + common.ZipClose(target_zip) return target_file @@ -624,7 +624,7 @@ def GetTargetFilesZipForPartialUpdates(input_file, ab_partitions): # TODO(xunchang) handle META/postinstall_config.txt' - partial_target_zip.close() + common.ZipClose(partial_target_zip) return partial_target_file @@ -709,7 +709,7 @@ def GetTargetFilesZipForRetrofitDynamicPartitions(input_file, # Write new ab_partitions.txt file common.ZipWrite(target_zip, new_ab_partitions, arcname=AB_PARTITIONS) - target_zip.close() + common.ZipClose(target_zip) return target_file @@ -1017,11 +1017,11 @@ def GenerateAbOtaPackage(target_file, output_file, source_file=None): common.ZipWriteStr(output_zip, "apex_info.pb", ota_apex_info, compress_type=zipfile.ZIP_STORED) - target_zip.close() + common.ZipClose(target_zip) # We haven't written the metadata entry yet, which will be handled in # FinalizeMetadata(). - output_zip.close() + common.ZipClose(output_zip) FinalizeMetadata(metadata, staging_file, output_file, package_key=OPTIONS.package_key) diff --git a/tools/releasetools/ota_utils.py b/tools/releasetools/ota_utils.py index e36a2be126..9f418749bc 100644 --- a/tools/releasetools/ota_utils.py +++ b/tools/releasetools/ota_utils.py @@ -22,7 +22,7 @@ import zipfile import ota_metadata_pb2 import common -from common import (ZipDelete, OPTIONS, MakeTempFile, +from common import (ZipDelete, ZipClose, OPTIONS, MakeTempFile, ZipWriteStr, BuildInfo, LoadDictionaryFromFile, SignFile, PARTITIONS_WITH_BUILD_PROP, PartitionBuildProps, GetRamdiskFormat) diff --git a/tools/releasetools/sign_target_files_apks.py b/tools/releasetools/sign_target_files_apks.py index 4e7274fbba..d3fbdadd3b 100755 --- a/tools/releasetools/sign_target_files_apks.py +++ b/tools/releasetools/sign_target_files_apks.py @@ -901,7 +901,7 @@ def WriteOtacerts(output_zip, filename, keys): certs_zip = zipfile.ZipFile(temp_file, "w", allowZip64=True) for k in keys: common.ZipWrite(certs_zip, k) - certs_zip.close() + common.ZipClose(certs_zip) common.ZipWriteStr(output_zip, filename, temp_file.getvalue()) @@ -1529,8 +1529,8 @@ def main(argv): platform_api_level, codename_to_api_level_map, compressed_extension) - input_zip.close() - output_zip.close() + common.ZipClose(input_zip) + common.ZipClose(output_zip) if OPTIONS.vendor_partitions and OPTIONS.vendor_otatools: BuildVendorPartitions(args[1]) diff --git a/tools/releasetools/test_common.py b/tools/releasetools/test_common.py index 8c9655ad0d..2a0e592c47 100644 --- a/tools/releasetools/test_common.py +++ b/tools/releasetools/test_common.py @@ -222,17 +222,17 @@ class BuildInfoTest(test_utils.ReleaseToolsTestCase): info_dict = copy.deepcopy(self.TEST_INFO_FINGERPRINT_DICT) build_info = common.BuildInfo(info_dict) self.assertEqual( - 'product-brand/product-name/product-device:version-release/build-id/' - 'version-incremental:build-type/build-tags', build_info.fingerprint) + 'product-brand/product-name/product-device:version-release/build-id/' + 'version-incremental:build-type/build-tags', build_info.fingerprint) build_props = info_dict['build.prop'].build_props del build_props['ro.build.id'] build_props['ro.build.legacy.id'] = 'legacy-build-id' build_info = common.BuildInfo(info_dict, use_legacy_id=True) self.assertEqual( - 'product-brand/product-name/product-device:version-release/' - 'legacy-build-id/version-incremental:build-type/build-tags', - build_info.fingerprint) + 'product-brand/product-name/product-device:version-release/' + 'legacy-build-id/version-incremental:build-type/build-tags', + build_info.fingerprint) self.assertRaises(common.ExternalError, common.BuildInfo, info_dict, None, False) @@ -241,9 +241,9 @@ class BuildInfoTest(test_utils.ReleaseToolsTestCase): info_dict['vbmeta_digest'] = 'abcde12345' build_info = common.BuildInfo(info_dict, use_legacy_id=False) self.assertEqual( - 'product-brand/product-name/product-device:version-release/' - 'legacy-build-id.abcde123/version-incremental:build-type/build-tags', - build_info.fingerprint) + 'product-brand/product-name/product-device:version-release/' + 'legacy-build-id.abcde123/version-incremental:build-type/build-tags', + build_info.fingerprint) def test___getitem__(self): target_info = common.BuildInfo(self.TEST_INFO_DICT, None) @@ -376,7 +376,7 @@ class BuildInfoTest(test_utils.ReleaseToolsTestCase): info_dict['build.prop'].build_props[ 'ro.product.property_source_order'] = 'bad-source' with self.assertRaisesRegexp(common.ExternalError, - 'Invalid ro.product.property_source_order'): + 'Invalid ro.product.property_source_order'): info = common.BuildInfo(info_dict, None) info.GetBuildProp('ro.product.device') @@ -459,7 +459,7 @@ class CommonZipTest(test_utils.ReleaseToolsTestCase): time.sleep(5) # Make sure the atime/mtime will change measurably. common.ZipWrite(zip_file, test_file_name, **extra_zipwrite_args) - zip_file.close() + common.ZipClose(zip_file) self._verify(zip_file, zip_file_name, arcname, sha1_hash.hexdigest(), test_file_name, expected_stat, expected_mode, @@ -494,7 +494,7 @@ class CommonZipTest(test_utils.ReleaseToolsTestCase): expected_mode = extra_args.get("perms", zinfo_perms) common.ZipWriteStr(zip_file, zinfo_or_arcname, contents, **extra_args) - zip_file.close() + common.ZipClose(zip_file) self._verify(zip_file, zip_file_name, arcname, sha1(contents).hexdigest(), expected_mode=expected_mode, @@ -536,7 +536,7 @@ class CommonZipTest(test_utils.ReleaseToolsTestCase): common.ZipWrite(zip_file, test_file_name, **extra_args) common.ZipWriteStr(zip_file, arcname_small, small, **extra_args) - zip_file.close() + common.ZipClose(zip_file) # Verify the contents written by ZipWrite(). self._verify(zip_file, zip_file_name, arcname_large, @@ -551,6 +551,12 @@ class CommonZipTest(test_utils.ReleaseToolsTestCase): os.remove(zip_file_name) os.remove(test_file_name) + def _test_reset_ZIP64_LIMIT(self, func, *args): + default_limit = (1 << 31) - 1 + self.assertEqual(default_limit, zipfile.ZIP64_LIMIT) + func(*args) + self.assertEqual(default_limit, zipfile.ZIP64_LIMIT) + def test_ZipWrite(self): file_contents = os.urandom(1024) self._test_ZipWrite(file_contents) @@ -575,7 +581,7 @@ class CommonZipTest(test_utils.ReleaseToolsTestCase): }) def test_ZipWrite_resets_ZIP64_LIMIT(self): - self._test_ZipWrite("") + self._test_reset_ZIP64_LIMIT(self._test_ZipWrite, "") def test_ZipWriteStr(self): random_string = os.urandom(1024) @@ -626,9 +632,9 @@ class CommonZipTest(test_utils.ReleaseToolsTestCase): }) def test_ZipWriteStr_resets_ZIP64_LIMIT(self): - self._test_ZipWriteStr('foo', b'') + self._test_reset_ZIP64_LIMIT(self._test_ZipWriteStr, 'foo', b'') zinfo = zipfile.ZipInfo(filename="foo") - self._test_ZipWriteStr(zinfo, b'') + self._test_reset_ZIP64_LIMIT(self._test_ZipWriteStr, zinfo, b'') def test_bug21309935(self): zip_file = tempfile.NamedTemporaryFile(delete=False) @@ -650,7 +656,7 @@ class CommonZipTest(test_utils.ReleaseToolsTestCase): zinfo = zipfile.ZipInfo(filename="qux") zinfo.external_attr = 0o700 << 16 common.ZipWriteStr(zip_file, zinfo, random_string, perms=0o400) - zip_file.close() + common.ZipClose(zip_file) self._verify(zip_file, zip_file_name, "foo", sha1(random_string).hexdigest(), @@ -677,7 +683,7 @@ class CommonZipTest(test_utils.ReleaseToolsTestCase): common.ZipWrite(output_zip, entry_file.name, arcname='Test1') common.ZipWrite(output_zip, entry_file.name, arcname='Test2') common.ZipWrite(output_zip, entry_file.name, arcname='Test3') - output_zip.close() + common.ZipClose(output_zip) zip_file.close() try: @@ -725,8 +731,8 @@ class CommonZipTest(test_utils.ReleaseToolsTestCase): common.ZipWrite(output_zip, entry_file.name, arcname='Foo3') common.ZipWrite(output_zip, entry_file.name, arcname='Bar4') common.ZipWrite(output_zip, entry_file.name, arcname='Dir5/Baz5') - output_zip.close() - output_zip.close() + common.ZipClose(output_zip) + common.ZipClose(output_zip) return zip_file @test_utils.SkipIfExternalToolsUnavailable() @@ -813,9 +819,9 @@ class CommonApkUtilsTest(test_utils.ReleaseToolsTestCase): ) APKCERTS_CERTMAP1 = { - 'RecoveryLocalizer.apk': 'certs/devkey', - 'Settings.apk': 'build/make/target/product/security/platform', - 'TV.apk': 'PRESIGNED', + 'RecoveryLocalizer.apk' : 'certs/devkey', + 'Settings.apk' : 'build/make/target/product/security/platform', + 'TV.apk' : 'PRESIGNED', } APKCERTS_TXT2 = ( @@ -830,10 +836,10 @@ class CommonApkUtilsTest(test_utils.ReleaseToolsTestCase): ) APKCERTS_CERTMAP2 = { - 'Compressed1.apk': 'certs/compressed1', - 'Compressed2a.apk': 'certs/compressed2', - 'Compressed2b.apk': 'certs/compressed2', - 'Compressed3.apk': 'certs/compressed3', + 'Compressed1.apk' : 'certs/compressed1', + 'Compressed2a.apk' : 'certs/compressed2', + 'Compressed2b.apk' : 'certs/compressed2', + 'Compressed3.apk' : 'certs/compressed3', } APKCERTS_TXT3 = ( @@ -842,7 +848,7 @@ class CommonApkUtilsTest(test_utils.ReleaseToolsTestCase): ) APKCERTS_CERTMAP3 = { - 'Compressed4.apk': 'certs/compressed4', + 'Compressed4.apk' : 'certs/compressed4', } # Test parsing with no optional fields, both optional fields, and only the @@ -859,9 +865,9 @@ class CommonApkUtilsTest(test_utils.ReleaseToolsTestCase): ) APKCERTS_CERTMAP4 = { - 'RecoveryLocalizer.apk': 'certs/devkey', - 'Settings.apk': 'build/make/target/product/security/platform', - 'TV.apk': 'PRESIGNED', + 'RecoveryLocalizer.apk' : 'certs/devkey', + 'Settings.apk' : 'build/make/target/product/security/platform', + 'TV.apk' : 'PRESIGNED', } def setUp(self): @@ -965,7 +971,7 @@ class CommonApkUtilsTest(test_utils.ReleaseToolsTestCase): extracted_from_privkey = common.ExtractAvbPublicKey('avbtool', privkey) extracted_from_pubkey = common.ExtractAvbPublicKey('avbtool', pubkey) with open(extracted_from_privkey, 'rb') as privkey_fp, \ - open(extracted_from_pubkey, 'rb') as pubkey_fp: + open(extracted_from_pubkey, 'rb') as pubkey_fp: self.assertEqual(privkey_fp.read(), pubkey_fp.read()) def test_ParseCertificate(self): @@ -1229,8 +1235,7 @@ class CommonUtilsTest(test_utils.ReleaseToolsTestCase): self.assertEqual( '1-5 9-10', sparse_image.file_map['//system/file1'].extra['text_str']) - self.assertTrue( - sparse_image.file_map['//system/file2'].extra['incomplete']) + self.assertTrue(sparse_image.file_map['//system/file2'].extra['incomplete']) self.assertTrue( sparse_image.file_map['/system/app/file3'].extra['incomplete']) @@ -1338,7 +1343,7 @@ class CommonUtilsTest(test_utils.ReleaseToolsTestCase): 'recovery_api_version': 3, 'fstab_version': 2, 'system_root_image': 'true', - 'no_recovery': 'true', + 'no_recovery' : 'true', 'recovery_as_boot': 'true', } @@ -1659,7 +1664,6 @@ class CommonUtilsTest(test_utils.ReleaseToolsTestCase): self.assertRaises(common.ExternalError, common._GenerateGkiCertificate, test_file.name, 'generic_kernel') - class InstallRecoveryScriptFormatTest(test_utils.ReleaseToolsTestCase): """Checks the format of install-recovery.sh. @@ -1669,7 +1673,7 @@ class InstallRecoveryScriptFormatTest(test_utils.ReleaseToolsTestCase): def setUp(self): self._tempdir = common.MakeTempDir() # Create a fake dict that contains the fstab info for boot&recovery. - self._info = {"fstab": {}} + self._info = {"fstab" : {}} fake_fstab = [ "/dev/soc.0/by-name/boot /boot emmc defaults defaults", "/dev/soc.0/by-name/recovery /recovery emmc defaults defaults"] @@ -2016,11 +2020,11 @@ class PartitionBuildPropsTest(test_utils.ReleaseToolsTestCase): input_zip, 'odm', placeholder_values) self.assertEqual({ - 'ro.odm.build.date.utc': '1578430045', - 'ro.odm.build.fingerprint': - 'google/coral/coral:10/RP1A.200325.001/6337676:user/dev-keys', - 'ro.product.odm.device': 'coral', - 'ro.product.odm.name': 'product1', + 'ro.odm.build.date.utc': '1578430045', + 'ro.odm.build.fingerprint': + 'google/coral/coral:10/RP1A.200325.001/6337676:user/dev-keys', + 'ro.product.odm.device': 'coral', + 'ro.product.odm.name': 'product1', }, partition_props.build_props) with zipfile.ZipFile(input_file, 'r', allowZip64=True) as input_zip: @@ -2203,8 +2207,8 @@ class PartitionBuildPropsTest(test_utils.ReleaseToolsTestCase): copied_props = copy.deepcopy(partition_props) self.assertEqual({ - 'ro.odm.build.date.utc': '1578430045', - 'ro.odm.build.fingerprint': - 'google/coral/coral:10/RP1A.200325.001/6337676:user/dev-keys', - 'ro.product.odm.device': 'coral', + 'ro.odm.build.date.utc': '1578430045', + 'ro.odm.build.fingerprint': + 'google/coral/coral:10/RP1A.200325.001/6337676:user/dev-keys', + 'ro.product.odm.device': 'coral', }, copied_props.build_props)