diff --git a/docs/topics/api/reviewers.rst b/docs/topics/api/reviewers.rst index 085bdc4999..c355a54192 100644 --- a/docs/topics/api/reviewers.rst +++ b/docs/topics/api/reviewers.rst @@ -169,6 +169,7 @@ This endpoint allows you to browse through the contents of an Add-on version. :>json string file.content: Raw content of the requested file. :>json string file.selected_file: The selected file, either from the ``file`` parameter or the default (manifest.json, install.rdf or package.json for Add-ons as well as the XML file for search engines). :>json string|null file.download_url: The download url of the selected file or ``null`` in case of a directory. + :>json boolean uses_unknown_minified_code: Indicates that the selected file could be using minified code. :>json array file.entries[]: The complete file-tree of the extracted XPI. :>json int file.entries[].depth: Level of folder-tree depth, starting with 0. :>json string file.entries[].filename: The filename of the file. @@ -214,6 +215,7 @@ This endpoint allows you to compare two Add-on versions with each other. :>json int|null file.entries[].size: The size in bytes. :>json string|null file.entries[].modified: The exact time of the commit, should be equivalent with ``created``. :>json object|null diff: See the following output with inline comments for a complete description. + :>json boolean uses_unknown_minified_code: Indicates that the selected file in either the current or the parent version could be using minified code. Git patch we're talking about: diff --git a/src/olympia/reviewers/serializers.py b/src/olympia/reviewers/serializers.py index 2bb0faf4b7..c149b8ff58 100644 --- a/src/olympia/reviewers/serializers.py +++ b/src/olympia/reviewers/serializers.py @@ -2,6 +2,7 @@ import io import os import mimetypes import pathlib +import json from collections import OrderedDict from datetime import datetime @@ -27,7 +28,7 @@ from olympia.addons.models import AddonReviewerFlags from olympia.api.fields import ReverseChoiceField, SplitField from olympia.users.models import UserProfile from olympia.files.utils import get_sha256 -from olympia.files.models import File +from olympia.files.models import File, FileValidation from olympia.reviewers.models import CannedResponse from olympia.versions.models import Version from olympia.lib.git import AddonGitRepository, get_mime_type_for_blob @@ -50,13 +51,15 @@ class AddonReviewerFlagsSerializer(serializers.ModelSerializer): class FileEntriesSerializer(FileSerializer): content = serializers.SerializerMethodField() + uses_unknown_minified_code = serializers.SerializerMethodField() download_url = serializers.SerializerMethodField() entries = serializers.SerializerMethodField() selected_file = serializers.SerializerMethodField() class Meta: fields = FileSerializer.Meta.fields + ( - 'content', 'entries', 'selected_file', 'download_url' + 'content', 'entries', 'selected_file', 'download_url', + 'uses_unknown_minified_code' ) model = File @@ -213,6 +216,20 @@ class FileEntriesSerializer(FileSerializer): # more explanation. return '' + def get_uses_unknown_minified_code(self, obj): + try: + validation = obj.validation + except FileValidation.DoesNotExist: + # We don't have any idea about whether it could be minified or not + # so let's assume it's not for now. + return False + + validation_data = json.loads(validation.validation) + + prop = 'unknownMinifiedFiles' + minified_files = validation_data.get('metadata', {}).get(prop, []) + return self.get_selected_file(obj) in minified_files + def get_download_url(self, obj): commit = self._get_commit(obj) tree = self.repo.get_root_tree(commit) @@ -285,10 +302,12 @@ class FileEntriesDiffSerializer(FileEntriesSerializer): entries = serializers.SerializerMethodField() selected_file = serializers.SerializerMethodField() download_url = serializers.SerializerMethodField() + uses_unknown_minified_code = serializers.SerializerMethodField() class Meta: fields = FileSerializer.Meta.fields + ( - 'diff', 'entries', 'selected_file', 'download_url' + 'diff', 'entries', 'selected_file', 'download_url', + 'uses_unknown_minified_code' ) model = File @@ -384,6 +403,22 @@ class FileEntriesDiffSerializer(FileEntriesSerializer): return entries + def get_uses_unknown_minified_code(self, obj): + parent = self.context['parent_version'] + selected_file = self.get_selected_file(obj) + + for file in [parent.current_file, obj]: + try: + data = json.loads(file.validation.validation) + except FileValidation.DoesNotExist: + continue + + prop = 'unknownMinifiedFiles' + minified_files = data.get('metadata', {}).get(prop, []) + if selected_file in minified_files: + return True + return False + class AddonCompareVersionSerializer(AddonBrowseVersionSerializer): file = FileEntriesDiffSerializer(source='current_file') diff --git a/src/olympia/reviewers/tests/test_serializers.py b/src/olympia/reviewers/tests/test_serializers.py index 3eff84b586..13579419da 100644 --- a/src/olympia/reviewers/tests/test_serializers.py +++ b/src/olympia/reviewers/tests/test_serializers.py @@ -1,4 +1,6 @@ # -*- coding: utf-8 -*- +import json + from datetime import datetime from django.core.cache import cache @@ -11,6 +13,7 @@ from olympia import amo from olympia.amo.templatetags.jinja_helpers import absolutify from olympia.amo.tests import TestCase, addon_factory, version_factory from olympia.amo.urlresolvers import reverse +from olympia.files.models import FileValidation from olympia.lib.git import AddonGitRepository from olympia.lib.tests.test_git import apply_changes from olympia.reviewers.models import CannedResponse @@ -68,6 +71,7 @@ class TestFileEntriesSerializer(TestCase): 'filename': 'manifest.json' } )) + assert not data['uses_unknown_minified_code'] assert set(data['entries'].keys()) == { 'README.md', @@ -211,6 +215,24 @@ class TestFileEntriesSerializer(TestCase): '71d4122c0f2f78e089136602f88dbf590f2fa04bb5bc417454bf21446d6cb4f0') assert data['entries']['icons/LICENSE']['sha256'] is None + def test_uses_unknown_minified_code(self): + validation_data = { + 'metadata': { + 'unknownMinifiedFiles': ['content-script.js'] + } + } + + fobj = self.addon.current_version.current_file + + FileValidation.objects.create( + file=fobj, validation=json.dumps(validation_data)) + + data = self.serialize(fobj, file='content-script.js') + assert data['uses_unknown_minified_code'] + + data = self.serialize(fobj, file='background-script.js') + assert not data['uses_unknown_minified_code'] + class TestFileEntriesDiffSerializer(TestCase): def setUp(self): @@ -294,6 +316,7 @@ class TestFileEntriesDiffSerializer(TestCase): 'filename': 'manifest.json' } )) + assert not data['uses_unknown_minified_code'] assert set(data['entries'].keys()) == { 'manifest.json', 'README.md', 'test.txt'} @@ -301,6 +324,8 @@ class TestFileEntriesDiffSerializer(TestCase): # The API always renders a diff, even for unmodified files. assert data['diff'] is not None + assert not data['uses_unknown_minified_code'] + # Unmodified file manifest_data = data['entries']['manifest.json'] assert manifest_data['depth'] == 0 @@ -531,6 +556,54 @@ class TestFileEntriesDiffSerializer(TestCase): assert data['diff'] is not None + def test_uses_unknown_minified_code(self): + parent_version = self.addon.current_version + + new_version = version_factory( + addon=self.addon, file_kw={ + 'filename': 'webextension_no_id.xpi', + 'is_webextension': True, + } + ) + AddonGitRepository.extract_and_commit_from_version(new_version) + + file = self.addon.current_version.current_file + + validation_data = { + 'metadata': { + 'unknownMinifiedFiles': ['README.md'] + } + } + + # Let's create a validation for the parent but not the current file + # which will result in us notifying the frontend of a minified file + # as well + current_validation = FileValidation.objects.create( + file=file, validation=json.dumps(validation_data)) + + data = self.serialize( + file, parent_version=parent_version, file='README.md') + assert data['uses_unknown_minified_code'] + + data = self.serialize( + file, parent_version=parent_version, file='manifest.json') + assert not data['uses_unknown_minified_code'] + + current_validation.delete() + + # Creating a validation object for the current one works as well + FileValidation.objects.create( + file=parent_version.current_file, + validation=json.dumps(validation_data)) + + data = self.serialize( + file, parent_version=parent_version, file='README.md') + assert data['uses_unknown_minified_code'] + + data = self.serialize( + file, parent_version=parent_version, file='manifest.json') + assert not data['uses_unknown_minified_code'] + class TestAddonBrowseVersionSerializer(TestCase): def setUp(self):