diff --git a/.travis.yml b/.travis.yml index b50cdec3b2..2fe7f01396 100644 --- a/.travis.yml +++ b/.travis.yml @@ -2,7 +2,7 @@ language: python dist: xenial python: - - 3.6 + - 3.8 addons: apt: diff --git a/Dockerfile b/Dockerfile index a9e9d5695c..743e27ab4a 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,4 +1,4 @@ -FROM python:3.6-slim-buster +FROM python:3.8-slim-buster ENV PYTHONDONTWRITEBYTECODE=1 diff --git a/Dockerfile.deploy b/Dockerfile.deploy index dcec32abf1..bc38d81c1d 100644 --- a/Dockerfile.deploy +++ b/Dockerfile.deploy @@ -1,4 +1,4 @@ -FROM python:3.6-slim-buster +FROM python:3.8-slim-buster # need to compile swig ENV SWIG_FEATURES="-D__x86_64__" diff --git a/src/olympia/addons/tests/test_utils_.py b/src/olympia/addons/tests/test_utils_.py index c6e89a0297..ae8b8c55cd 100644 --- a/src/olympia/addons/tests/test_utils_.py +++ b/src/olympia/addons/tests/test_utils_.py @@ -1,17 +1,11 @@ # -*- coding: utf-8 -*- -import json from unittest import mock import pytest -import tempfile -import zipfile from django.conf import settings from django.forms import ValidationError -from django.utils.encoding import force_text -from olympia import amo from olympia.addons.utils import ( - build_webext_dictionary_from_legacy, get_addon_recommendations, get_addon_recommendations_invalid, is_outcome_recommended, TAAR_LITE_FALLBACK_REASON_EMPTY, TAAR_LITE_FALLBACK_REASON_TIMEOUT, @@ -19,7 +13,7 @@ from olympia.addons.utils import ( TAAR_LITE_OUTCOME_REAL_FAIL, TAAR_LITE_OUTCOME_REAL_SUCCESS, TAAR_LITE_FALLBACK_REASON_INVALID, verify_mozilla_trademark) -from olympia.amo.tests import AMOPaths, TestCase, addon_factory, user_factory +from olympia.amo.tests import TestCase, addon_factory, user_factory @pytest.mark.django_db @@ -126,70 +120,3 @@ class TestGetAddonRecommendations(TestCase): assert not is_outcome_recommended(TAAR_LITE_OUTCOME_REAL_FAIL) assert not is_outcome_recommended(TAAR_LITE_OUTCOME_CURATED) assert not self.recommendation_server_mock.called - - -class TestBuildWebextDictionaryFromLegacy(AMOPaths, TestCase): - def setUp(self): - self.addon = addon_factory( - target_locale='ar', type=amo.ADDON_DICT, - version_kw={'version': '1.0'}, - file_kw={'is_webextension': False}) - self.xpi_copy_over( - self.addon.current_version.all_files[0], 'dictionary-test.xpi') - - def check_xpi_file_contents(self, xpi_file_path, expected_version): - with zipfile.ZipFile(xpi_file_path, 'r', zipfile.ZIP_DEFLATED) as xpi: - # Check that manifest is present, contains proper version and - # dictionaries properties. - manifest = force_text(xpi.read('manifest.json')) - manifest_json = json.loads(manifest) - assert ( - manifest_json['browser_specific_settings']['gecko']['id'] == - self.addon.guid) - assert manifest_json['version'] == expected_version - expected_dict_obj = {'ar': 'dictionaries/ar.dic'} - assert manifest_json['dictionaries'] == expected_dict_obj - - # Check that we haven't included any useless files. - expected_files = sorted([ - 'dictionaries/', - 'dictionaries/ar.aff', - 'dictionaries/ar.dic', - 'dictionaries/license.txt', - 'manifest.json' - ]) - assert sorted([x.filename for x in xpi.filelist]) == expected_files - - def test_basic(self): - with tempfile.NamedTemporaryFile(suffix='.xpi') as destination: - build_webext_dictionary_from_legacy(self.addon, destination) - self.check_xpi_file_contents(destination, '1.0.1webext') - - def test_current_not_valid_raises(self): - mod = 'olympia.files.utils.SafeZip.initialize_and_validate' - with mock.patch(mod) as is_valid: - is_valid.return_value = False - with tempfile.NamedTemporaryFile(suffix='.xpi') as destination: - with self.assertRaises(ValidationError): - build_webext_dictionary_from_legacy( - self.addon, destination) - - def test_addon_has_no_target_locale(self): - self.addon.update(target_locale=None) - with tempfile.NamedTemporaryFile(suffix='.xpi') as destination: - build_webext_dictionary_from_legacy(self.addon, destination) - self.check_xpi_file_contents(destination, '1.0.1webext') - self.addon.reload() - - def test_invalid_dictionary_path_raises(self): - self.xpi_copy_over( - self.addon.current_version.all_files[0], 'extension.xpi') - with tempfile.NamedTemporaryFile(suffix='.xpi') as destination: - with self.assertRaises(ValidationError): - build_webext_dictionary_from_legacy(self.addon, destination) - - def test_version_number_typefix(self): - self.addon.current_version.update(version='1.1-typefix') - with tempfile.NamedTemporaryFile(suffix='.xpi') as destination: - build_webext_dictionary_from_legacy(self.addon, destination) - self.check_xpi_file_contents(destination, '1.2webext') diff --git a/src/olympia/addons/utils.py b/src/olympia/addons/utils.py index d6b2ec0162..bdec6f04aa 100644 --- a/src/olympia/addons/utils.py +++ b/src/olympia/addons/utils.py @@ -1,16 +1,11 @@ -import json -import os.path -import re import uuid -import zipfile from django import forms from django.conf import settings -from django.forms import ValidationError from django.utils.translation import ugettext from olympia import amo -from olympia.amo.utils import normalize_string, to_language +from olympia.amo.utils import normalize_string from olympia.discovery.utils import call_recommendation_server from olympia.translations.fields import LocaleErrorMessage @@ -96,86 +91,3 @@ def get_addon_recommendations_invalid(): return ( TAAR_LITE_FALLBACKS, TAAR_LITE_OUTCOME_REAL_FAIL, TAAR_LITE_FALLBACK_REASON_INVALID) - - -MULTIPLE_STOPS_REGEX = re.compile(r'\.{2,}') - - -def build_webext_dictionary_from_legacy(addon, destination): - """Create a webext package of a legacy dictionary `addon`, and put it in - `destination` path.""" - from olympia.files.utils import SafeZip # Avoid circular import. - old_path = addon.current_version.all_files[0].file_path - old_zip = SafeZip(old_path) - if not old_zip.is_valid: - raise ValidationError('Current dictionary xpi is not valid') - - dictionary_path = '' - - with zipfile.ZipFile(destination, 'w', zipfile.ZIP_DEFLATED) as new_zip: - for obj in old_zip.filelist: - splitted = obj.filename.split('/') - # Ignore useless directories and files. - if splitted[0] in ('META-INF', '__MACOSX', 'chrome', - 'chrome.manifest', 'install.rdf'): - continue - - # Also ignore javascript (regardless of where they are, not just at - # the root), since dictionaries should not contain any code. - if splitted[-1].endswith('.js'): - continue - - # Store the path of the last .dic file we find. It can be inside a - # directory. - if (splitted[-1].endswith('.dic')): - dictionary_path = obj.filename - - new_zip.writestr(obj.filename, old_zip.read(obj.filename)) - - # Now that all files we want from the old zip are copied, build and - # add manifest.json. - if not dictionary_path: - # This should not happen... It likely means it's an invalid - # dictionary to begin with, or one that has its .dic file in a - # chrome/ directory for some reason. Abort! - raise ValidationError('Current dictionary xpi has no .dic file') - - if addon.target_locale: - target_language = addon.target_locale - else: - # Guess target_locale since we don't have one already. Note that - # for extra confusion, target_locale is a language, not a locale. - target_language = to_language(os.path.splitext( - os.path.basename(dictionary_path))[0]) - if target_language not in settings.AMO_LANGUAGES: - # We couldn't find that language in the list we support. Let's - # try with just the prefix. - target_language = target_language.split('-')[0] - if target_language not in settings.AMO_LANGUAGES: - # We tried our best. - raise ValidationError(u'Addon has no target_locale and we' - u' could not guess one from the xpi') - - # Dumb version number increment. This will be invalid in some cases, - # but some of the dictionaries we have currently already have wild - # version numbers anyway. - version_number = addon.current_version.version - if version_number.endswith('.1-typefix'): - version_number = version_number.replace('.1-typefix', '.2webext') - else: - version_number = '%s.1webext' % version_number - - manifest = { - 'manifest_version': 2, - 'name': str(addon.name), - 'browser_specific_settings': { - 'gecko': { - 'id': addon.guid, - }, - }, - 'version': version_number, - 'dictionaries': {target_language: dictionary_path}, - } - - # Write manifest.json we just build. - new_zip.writestr('manifest.json', json.dumps(manifest)) diff --git a/src/olympia/devhub/forms.py b/src/olympia/devhub/forms.py index 327ce45535..c71030dfb0 100644 --- a/src/olympia/devhub/forms.py +++ b/src/olympia/devhub/forms.py @@ -748,7 +748,7 @@ class WithSourceMixin(object): zip_file = SafeZip(source) # testzip() returns None if there are no broken CRCs. if zip_file.zip_file.testzip() is not None: - raise zipfile.BadZipfile() + raise zipfile.BadZipFile() elif source.name.endswith(('.tar.gz', '.tar.bz2', '.tgz')): # For tar files we need to do a little more work. mode = 'r:bz2' if source.name.endswith('bz2') else 'r:gz' @@ -759,7 +759,7 @@ class WithSourceMixin(object): else: raise forms.ValidationError( self.get_invalid_source_file_type_message()) - except (zipfile.BadZipfile, tarfile.ReadError, IOError, EOFError): + except (zipfile.BadZipFile, tarfile.ReadError, IOError, EOFError): raise forms.ValidationError( ugettext('Invalid or broken archive.')) return source diff --git a/src/olympia/devhub/tasks.py b/src/olympia/devhub/tasks.py index 3137b33778..0333d797d8 100644 --- a/src/olympia/devhub/tasks.py +++ b/src/olympia/devhub/tasks.py @@ -9,7 +9,7 @@ import tempfile from copy import deepcopy from decimal import Decimal from functools import wraps -from zipfile import BadZipfile +from zipfile import BadZipFile from django.conf import settings from django.core.cache import cache @@ -180,11 +180,12 @@ def validation_task(fn): results, type_='error', message=exc.message, msg_id='unsupported_filetype') return results - except BadZipfile: + except BadZipFile: + # If we raised a BadZipFile we can return a single exception with + # a generic message indicating the zip is invalid or corrupt. results = deepcopy(amo.VALIDATOR_SKELETON_EXCEPTION_WEBEXT) - annotations.insert_validation_message( - results, type_='error', - message=ugettext('Invalid or corrupt add-on file.')) + results['messages'][0]['message'] = ugettext( + 'Invalid or corrupt add-on file.') return results except Exception as exc: log.exception('Unhandled error during validation: %r' % exc) @@ -395,7 +396,7 @@ def check_for_api_keys_in_file(results, upload_pk): revoke_api_key.apply_async( kwargs={'key_id': key.id}, countdown=120) zipfile.close() - except (ValidationError, BadZipfile, IOError): + except (ValidationError, BadZipFile, IOError): pass return results diff --git a/src/olympia/devhub/tests/test_views.py b/src/olympia/devhub/tests/test_views.py index aa342144fb..7fa0a0d558 100644 --- a/src/olympia/devhub/tests/test_views.py +++ b/src/olympia/devhub/tests/test_views.py @@ -1352,8 +1352,7 @@ class TestUploadDetail(BaseUploadTest): # We do raise a specific error message explaining that the archive is # not valid instead of a generic exception. assert message == [ - ('Invalid or corrupt add-on file.', False), - ('Sorry, we couldn\'t load your WebExtension.', True), + ('Invalid or corrupt add-on file.', True), ] @mock.patch('olympia.devhub.tasks.run_addons_linter') diff --git a/src/olympia/files/tests/test_file_viewer.py b/src/olympia/files/tests/test_file_viewer.py index 59cdad9470..f72c705b69 100644 --- a/src/olympia/files/tests/test_file_viewer.py +++ b/src/olympia/files/tests/test_file_viewer.py @@ -464,12 +464,11 @@ class TestSafeZipFile(TestCase, amo.tests.AMOPaths): SafeZip(self.xpi_path('langpack-localepicker')) def test_unzip_fatal(self): - with pytest.raises(zipfile.BadZipfile): + with pytest.raises(zipfile.BadZipFile): SafeZip(self.xpi_path('search.xml')) def test_read(self): zip_file = SafeZip(self.xpi_path('langpack-localepicker')) - assert zip_file.is_valid assert b'locale browser de' in zip_file.read('chrome.manifest') def test_not_secure(self): diff --git a/src/olympia/files/tests/test_utils.py b/src/olympia/files/tests/test_utils.py index d601140503..34d98a039f 100644 --- a/src/olympia/files/tests/test_utils.py +++ b/src/olympia/files/tests/test_utils.py @@ -1211,7 +1211,7 @@ def test_id_to_path(value, expected): class TestSafeZip(TestCase): def test_raises_error_for_invalid_webextension_xpi(self): - with pytest.raises(forms.ValidationError): + with pytest.raises(zipfile.BadZipFile): utils.SafeZip(get_addon_file('invalid_webextension.xpi')) def test_raises_validation_error_when_uncompressed_size_is_too_large(self): diff --git a/src/olympia/files/utils.py b/src/olympia/files/utils.py index 5125affd13..d2984de04e 100644 --- a/src/olympia/files/utils.py +++ b/src/olympia/files/utils.py @@ -744,16 +744,12 @@ class SafeZip(object): self.info_list = None self.mode = mode self.force_fsync = force_fsync - self.is_valid = self.initialize_and_validate() + self.initialize_and_validate() def initialize_and_validate(self): """ Runs some overall archive checks. """ - # Shortcut to avoid expensive check over and over again - if getattr(self, 'is_valid', False): - return True - if self.force_fsync: zip_file = FSyncedZipFile(self.source, self.mode) else: @@ -772,7 +768,6 @@ class SafeZip(object): self.info_list = info_list self.zip_file = zip_file - return True def is_signed(self): """Tells us if an addon is signed.""" @@ -870,6 +865,7 @@ def extract_extension_to_dest(source, dest=None, force_fsync=False): :returns: Extraction target directory, if `dest` is `None` it'll be a temporary directory. :raises FileNotFoundError: if the source file is not found on the filestem + :raises forms.ValidationError: if the zip is invalid """ target, tempdir = None, None @@ -894,14 +890,17 @@ def extract_extension_to_dest(source, dest=None, force_fsync=False): shutil.copy(source, target) if force_fsync: FSyncMixin()._fsync_file(target) - except (zipfile.BadZipfile, tarfile.ReadError, IOError) as e: + except (zipfile.BadZipFile, tarfile.ReadError, IOError, + forms.ValidationError) as e: if tempdir is not None: rm_local_tmp_dir(tempdir) - if isinstance(e, FileNotFoundError): + if isinstance(e, (FileNotFoundError, forms.ValidationError)): # We let FileNotFoundError (which are a subclass of IOError, or - # rather OSError but that's an alias) be raised, the caller will - # have to deal with it. + # rather OSError but that's an alias) and ValidationError be + # raised, the caller will have to deal with it. raise + # Any other exceptions we caught, we raise a generic ValidationError + # instead. raise forms.ValidationError( ugettext('Invalid or broken archive.')) return target diff --git a/src/olympia/lib/crypto/signing.py b/src/olympia/lib/crypto/signing.py index 2c2e3ec20d..0b6d2f2810 100644 --- a/src/olympia/lib/crypto/signing.py +++ b/src/olympia/lib/crypto/signing.py @@ -210,7 +210,7 @@ def is_signed(file_path): try: with zipfile.ZipFile(file_path, mode='r') as zf: filenames = set(zf.namelist()) - except (zipfile.BadZipfile, IOError): + except (zipfile.BadZipFile, IOError): filenames = set() return set([u'META-INF/mozilla.rsa', u'META-INF/mozilla.sf', u'META-INF/manifest.mf']).issubset(filenames) diff --git a/src/olympia/lib/jingo_minify_helpers.py b/src/olympia/lib/jingo_minify_helpers.py index d758a129db..6b911472bf 100644 --- a/src/olympia/lib/jingo_minify_helpers.py +++ b/src/olympia/lib/jingo_minify_helpers.py @@ -1,3 +1,4 @@ +import errno import os import subprocess import time @@ -51,7 +52,7 @@ def ensure_path_exists(path): os.makedirs(os.path.dirname(path)) except OSError as e: # If the directory already exists, that is fine. Otherwise re-raise. - if e.errno != os.errno.EEXIST: + if e.errno != errno.EEXIST: raise return path