diff --git a/.travis.yml b/.travis.yml index 19ed0fbedf5d..f522f58574bf 100644 --- a/.travis.yml +++ b/.travis.yml @@ -24,10 +24,10 @@ jobs: - { stage: python3working, python: 3.6, env: TOXENV=assets } - { stage: python3working, python: 3.6, env: TOXENV=es } - { stage: python3working, python: 3.6, env: TOXENV=addons-versions-and-files } + - { stage: python3working, python: 3.6, env: TOXENV=devhub } - { stage: python3working, python: 3.6, env: TOXENV=reviewers-and-zadmin } - { stage: python3working, python: 3.6, env: TOXENV=accounts-users-and-ratings } - { stage: python3working, python: 3.6, env: TOXENV=main } - - { stage: python3, python: 3.6, env: TOXENV=devhub } - { stage: python3, python: 3.6, env: TOXENV=amo-lib-locales-and-signing } env: diff --git a/src/olympia/addons/forms.py b/src/olympia/addons/forms.py index 762a1b2a7279..da4fa93e4138 100644 --- a/src/olympia/addons/forms.py +++ b/src/olympia/addons/forms.py @@ -5,6 +5,7 @@ from django.conf import settings from django.core.files.storage import default_storage as storage from django.forms.formsets import BaseFormSet, formset_factory +from django.utils.encoding import force_text from django.utils.translation import ugettext, ugettext_lazy as _, ungettext import six @@ -229,7 +230,7 @@ def clean_categories(self): 'You can have only {0} categories.', max_cat).format(max_cat)) - has_misc = filter(lambda x: x.misc, categories) + has_misc = list(filter(lambda x: x.misc, categories)) if has_misc and total > 1: raise forms.ValidationError(ugettext( 'The miscellaneous category cannot be combined with ' @@ -292,7 +293,7 @@ def icons(): dirs, files = storage.listdir(settings.ADDON_ICONS_DEFAULT_PATH) for fname in files: if b'32' in fname and b'default' not in fname: - icon_name = fname.split(b'-')[0] + icon_name = force_text(fname.split(b'-')[0]) icons.append(('icon/%s' % icon_name, icon_name)) return sorted(icons) diff --git a/src/olympia/devhub/file_validation_annotations.py b/src/olympia/devhub/file_validation_annotations.py index da97165ac0cc..c378f57fda4e 100644 --- a/src/olympia/devhub/file_validation_annotations.py +++ b/src/olympia/devhub/file_validation_annotations.py @@ -5,7 +5,6 @@ from xml.parsers.expat import ExpatError from django.utils.translation import ugettext -from django.utils.encoding import force_bytes from olympia import amo from olympia.lib.akismet.models import AkismetReport @@ -123,10 +122,8 @@ def annotate_search_plugin_validation(results, file_path, channel): return try: - # Requires bytes because defusedxml fails to detect - # unicode strings as filenames. - # https://gist.github.com/EnTeQuAk/25f99701d8b123f7611acd6ce0d5840b - dom = minidom.parse(force_bytes(file_path)) + with open(file_path, 'r') as file_obj: + dom = minidom.parse(file_obj) except DefusedXmlException: url = 'https://pypi.python.org/pypi/defusedxml/0.3#attack-vectors' insert_validation_message( diff --git a/src/olympia/devhub/forms.py b/src/olympia/devhub/forms.py index ad2d01142233..3f2b8793eba5 100644 --- a/src/olympia/devhub/forms.py +++ b/src/olympia/devhub/forms.py @@ -67,15 +67,15 @@ def clean(self): if any(self.errors): return # cleaned_data could be None if it's the empty extra form. - data = filter(None, [f.cleaned_data for f in self.forms - if not f.cleaned_data.get('DELETE', False)]) + data = list(filter(None, [f.cleaned_data for f in self.forms + if not f.cleaned_data.get('DELETE', False)])) if not any(d['role'] == amo.AUTHOR_ROLE_OWNER for d in data): raise forms.ValidationError( ugettext('Must have at least one owner.')) if not any(d['listed'] for d in data): raise forms.ValidationError( ugettext('At least one author must be listed.')) - users = [d['user'] for d in data] + users = [d['user'].id for d in data] if sorted(users) != sorted(set(users)): raise forms.ValidationError( ugettext('An author can only be listed once.')) @@ -313,7 +313,7 @@ def clean_source(self): 'Unsupported file type, please upload an archive ' 'file {extensions}.'.format( extensions=valid_extensions_string))) - except (zipfile.BadZipfile, tarfile.ReadError, IOError): + except (zipfile.BadZipfile, tarfile.ReadError, IOError, EOFError): raise forms.ValidationError( ugettext('Invalid or broken archive.')) return source @@ -420,9 +420,9 @@ def __init__(self, *args, **kwargs): self.fields['max'].queryset = qs.all() def clean(self): - min = self.cleaned_data.get('min') - max = self.cleaned_data.get('max') - if not (min and max and min.version_int <= max.version_int): + min_ = self.cleaned_data.get('min') + max_ = self.cleaned_data.get('max') + if not (min_ and max_ and min_.version_int <= max_.version_int): raise forms.ValidationError(ugettext('Invalid version range.')) return self.cleaned_data @@ -465,8 +465,8 @@ def clean(self): if any(self.errors): return - apps = filter(None, [f.cleaned_data for f in self.forms - if not f.cleaned_data.get('DELETE', False)]) + apps = list(filter(None, [f.cleaned_data for f in self.forms + if not f.cleaned_data.get('DELETE', False)])) if not apps: # At this point, we're raising a global error and re-displaying the @@ -653,7 +653,7 @@ def clean(self): super(CombinedNameSummaryCleanMixin, self).clean() name_summary_locales = set( list(self.cleaned_data.get('name', {}).keys()) + - self.cleaned_data.get('summary', {}).keys()) + list(self.cleaned_data.get('summary', {}).keys())) default_locale = self.instance.default_locale.lower() name_values = self.cleaned_data.get('name') or {} name_default = name_values.get(default_locale) or '' diff --git a/src/olympia/devhub/tasks.py b/src/olympia/devhub/tasks.py index 0395565b017f..2f98c7e3e3b4 100644 --- a/src/olympia/devhub/tasks.py +++ b/src/olympia/devhub/tasks.py @@ -17,6 +17,7 @@ from django.core.validators import ValidationError from django.db import transaction from django.template import loader +from django.utils.encoding import force_text from django.utils.translation import ugettext import celery @@ -161,7 +162,7 @@ def wrapper(task, akismet_results, id_or_path, **kwargs): task.ignore_result = True try: data = fn(id_or_path, **kwargs) - results = json.loads(data) + results = json.loads(force_text(data)) if akismet_results: annotations.annotate_akismet_spam_check( results, akismet_results) @@ -448,7 +449,7 @@ def run_addons_linter(path, channel): if error: raise ValueError(error) - parsed_data = json.loads(output) + parsed_data = json.loads(force_text(output)) result = json.dumps(fix_addons_linter_output(parsed_data, channel)) track_validation_stats(result) @@ -460,7 +461,7 @@ def track_validation_stats(json_result): """ Given a raw JSON string of validator results, log some stats. """ - result = json.loads(json_result) + result = json.loads(force_text(json_result)) result_kind = 'success' if result['errors'] == 0 else 'failure' statsd.incr('devhub.linter.results.all.{}'.format(result_kind)) diff --git a/src/olympia/devhub/templatetags/jinja_helpers.py b/src/olympia/devhub/templatetags/jinja_helpers.py index afc63979f49f..950bb12208ce 100644 --- a/src/olympia/devhub/templatetags/jinja_helpers.py +++ b/src/olympia/devhub/templatetags/jinja_helpers.py @@ -1,5 +1,3 @@ -import urllib - from collections import defaultdict from django.utils.encoding import force_bytes @@ -9,6 +7,7 @@ import six from django_jinja import library +from six.moves.urllib.parse import unquote_to_bytes from olympia import amo from olympia.access import acl @@ -120,7 +119,7 @@ def display_url(url): Note: returns a Unicode object, not a valid URL. """ url = force_bytes(url, errors='replace') - return urllib.unquote(url).decode('utf-8', errors='replace') + return unquote_to_bytes(url).decode('utf-8', errors='replace') @library.global_function diff --git a/src/olympia/devhub/tests/test_feeds.py b/src/olympia/devhub/tests/test_feeds.py index 3999ef6001f5..06ce05005ce0 100644 --- a/src/olympia/devhub/tests/test_feeds.py +++ b/src/olympia/devhub/tests/test_feeds.py @@ -1,5 +1,7 @@ import uuid +from django.utils.encoding import force_text + import six from six.moves.urllib_parse import urlencode @@ -176,34 +178,35 @@ def test_filter_addon_otherguy(self): def test_rss(self): self.log_creates(5) # This will give us a new RssKey - r = self.get_response() + self.get_response() key = RssKey.objects.get() assert isinstance(key.key, uuid.UUID) - r = self.get_response(privaterss=key.key) - assert r['content-type'] == 'application/rss+xml; charset=utf-8' - assert 'Recent Changes for My Add-ons' in r.content + res = self.get_response(privaterss=key.key) + assert res['content-type'] == 'application/rss+xml; charset=utf-8' + assert b'Recent Changes for My Add-ons' in res.content def test_rss_accepts_verbose(self): self.log_creates(5) - r = self.get_response() + self.get_response() key = RssKey.objects.get() - r = self.get_response(privaterss=str(key.key)) - assert r['content-type'] == 'application/rss+xml; charset=utf-8' - assert 'Recent Changes for My Add-ons' in r.content + res = self.get_response(privaterss=str(key.key)) + assert res['content-type'] == 'application/rss+xml; charset=utf-8' + assert b'Recent Changes for My Add-ons' in res.content def test_rss_single(self): self.log_creates(5) self.log_creates(13, self.addon2) # This will give us a new RssKey - r = self.get_response(addon=self.addon.id) + self.get_response(addon=self.addon.id) key = RssKey.objects.get() - r = self.get_response(privaterss=key.key) - assert r['content-type'] == 'application/rss+xml; charset=utf-8' - assert len(pq(r.content)('item')) == 5 - assert 'Recent Changes for %s' % self.addon in r.content + res = self.get_response(privaterss=key.key) + assert res['content-type'] == 'application/rss+xml; charset=utf-8' + assert len(pq(res.content)('item')) == 5 + assert 'Recent Changes for %s' % self.addon in ( + force_text(res.content)) def test_rss_unlisted_addon(self): """Unlisted addon logs appear in the rss feed.""" @@ -293,11 +296,11 @@ def test_hidden(self): res = self.get_response(addon=self.addon.id) key = RssKey.objects.get() res = self.get_response(privaterss=key.key) - assert "Comment on" not in res.content + assert b'<title>Comment on' not in res.content def test_no_guid(self): self.log_creates(1) self.get_response(addon=self.addon.id) key = RssKey.objects.get() res = self.get_response(privaterss=key.key) - assert "<guid>" not in res.content + assert b'<guid>' not in res.content diff --git a/src/olympia/devhub/tests/test_forms.py b/src/olympia/devhub/tests/test_forms.py index c2a08c4f14e9..c85790390335 100644 --- a/src/olympia/devhub/tests/test_forms.py +++ b/src/olympia/devhub/tests/test_forms.py @@ -6,6 +6,7 @@ from django.conf import settings from django.core.files.storage import default_storage as storage from django.utils import translation +from django.utils.encoding import force_text import mock import pytest @@ -255,8 +256,8 @@ def test_preview_modified(self, update_mock): name = 'transparent.png' form = forms.PreviewForm({'caption': 'test', 'upload_hash': name, 'position': 1}) - with storage.open(os.path.join(self.dest, name), 'w') as f: - shutil.copyfileobj(open(get_image_path(name)), f) + with storage.open(os.path.join(self.dest, name), 'wb') as f: + shutil.copyfileobj(open(get_image_path(name), 'rb'), f) assert form.is_valid() form.save(addon) assert update_mock.called @@ -267,8 +268,8 @@ def test_preview_size(self, pngcrush_image_mock): name = 'teamaddons.jpg' form = forms.PreviewForm({'caption': 'test', 'upload_hash': name, 'position': 1}) - with storage.open(os.path.join(self.dest, name), 'w') as f: - shutil.copyfileobj(open(get_image_path(name)), f) + with storage.open(os.path.join(self.dest, name), 'wb') as f: + shutil.copyfileobj(open(get_image_path(name), 'rb'), f) assert form.is_valid() form.save(addon) preview = addon.previews.all()[0] @@ -433,7 +434,8 @@ def test_success(self, save_persona_image_mock, # Upload header image. img = open(get_image_path('persona-header.jpg'), 'rb') r_ajax = self.client.post(header_url, {'upload_image': img}) - data.update(header_hash=json.loads(r_ajax.content)['upload_hash']) + content = json.loads(force_text(r_ajax.content)) + data.update(header_hash=content['upload_hash']) # Populate and save form. self.post() diff --git a/src/olympia/devhub/tests/test_helpers.py b/src/olympia/devhub/tests/test_helpers.py index e0e57cd1c96d..e76699d796fd 100644 --- a/src/olympia/devhub/tests/test_helpers.py +++ b/src/olympia/devhub/tests/test_helpers.py @@ -1,12 +1,12 @@ # -*- coding: utf-8 -*- -import urllib - from django.utils import translation +from django.utils.encoding import force_bytes, force_text import pytest import six from mock import Mock +from six.moves.urllib.parse import quote from olympia import amo from olympia.activity.models import ActivityLog @@ -72,17 +72,17 @@ class TestDisplayUrl(amo.tests.BaseTestCase): def setUp(self): super(TestDisplayUrl, self).setUp() - self.raw_url = u'http://host/%s' % 'フォクすけといっしょ'.decode('utf8') + self.raw_url = u'http://host/%s' % u'フォクすけといっしょ' def test_utf8(self): - url = urllib.quote(self.raw_url.encode('utf8')) - assert render('{{ url|display_url }}', {'url': url}) == ( + url = quote(self.raw_url.encode('utf8')) + assert render(u'{{ url|display_url }}', {'url': url}) == ( self.raw_url) def test_unicode(self): - url = urllib.quote(self.raw_url.encode('utf8')) - url = six.text_type(url, 'utf8') - assert render('{{ url|display_url }}', {'url': url}) == ( + url = quote(self.raw_url.encode('utf8')) + url = force_text(force_bytes(url, 'utf8'), 'utf8') + assert render(u'{{ url|display_url }}', {'url': url}) == ( self.raw_url) diff --git a/src/olympia/devhub/tests/test_tasks.py b/src/olympia/devhub/tests/test_tasks.py index 97dc5a5a314a..f670671d4583 100644 --- a/src/olympia/devhub/tests/test_tasks.py +++ b/src/olympia/devhub/tests/test_tasks.py @@ -75,7 +75,7 @@ def _uploader(resize_size, final_size): original_size = (339, 128) src = tempfile.NamedTemporaryFile( - mode='r+w+b', suffix='.png', delete=False, dir=settings.TMP_PATH) + mode='r+b', suffix='.png', delete=False, dir=settings.TMP_PATH) if not isinstance(final_size, list): final_size = [final_size] @@ -122,18 +122,20 @@ def test_recreate_previews(pngcrush_image_mock): addon = addon_factory() # Set up the preview so it has files in the right places. preview_no_original = Preview.objects.create(addon=addon) - with storage.open(preview_no_original.image_path, 'w') as dest: - shutil.copyfileobj(open(get_image_path('preview_landscape.jpg')), dest) - with storage.open(preview_no_original.thumbnail_path, 'w') as dest: - shutil.copyfileobj(open(get_image_path('mozilla.png')), dest) + with storage.open(preview_no_original.image_path, 'wb') as dest: + shutil.copyfileobj(open(get_image_path('preview_landscape.jpg'), 'rb'), + dest) + with storage.open(preview_no_original.thumbnail_path, 'wb') as dest: + shutil.copyfileobj(open(get_image_path('mozilla.png'), 'rb'), dest) # And again but this time with an "original" image. preview_has_original = Preview.objects.create(addon=addon) - with storage.open(preview_has_original.image_path, 'w') as dest: - shutil.copyfileobj(open(get_image_path('preview_landscape.jpg')), dest) - with storage.open(preview_has_original.thumbnail_path, 'w') as dest: - shutil.copyfileobj(open(get_image_path('mozilla.png')), dest) - with storage.open(preview_has_original.original_path, 'w') as dest: - shutil.copyfileobj(open(get_image_path('teamaddons.jpg')), dest) + with storage.open(preview_has_original.image_path, 'wb') as dest: + shutil.copyfileobj(open(get_image_path('preview_landscape.jpg'), 'rb'), + dest) + with storage.open(preview_has_original.thumbnail_path, 'wb') as dest: + shutil.copyfileobj(open(get_image_path('mozilla.png'), 'rb'), dest) + with storage.open(preview_has_original.original_path, 'wb') as dest: + shutil.copyfileobj(open(get_image_path('teamaddons.jpg'), 'rb'), dest) tasks.recreate_previews([addon.id]) diff --git a/src/olympia/devhub/tests/test_views.py b/src/olympia/devhub/tests/test_views.py index 6188b95a23b8..2b64da57d60c 100644 --- a/src/olympia/devhub/tests/test_views.py +++ b/src/olympia/devhub/tests/test_views.py @@ -9,6 +9,7 @@ from django.core.files.storage import default_storage as storage from django.test import RequestFactory from django.test.utils import override_settings +from django.utils.encoding import force_text from django.utils.translation import trim_whitespace import mock @@ -425,7 +426,7 @@ def test_counts(self): version=addon.current_version) url = reverse('devhub.versions.stats', args=[addon.slug]) - data = json.loads(self.client.get(url).content) + data = json.loads(force_text(self.client.get(url).content)) exp = {str(version.id): {'reviews': 10, 'files': 1, 'version': version.version, 'id': version.id}} @@ -531,7 +532,7 @@ def test_basic_logged_out(self): response = self.client.get(self.url) assert response.status_code == 200 self.assertTemplateUsed(response, 'devhub/index.html') - assert 'Customize Firefox' in response.content + assert b'Customize Firefox' in response.content def test_default_lang_selected(self): self.client.logout() @@ -543,7 +544,7 @@ def test_basic_logged_in(self): response = self.client.get(self.url) assert response.status_code == 200 self.assertTemplateUsed(response, 'devhub/index.html') - assert 'My Add-ons' in response.content + assert b'My Add-ons' in response.content def test_my_addons_addon_versions_link(self): assert self.client.login(email='del@icio.us') @@ -953,7 +954,7 @@ def test_detail_json(self): response = self.client.get(reverse('devhub.upload_detail', args=[upload.uuid.hex, 'json'])) assert response.status_code == 200 - data = json.loads(response.content) + data = json.loads(force_text(response.content)) assert data['validation']['errors'] == 1 assert data['url'] == ( @@ -1042,7 +1043,7 @@ def test_no_servererror_on_missing_version(self): upload = FileUpload.objects.get() response = self.client.get( reverse('devhub.upload_detail', args=[upload.uuid.hex, 'json'])) - data = json.loads(response.content) + data = json.loads(force_text(response.content)) message = [(m['message'], m.get('type') == 'error') for m in data['validation']['messages']] expected = [(u'"/version" is a required property', True)] @@ -1057,7 +1058,7 @@ def test_unparsable_xpi(self, flag_is_active, v): upload = FileUpload.objects.get() response = self.client.get( reverse('devhub.upload_detail', args=[upload.uuid.hex, 'json'])) - data = json.loads(response.content) + data = json.loads(force_text(response.content)) message = [(m['message'], m.get('fatal', False)) for m in data['validation']['messages']] assert message == [ @@ -1074,7 +1075,7 @@ def test_experiment_xpi_allowed(self, mock_validator): upload = FileUpload.objects.get() response = self.client.get(reverse('devhub.upload_detail', args=[upload.uuid.hex, 'json'])) - data = json.loads(response.content) + data = json.loads(force_text(response.content)) assert data['validation']['messages'] == [] @mock.patch('olympia.devhub.tasks.run_addons_linter') @@ -1085,7 +1086,7 @@ def test_experiment_xpi_not_allowed(self, mock_validator): upload = FileUpload.objects.get() response = self.client.get(reverse('devhub.upload_detail', args=[upload.uuid.hex, 'json'])) - data = json.loads(response.content) + data = json.loads(force_text(response.content)) assert data['validation']['messages'] == [ {u'tier': 1, u'message': u'You cannot submit this type of add-on', u'fatal': True, u'type': u'error'}] @@ -1100,7 +1101,7 @@ def test_system_addon_allowed(self, mock_validator): upload = FileUpload.objects.get() response = self.client.get(reverse('devhub.upload_detail', args=[upload.uuid.hex, 'json'])) - data = json.loads(response.content) + data = json.loads(force_text(response.content)) assert data['validation']['messages'] == [] @mock.patch('olympia.devhub.tasks.run_addons_linter') @@ -1113,7 +1114,7 @@ def test_system_addon_not_allowed_not_mozilla(self, mock_validator): upload = FileUpload.objects.get() response = self.client.get(reverse('devhub.upload_detail', args=[upload.uuid.hex, 'json'])) - data = json.loads(response.content) + data = json.loads(force_text(response.content)) assert data['validation']['messages'] == [ {u'tier': 1, u'message': u'You cannot submit an add-on with a guid ending ' @@ -1133,7 +1134,7 @@ def test_mozilla_signed_allowed(self, mock_get_signature, mock_validator): upload = FileUpload.objects.get() response = self.client.get(reverse('devhub.upload_detail', args=[upload.uuid.hex, 'json'])) - data = json.loads(response.content) + data = json.loads(force_text(response.content)) assert data['validation']['messages'] == [] @mock.patch('olympia.files.utils.get_signer_organizational_unit_name') @@ -1146,7 +1147,7 @@ def test_mozilla_signed_not_allowed_not_mozilla(self, mock_get_signature): upload = FileUpload.objects.get() response = self.client.get(reverse('devhub.upload_detail', args=[upload.uuid.hex, 'json'])) - data = json.loads(response.content) + data = json.loads(force_text(response.content)) assert data['validation']['messages'] == [ {u'tier': 1, u'message': u'You cannot submit a Mozilla Signed Extension', @@ -1172,7 +1173,7 @@ def test_legacy_mozilla_signed_fx57_compat_allowed(self): upload = FileUpload.objects.get() response = self.client.get(reverse('devhub.upload_detail', args=[upload.uuid.hex, 'json'])) - data = json.loads(response.content) + data = json.loads(force_text(response.content)) msg = data['validation']['messages'][0] @@ -1195,7 +1196,7 @@ def test_system_addon_update_allowed(self, mock_validator): upload = FileUpload.objects.get() response = self.client.get(reverse('devhub.upload_detail_for_version', args=[addon.slug, upload.uuid.hex])) - data = json.loads(response.content) + data = json.loads(force_text(response.content)) assert data['validation']['messages'] == [] def test_legacy_langpacks_disallowed(self): @@ -1204,7 +1205,7 @@ def test_legacy_langpacks_disallowed(self): upload = FileUpload.objects.get() response = self.client.get(reverse('devhub.upload_detail', args=[upload.uuid.hex, 'json'])) - data = json.loads(response.content) + data = json.loads(force_text(response.content)) assert data['validation']['messages'][0]['id'] == [ u'validation', u'messages', u'legacy_addons_unsupported' ] @@ -1246,7 +1247,7 @@ def test_akismet_reports_created_ham_outcome(self, comment_check_mock): report = AkismetReport.objects.get(upload_instance=upload) assert report.comment_type == 'product-name' assert report.comment == 'Beastify' # the addon's name - assert 'spam' not in response.content + assert b'spam' not in response.content @override_switch('akismet-spam-check', active=True) @override_switch('akismet-addon-action', active=False) @@ -1266,7 +1267,7 @@ def test_akismet_reports_created_spam_outcome_logging_only(self): assert report.comment_type == 'product-name' assert report.comment == 'Beastify' # the addon's name assert report.result == AkismetReport.MAYBE_SPAM - assert 'spam' not in response.content + assert b'spam' not in response.content @override_switch('akismet-spam-check', active=True) @override_switch('akismet-addon-action', active=True) @@ -1285,9 +1286,9 @@ def test_akismet_reports_created_spam_outcome_action_taken(self): report = AkismetReport.objects.get(upload_instance=upload) assert report.comment_type == 'product-name' assert report.comment == 'Beastify' # the addon's name - assert 'spam' in response.content + assert b'spam' in response.content assert report.result == AkismetReport.MAYBE_SPAM - data = json.loads(response.content) + data = json.loads(force_text(response.content)) assert data['validation']['messages'][0]['id'] == [ u'validation', u'messages', u'akismet_is_spam_name' ] @@ -1317,7 +1318,7 @@ def test_akismet_reports_created_l10n(self, comment_check_mock): u'Varsling ved trykk på lenke i18n', # nb_NO ] assert report.comment in names - assert 'spam' not in response.content + assert b'spam' not in response.content @override_switch('akismet-spam-check', active=True) def test_akismet_reports_not_created_for_unlisted(self): @@ -1446,8 +1447,8 @@ def test_unique_version_num(self): version='<script>alert("Happy XSS-Xmas");<script>') response = self.client.get(reverse('devhub.addons')) assert response.status_code == 200 - assert '<script>alert' not in response.content - assert '<script>alert' in response.content + assert b'<script>alert' not in response.content + assert b'<script>alert' in response.content class TestDeleteAddon(TestCase): @@ -1740,7 +1741,7 @@ def test_wrong_user(self): def test_no_header_image(self): response = self.client.post(self.url, follow=True) assert response.status_code == 200 - data = json.loads(response.content) + data = json.loads(force_text(response.content)) assert data == {} def test_header_image(self): @@ -1750,7 +1751,7 @@ def test_header_image(self): copy_stored_file(zip_file, destination) response = self.client.post(self.url, follow=True) assert response.status_code == 200 - data = json.loads(response.content) + data = json.loads(force_text(response.content)) assert data assert len(data.items()) == 1 assert 'weta.png' in data diff --git a/src/olympia/devhub/tests/test_views_edit.py b/src/olympia/devhub/tests/test_views_edit.py index 7eca7d377232..7d96649af5e8 100644 --- a/src/olympia/devhub/tests/test_views_edit.py +++ b/src/olympia/devhub/tests/test_views_edit.py @@ -4,6 +4,7 @@ from django.core.cache import cache from django.core.files.storage import default_storage as storage +from django.utils.encoding import force_text import mock import six @@ -345,7 +346,7 @@ def test_akismet_edit_is_ham(self, comment_check_mock): assert description_report.comment == data['description'] assert comment_check_mock.call_count == 3 - assert 'spam' not in response.content + assert b'spam' not in response.content # And metadata was updated addon = self.get_addon() @@ -377,7 +378,7 @@ def test_akismet_edit_is_spam_logging_only(self, comment_check_mock): assert comment_check_mock.call_count == 3 # But because we're not taking any action from the spam, don't report. - assert 'spam' not in response.content + assert b'spam' not in response.content # And metadata was updated addon = self.get_addon() @@ -446,9 +447,9 @@ def test_edit_xss(self): response = self.client.get(self.describe_edit_url) assert response.status_code == 200 - assert '<script>' not in response.content - assert ('This\n<b>IS</b><script>alert('awesome' - '')</script></textarea>') in response.content + assert b'<script>' not in response.content + assert (b'This\n<b>IS</b><script>alert('awesome' + b'')</script></textarea>') in response.content def test_description_optional(self): """Description is optional by default - so confirm that here and @@ -472,8 +473,8 @@ def test_description_min_length_not_in_html_attrs(self): def test_name_summary_lengths_short(self): # check the separate name and summary labels, etc are served response = self.client.get(self.url) - assert 'Name and Summary' not in response.content - assert 'It will be shown in listings and searches' in response.content + assert b'Name and Summary' not in response.content + assert b'It will be shown in listings and searches' in response.content self.client.post( self.describe_edit_url, self.get_dict(name='a', summary='b')) @@ -492,7 +493,7 @@ def test_name_summary_lengths_long(self): def test_name_summary_lengths_content_optimization(self): # check the combined name and summary label, etc are served response = self.client.get(self.url) - assert 'Name and Summary' in response.content + assert b'Name and Summary' in response.content # name and summary are too short response = self.client.post( @@ -625,8 +626,9 @@ def test_edit_no_previous_categories(self): # Make sure the categories list we display to the user in the response # has been updated. - assert set(response.context['addon'].all_categories) == set( - self.addon.all_categories) + assert ( + set(cat.id for cat in response.context['addon'].all_categories) == + set(cat.id for cat in self.addon.all_categories)) def test_edit_categories_addandremove(self): AddonCategory(addon=self.addon, category_id=1).save() @@ -641,8 +643,9 @@ def test_edit_categories_addandremove(self): # Make sure the categories list we display to the user in the response # has been updated. - assert set(response.context['addon'].all_categories) == set( - self.addon.all_categories) + assert ( + set(cat.id for cat in response.context['addon'].all_categories) == + set(cat.id for cat in self.addon.all_categories)) def test_edit_categories_remove(self): category = Category.objects.get(id=1) @@ -659,7 +662,7 @@ def test_edit_categories_remove(self): # Make sure the categories list we display to the user in the response # has been updated. - assert set(response.context['addon'].all_categories) == set( + assert response.context['addon'].all_categories == ( self.addon.all_categories) def test_edit_categories_required(self): @@ -928,7 +931,7 @@ def test_edit_media_uploadedicon(self): data = {'upload_image': src_image} response = self.client.post(self.icon_upload, data) - response_json = json.loads(response.content) + response_json = json.loads(force_text(response.content)) addon = self.get_addon() # Now, save the form so it gets moved properly. @@ -951,7 +954,7 @@ def test_edit_media_uploadedicon(self): # Check that it was actually uploaded dirname = os.path.join(user_media_path('addon_icons'), - '%s' % (addon.id / 1000)) + '%s' % (addon.id // 1000)) dest = os.path.join(dirname, '%s-32.png' % addon.id) assert storage.exists(dest) @@ -974,7 +977,7 @@ def test_edit_media_uploadedicon_noresize(self): data = {'upload_image': src_image} response = self.client.post(self.icon_upload, data) - response_json = json.loads(response.content) + response_json = json.loads(force_text(response.content)) addon = self.get_addon() # Now, save the form so it gets moved properly. @@ -997,7 +1000,7 @@ def test_edit_media_uploadedicon_noresize(self): # Check that it was actually uploaded dirname = os.path.join(user_media_path('addon_icons'), - '%s' % (addon.id / 1000)) + '%s' % (addon.id // 1000)) dest = os.path.join(dirname, '%s-64.png' % addon.id) assert storage.exists(dest) @@ -1012,7 +1015,7 @@ def check_image_type(self, url, msg): src_image = open(img, 'rb') res = self.client.post(url, {'upload_image': src_image}) - response_json = json.loads(res.content) + response_json = json.loads(force_text(res.content)) assert response_json['errors'][0] == msg def test_edit_media_icon_wrong_type(self): @@ -1042,55 +1045,55 @@ def test_image_status_no_choice(self): addon = self.get_addon() addon.update(icon_type='') url = reverse('devhub.ajax.image.status', args=[addon.slug]) - result = json.loads(self.client.get(url).content) + result = json.loads(force_text(self.client.get(url).content)) assert result['icons'] def test_image_status_works(self): self.setup_image_status() - result = json.loads(self.client.get(self.url).content) + result = json.loads(force_text(self.client.get(self.url).content)) assert result['icons'] def test_image_status_fails(self): self.setup_image_status() storage.delete(self.icon_dest) - result = json.loads(self.client.get(self.url).content) + result = json.loads(force_text(self.client.get(self.url).content)) assert not result['icons'] def test_preview_status_works(self): self.setup_image_status() - result = json.loads(self.client.get(self.url).content) + result = json.loads(force_text(self.client.get(self.url).content)) assert result['previews'] # No previews means that all the images are done. self.addon.previews.all().delete() - result = json.loads(self.client.get(self.url).content) + result = json.loads(force_text(self.client.get(self.url).content)) assert result['previews'] def test_preview_status_fails(self): self.setup_image_status() storage.delete(self.preview.thumbnail_path) - result = json.loads(self.client.get(self.url).content) + result = json.loads(force_text(self.client.get(self.url).content)) assert not result['previews'] def test_image_status_persona(self): self.setup_image_status() storage.delete(self.icon_dest) self.get_addon().update(type=amo.ADDON_PERSONA) - result = json.loads(self.client.get(self.url).content) + result = json.loads(force_text(self.client.get(self.url).content)) assert result['icons'] def test_image_status_default(self): self.setup_image_status() storage.delete(self.icon_dest) self.get_addon().update(icon_type='icon/photos') - result = json.loads(self.client.get(self.url).content) + result = json.loads(force_text(self.client.get(self.url).content)) assert result['icons'] def check_image_animated(self, url, msg): filehandle = open(get_image_path('animated.png'), 'rb') res = self.client.post(url, {'upload_image': filehandle}) - response_json = json.loads(res.content) + response_json = json.loads(force_text(res.content)) assert response_json['errors'][0] == msg def test_icon_animated(self): @@ -1110,27 +1113,29 @@ def test_icon_dimensions_and_ratio(self): response = self.client.post( self.icon_upload, {'upload_image': open(get_image_path('mozilla-small.png'), 'rb')}) - assert json.loads(response.content)['errors'] == [size_msg, ratio_msg] + assert json.loads(force_text(response.content))['errors'] == [ + size_msg, ratio_msg] # icon64.png is the right ratio, but only 64x64 response = self.client.post( self.icon_upload, {'upload_image': open( get_image_path('icon64.png'), 'rb')}) - assert json.loads(response.content)['errors'] == [size_msg] + assert json.loads(force_text(response.content))['errors'] == [size_msg] # mozilla.png is big enough but still not square response = self.client.post( self.icon_upload, {'upload_image': open(get_image_path('mozilla.png'), 'rb')}) - assert json.loads(response.content)['errors'] == [ratio_msg] + assert json.loads(force_text(response.content))['errors'] == [ + ratio_msg] # and mozilla-sq is the right ratio and big enough response = self.client.post( self.icon_upload, {'upload_image': open(get_image_path('mozilla-sq.png'), 'rb')}) - assert json.loads(response.content)['errors'] == [] - assert json.loads(response.content)['upload_hash'] + assert json.loads(force_text(response.content))['errors'] == [] + assert json.loads(force_text(response.content))['upload_hash'] def preview_add(self, amount=1, image_name='preview_4x3.jpg'): src_image = open(get_image_path(image_name), 'rb') @@ -1140,7 +1145,7 @@ def preview_add(self, amount=1, image_name='preview_4x3.jpg'): url = self.preview_upload response = self.client.post(url, data_formset) - details = json.loads(response.content) + details = json.loads(force_text(response.content)) upload_hash = details['upload_hash'] # Create and post with the formset. @@ -1175,27 +1180,29 @@ def test_preview_dimensions_and_ratio(self): response = self.client.post( self.preview_upload, {'upload_image': open(get_image_path('mozilla.png'), 'rb')}) - assert json.loads(response.content)['errors'] == [size_msg, ratio_msg] + assert json.loads(force_text(response.content))['errors'] == [ + size_msg, ratio_msg] # preview_landscape.jpg is the right ratio-ish, but too small response = self.client.post( self.preview_upload, {'upload_image': open( get_image_path('preview_landscape.jpg'), 'rb')}) - assert json.loads(response.content)['errors'] == [size_msg] + assert json.loads(force_text(response.content))['errors'] == [size_msg] # teamaddons.jpg is big enough but still wrong ratio. response = self.client.post( self.preview_upload, {'upload_image': open(get_image_path('teamaddons.jpg'), 'rb')}) - assert json.loads(response.content)['errors'] == [ratio_msg] + assert json.loads(force_text(response.content))['errors'] == [ + ratio_msg] # and preview_4x3.jpg is the right ratio and big enough response = self.client.post( self.preview_upload, {'upload_image': open(get_image_path('preview_4x3.jpg'), 'rb')}) - assert json.loads(response.content)['errors'] == [] - assert json.loads(response.content)['upload_hash'] + assert json.loads(force_text(response.content))['errors'] == [] + assert json.loads(force_text(response.content))['upload_hash'] def test_edit_media_preview_edit(self): self.preview_add() @@ -1657,7 +1664,7 @@ def test_edit_categories_set(self): assert [cat.id for cat in self.get_addon().all_categories] == [] response = self.client.post( self.describe_edit_url, self.get_dict(category='firefox')) - assert set(response.context['addon'].all_categories) == set( + assert response.context['addon'].all_categories == ( self.get_addon().all_categories) addon_cats = self.get_addon().categories.values_list('id', flat=True) diff --git a/src/olympia/devhub/tests/test_views_submit.py b/src/olympia/devhub/tests/test_views_submit.py index 048f30af3903..e1c7eb34ba58 100644 --- a/src/olympia/devhub/tests/test_views_submit.py +++ b/src/olympia/devhub/tests/test_views_submit.py @@ -106,7 +106,7 @@ def get_addon(self): def get_version(self): return self.get_addon().versions.latest() - def generate_source_zip(self, suffix='.zip', data='z' * (2 ** 21), + def generate_source_zip(self, suffix='.zip', data=u'z' * (2 ** 21), compression=zipfile.ZIP_DEFLATED): tdir = temp.gettempdir() source = temp.NamedTemporaryFile(suffix=suffix, dir=tdir) @@ -116,7 +116,7 @@ def generate_source_zip(self, suffix='.zip', data='z' * (2 ** 21), return source def generate_source_tar( - self, suffix='.tar.gz', data='t' * (2 ** 21), mode=None): + self, suffix='.tar.gz', data=b't' * (2 ** 21), mode=None): tdir = temp.gettempdir() source = temp.NamedTemporaryFile(suffix=suffix, dir=tdir) if mode is None: @@ -124,12 +124,12 @@ def generate_source_tar( with tarfile.open(fileobj=source, mode=mode) as tar_file: tar_info = tarfile.TarInfo('foo') tar_info.size = len(data) - tar_file.addfile(tar_info, six.StringIO(data)) + tar_file.addfile(tar_info, six.BytesIO(data)) source.seek(0) return source - def generate_source_garbage(self, suffix='.zip', data='g' * (2 ** 21)): + def generate_source_garbage(self, suffix='.zip', data=b'g' * (2 ** 21)): tdir = temp.gettempdir() source = temp.NamedTemporaryFile(suffix=suffix, dir=tdir) source.write(data) @@ -284,7 +284,7 @@ def test_check_agreement_okay(self): response = self.client.get(reverse('devhub.submit.distribution')) assert response.status_code == 200 # No error shown for a redirect from previous step. - assert 'This field is required' not in response.content + assert b'This field is required' not in response.content def test_submit_notification_warning(self): config = Config.objects.create( @@ -327,14 +327,14 @@ def test_unlisted_redirects_to_next_step(self): def test_channel_selection_error_shown(self): url = reverse('devhub.submit.distribution') # First load should have no error - assert 'This field is required' not in self.client.get(url).content + assert b'This field is required' not in self.client.get(url).content # Load with channel preselected (e.g. back from next step) - no error. - assert 'This field is required' not in self.client.get( + assert b'This field is required' not in self.client.get( url, args=['listed']).content # A post submission without channel selection should be an error - assert 'This field is required' in self.client.post(url).content + assert b'This field is required' in self.client.post(url).content class TestAddonSubmitUpload(UploadTest, TestCase): @@ -470,8 +470,8 @@ def test_default_supported_platforms(self): assert addon.current_version.supported_platforms == [amo.PLATFORM_ALL] # And check that compatible apps have a sensible default too - apps = addon.current_version.compatible_apps.keys() - assert sorted(apps) == sorted([amo.FIREFOX, amo.ANDROID]) + apps = [app.id for app in addon.current_version.compatible_apps.keys()] + assert sorted(apps) == sorted([amo.FIREFOX.id, amo.ANDROID.id]) @mock.patch('olympia.devhub.views.auto_sign_file') def test_one_xpi_for_multiple_apps_unlisted_addon( @@ -661,7 +661,7 @@ def test_submit_source(self): assert self.get_version().source assert self.addon.needs_admin_code_review mode = ( - oct(os.stat(self.get_version().source.path)[stat.ST_MODE])) + '0%o' % (os.stat(self.get_version().source.path)[stat.ST_MODE])) assert mode == '0100644' def test_submit_source_targz(self): @@ -672,7 +672,7 @@ def test_submit_source_targz(self): assert self.get_version().source assert self.addon.needs_admin_code_review mode = ( - oct(os.stat(self.get_version().source.path)[stat.ST_MODE])) + '0%o' % (os.stat(self.get_version().source.path)[stat.ST_MODE])) assert mode == '0100644' def test_submit_source_tgz(self): @@ -684,7 +684,7 @@ def test_submit_source_tgz(self): assert self.get_version().source assert self.addon.needs_admin_code_review mode = ( - oct(os.stat(self.get_version().source.path)[stat.ST_MODE])) + '0%o' % (os.stat(self.get_version().source.path)[stat.ST_MODE])) assert mode == '0100644' def test_submit_source_tarbz2(self): @@ -696,7 +696,7 @@ def test_submit_source_tarbz2(self): assert self.get_version().source assert self.addon.needs_admin_code_review mode = ( - oct(os.stat(self.get_version().source.path)[stat.ST_MODE])) + '0%o' % (os.stat(self.get_version().source.path)[stat.ST_MODE])) assert mode == '0100644' @override_settings(FILE_UPLOAD_MAX_MEMORY_SIZE=1) @@ -734,7 +734,7 @@ def test_submit_source_in_memory_upload(self): assert self.get_version().source assert self.addon.needs_admin_code_review mode = ( - oct(os.stat(self.get_version().source.path)[stat.ST_MODE])) + '0%o' % (os.stat(self.get_version().source.path)[stat.ST_MODE])) assert mode == '0100644' @override_settings(FILE_UPLOAD_MAX_MEMORY_SIZE=2 ** 22) @@ -748,7 +748,7 @@ def test_submit_source_in_memory_upload_with_targz(self): assert self.get_version().source assert self.addon.needs_admin_code_review mode = ( - oct(os.stat(self.get_version().source.path)[stat.ST_MODE])) + '0%o' % (os.stat(self.get_version().source.path)[stat.ST_MODE])) assert mode == '0100644' def test_with_bad_source_extension(self): @@ -790,7 +790,7 @@ def test_with_bad_source_not_an_actual_archive(self): def test_with_bad_source_broken_archive(self): source = self.generate_source_zip( data='Hello World', compression=zipfile.ZIP_STORED) - data = source.read().replace('Hello World', 'dlroW olleH') + data = source.read().replace(b'Hello World', b'dlroW olleH') source.seek(0) # First seek to rewrite from the beginning source.write(data) source.seek(0) # Second seek to reset like it's fresh. @@ -1064,7 +1064,7 @@ def test_akismet_spam_check_spam_logging_only(self, comment_check_mock): assert report.comment_type == 'product-name' assert report.comment == u'spám' assert text_type(self.addon.name) == u'spám' - assert 'spam' not in response.content + assert b'spam' not in response.content comment_check_mock.assert_called_once() @@ -1082,7 +1082,7 @@ def test_akismet_spam_check_ham(self, comment_check_mock): report = AkismetReport.objects.get() assert report.comment_type == 'product-name' assert report.comment == u'spám' - assert 'spam' not in response.content + assert b'spam' not in response.content @override_switch('akismet-spam-check', active=True) @mock.patch('olympia.lib.akismet.tasks.AkismetReport.comment_check') @@ -1097,8 +1097,8 @@ def test_akismet_spam_check_no_changes(self, comment_check_mock): def test_name_summary_lengths_short(self): # check the separate name and summary labels, etc are served response = self.client.get(self.url) - assert 'Name and Summary' not in response.content - assert 'It will be shown in listings and searches' in response.content + assert b'Name and Summary' not in response.content + assert b'It will be shown in listings and searches' in response.content data = self.get_dict(name='a', summary='b') self.is_success(data) @@ -1112,7 +1112,7 @@ def test_name_summary_lengths_long(self): def test_name_summary_lengths_content_optimization(self): # check the combined name and summary label, etc are served response = self.client.get(self.url) - assert 'Name and Summary' in response.content + assert b'Name and Summary' in response.content # name and summary are too short response = self.client.post( @@ -1721,7 +1721,7 @@ def test_finish_submitting_listed_static_theme(self): assert links[1].attrib['href'] == reverse('devhub.themes') # Text is static theme specific. - assert "This version will be available after it passes review." in ( + assert b'This version will be available after it passes review.' in ( response.content) # Show the preview we started generating just after the upload step. imgs = content('section.addon-submission-process img') @@ -1998,7 +1998,7 @@ def test_static_theme_wizard(self): assert doc('#theme-header').attr('data-existing-header') == ( 'header.png') # No warning about extra properties - assert 'are unsupported in this wizard' not in response.content + assert b'are unsupported in this wizard' not in response.content # And then check the upload works. path = os.path.join( @@ -2056,7 +2056,7 @@ def test_static_theme_wizard_unsupported_properties(self): assert doc('#accentcolor').attr('value') == '#123456' assert doc('#textcolor').attr('value') == 'rgba(1,2,3,0.4)' # Warning about extra properties this time: - assert 'are unsupported in this wizard' in response.content + assert b'are unsupported in this wizard' in response.content unsupported_list = doc('.notification-box.error ul.note li') assert unsupported_list.length == 3 assert 'tab_line' in unsupported_list.text() @@ -2301,8 +2301,8 @@ def test_show_request_for_information(self): user=self.user, details={'comments': 'this is an info request'}) response = self.client.get(self.url) assert response.status_code == 200 - assert 'this should not be shown' not in response.content - assert 'this is an info request' in response.content + assert b'this should not be shown' not in response.content + assert b'this is an info request' in response.content def test_dont_show_request_for_information_if_none_pending(self): ActivityLog.create( @@ -2313,8 +2313,8 @@ def test_dont_show_request_for_information_if_none_pending(self): user=self.user, details={'comments': 'this is an info request'}) response = self.client.get(self.url) assert response.status_code == 200 - assert 'this should not be shown' not in response.content - assert 'this is an info request' not in response.content + assert b'this should not be shown' not in response.content + assert b'this is an info request' not in response.content def test_clear_request_for_information(self): AddonReviewerFlags.objects.create( @@ -2474,7 +2474,7 @@ def test_akismet_spam_check_spam_logging_only(self, comment_check_mock): report = AkismetReport.objects.last() assert report.comment_type == 'product-summary' assert report.comment == u'Delicious Bookmarks is the official' - assert 'spam' not in response.content + assert b'spam' not in response.content assert comment_check_mock.call_count == 2 @@ -2498,7 +2498,7 @@ def test_akismet_spam_check_ham(self, comment_check_mock): report = AkismetReport.objects.last() assert report.comment_type == 'product-summary' assert report.comment == u'Delicious Bookmarks is the official' - assert 'spam' not in response.content + assert b'spam' not in response.content assert comment_check_mock.call_count == 2 @@ -2512,7 +2512,7 @@ def test_akismet_spam_check_no_changes(self, comment_check_mock): # No changes but both values were spam checked. assert AkismetReport.objects.count() == 2 - assert 'spam' not in response.content + assert b'spam' not in response.content assert comment_check_mock.call_count == 2 diff --git a/src/olympia/devhub/tests/test_views_validation.py b/src/olympia/devhub/tests/test_views_validation.py index 2e12559fccc9..5f9c82875cd8 100644 --- a/src/olympia/devhub/tests/test_views_validation.py +++ b/src/olympia/devhub/tests/test_views_validation.py @@ -49,7 +49,7 @@ def test_date_on_upload(self): def test_upload_processed_validation_error(self): addon_file = open( - 'src/olympia/devhub/tests/addons/invalid_webextension.xpi') + 'src/olympia/devhub/tests/addons/invalid_webextension.xpi', 'rb') response = self.client.post(reverse('devhub.upload'), {'name': 'addon.xpi', 'upload': addon_file}) @@ -119,8 +119,8 @@ def setUp(self): self.user = UserProfile.objects.get(email='del@icio.us') self.file_validation = FileValidation.objects.get(pk=1) self.file = self.file_validation.file - with storage.open(self.file.file_path, 'w') as f: - f.write('<pretend this is an xpi>\n') + with storage.open(self.file.file_path, 'wb') as f: + f.write(b'<pretend this is an xpi>\n') self.addon = self.file.version.addon args = [self.addon.slug, self.file.id] self.url = reverse('devhub.file_validation', args=args) @@ -229,7 +229,7 @@ def test_context_and_content(self): response = self.client.get(reverse('devhub.validate_addon')) assert response.status_code == 200 - assert 'this tool only works with legacy' not in response.content + assert b'this tool only works with legacy' not in response.content doc = pq(response.content) assert doc('#upload-addon').attr('data-upload-url') == ( @@ -246,13 +246,13 @@ def test_filename_not_uuidfied(self, validate_mock): fpath = 'src/olympia/files/fixtures/files/webextension_no_id.xpi' - with open(fpath) as file_: + with open(fpath, 'rb') as file_: self.client.post(url, {'upload': file_}) upload = FileUpload.objects.get() response = self.client.get( reverse('devhub.upload_detail', args=(upload.uuid.hex,))) - assert 'Validation Results for webextension_no_id' in response.content + assert b'Validation Results for webextension_no_id' in response.content @mock.patch('olympia.devhub.tasks.run_addons_linter') def test_upload_listed_addon(self, validate_mock): @@ -279,7 +279,7 @@ def test_upload_unlisted_addon(self, validate_mock): fpath = 'src/olympia/files/fixtures/files/webextension_no_id.xpi' - with open(fpath) as file_: + with open(fpath, 'rb') as file_: self.client.post(url, {'upload': file_}) assert ( @@ -338,7 +338,7 @@ def upload(self, view, **kw): 'src/olympia/files/fixtures/files/' 'webextension_validation_error.zip') - with open(fpath) as file_: + with open(fpath, 'rb') as file_: resp = self.client.post(reverse(view, kwargs=kw), {'upload': file_}) assert resp.status_code == 302 @@ -391,9 +391,9 @@ def setUp(self): self.user = UserProfile.objects.get(email='del@icio.us') self.file = File.objects.get(pk=100456) # Move the file into place as if it were a real file - with storage.open(self.file.file_path, 'w') as dest: + with storage.open(self.file.file_path, 'wb') as dest: fpath = self.file_fixture_path('webextension_validation_error.zip') - shutil.copyfileobj(open(fpath), dest) + shutil.copyfileobj(open(fpath, 'rb'), dest) self.addon = self.file.version.addon def tearDown(self): @@ -528,7 +528,7 @@ def test_linkify_validation_messages(self, v): def test_opensearch_validation(self): addon_file = open( - 'src/olympia/files/fixtures/files/opensearch/sp_no_url.xml') + 'src/olympia/files/fixtures/files/opensearch/sp_no_url.xml', 'rb') response = self.client.post( reverse('devhub.upload'), {'name': 'sp_no_url.xml', 'upload': addon_file}) diff --git a/src/olympia/devhub/tests/test_views_versions.py b/src/olympia/devhub/tests/test_views_versions.py index f2386581bc6f..15e4794cb598 100644 --- a/src/olympia/devhub/tests/test_views_versions.py +++ b/src/olympia/devhub/tests/test_views_versions.py @@ -786,8 +786,8 @@ def test_show_request_for_information(self): user=self.user, details={'comments': 'this is an info request'}) response = self.client.get(self.url) assert response.status_code == 200 - assert 'this should not be shown' not in response.content - assert 'this is an info request' in response.content + assert b'this should not be shown' not in response.content + assert b'this is an info request' in response.content def test_dont_show_request_for_information_if_none_pending(self): self.user = UserProfile.objects.latest('pk') @@ -799,8 +799,8 @@ def test_dont_show_request_for_information_if_none_pending(self): user=self.user, details={'comments': 'this is an info request'}) response = self.client.get(self.url) assert response.status_code == 200 - assert 'this should not be shown' not in response.content - assert 'this is an info request' not in response.content + assert b'this should not be shown' not in response.content + assert b'this is an info request' not in response.content def test_clear_request_for_information(self): AddonReviewerFlags.objects.create( @@ -901,8 +901,8 @@ def test_add_appversion(self): initial_count=1) response = self.client.post(self.url, data) assert response.status_code == 302 - apps = self.get_version().compatible_apps.keys() - assert sorted(apps) == sorted([amo.FIREFOX, amo.ANDROID]) + apps = [app.id for app in self.get_version().compatible_apps.keys()] + assert sorted(apps) == sorted([amo.FIREFOX.id, amo.ANDROID.id]) assert list(ActivityLog.objects.all().values_list('action')) == ( [(amo.LOG.MAX_APPVERSION_UPDATED.id,)]) @@ -944,13 +944,13 @@ def test_delete_appversion(self): # Add android compat so we can delete firefox. self.test_add_appversion() form = self.client.get(self.url).context['compat_form'] - data = map(initial, form.initial_forms) + data = list(map(initial, form.initial_forms)) data[0]['DELETE'] = True response = self.client.post( self.url, self.formset(*data, initial_count=2)) assert response.status_code == 302 - apps = self.get_version().compatible_apps.keys() - assert apps == [amo.ANDROID] + apps = [app.id for app in self.get_version().compatible_apps.keys()] + assert apps == [amo.ANDROID.id] assert list(ActivityLog.objects.all().values_list('action')) == ( [(amo.LOG.MAX_APPVERSION_UPDATED.id,)]) diff --git a/src/olympia/devhub/utils.py b/src/olympia/devhub/utils.py index 23031e27085b..7f455ce3dacb 100644 --- a/src/olympia/devhub/utils.py +++ b/src/olympia/devhub/utils.py @@ -211,7 +211,7 @@ def find_previous_version(addon, file, version_string, channel): vint = version_int(version_string) for file_ in qs.order_by('-id'): # Only accept versions which come before the one we're validating. - if file_.version.version_int < vint: + if (file_.version.version_int or 0) < vint: return file_ diff --git a/src/olympia/devhub/views.py b/src/olympia/devhub/views.py index ed9725467473..6cd0118ca85c 100644 --- a/src/olympia/devhub/views.py +++ b/src/olympia/devhub/views.py @@ -918,11 +918,11 @@ def ajax_upload_image(request, upload_type, addon_id=None): if is_icon: errors.append( ugettext('Please use images smaller than %dMB.') - % (max_size / 1024 / 1024)) + % (max_size // 1024 // 1024)) if is_persona: errors.append( ugettext('Images cannot be larger than %dKB.') - % (max_size / 1024)) + % (max_size // 1024)) if image_check.is_image() and is_persona: persona, img_type = upload_type.split('_') # 'header' or 'footer' @@ -938,9 +938,9 @@ def ajax_upload_image(request, upload_type, addon_id=None): if image_check.is_image() and content_waffle and is_preview: min_size = amo.ADDON_PREVIEW_SIZES.get('min') # * 100 to get a nice integer to compare against rather than 1.3333 - required_ratio = min_size[0] * 100 / min_size[1] + required_ratio = min_size[0] * 100 // min_size[1] actual_size = image_check.size - actual_ratio = actual_size[0] * 100 / actual_size[1] + actual_ratio = actual_size[0] * 100 // actual_size[1] if actual_size[0] < min_size[0] or actual_size[1] < min_size[1]: # L10n: {0} is an image width (in pixels), {1} is a height. errors.append( diff --git a/src/olympia/files/utils.py b/src/olympia/files/utils.py index 7bac866bfb5b..b5f60672d37e 100644 --- a/src/olympia/files/utils.py +++ b/src/olympia/files/utils.py @@ -862,16 +862,18 @@ def extract_extension_to_dest(source, dest=None, force_fsync=False): target = dest try: - if source.endswith(('.zip', '.xpi')): - zip_file = SafeZip(source, force_fsync=force_fsync) - zip_file.extract_to_dest(target) - elif source.endswith(('.tar.gz', '.tar.bz2', '.tgz')): + source = force_text(source) + if source.endswith((u'.zip', u'.xpi')): + with open(source, 'rb') as source_file: + zip_file = SafeZip(source_file, force_fsync=force_fsync) + zip_file.extract_to_dest(target) + elif source.endswith((u'.tar.gz', u'.tar.bz2', u'.tgz')): tarfile_class = ( tarfile.TarFile if not force_fsync else FSyncedTarFile) with tarfile_class.open(source) as archive: archive.extractall(target) - elif source.endswith('.xml'): + elif source.endswith(u'.xml'): shutil.copy(source, target) if force_fsync: FSyncMixin()._fsync_file(target)