Block version control; too much garbage (bug 918280)

appvalidator/constants.py

@@ -26,6 +26,8 @@ MAX_RESOURCE_SIZE = 2 * 1024 * 1024
 
 ICON_LIMIT = 10
 
+MAX_GARBAGE = 100 * 1024
+
 PERMISSIONS = {
     'web': set([
         'geolocation', 'alarms', 'fmradio', 'desktop-notification', 'storage',

appvalidator/testcases/content.py

@@ -24,51 +24,50 @@ def test_packed_packages(err, package=None):
         return
 
     processed_files = 0
-    pretested_files = err.get_resource("pretested_files") or []
+
+    garbage_files = 0
 
     # Iterate each item in the package.
     for name in package:
+        file_info = package.info(name)
+        file_name = file_info["name_lower"]
+        file_size = file_info["size"]
 
-        # Warn for things like __MACOSX directories and .old files.
-        if ("__MACOSX" in name or
-            name.split("/")[-1].startswith(".")):
+        if "__MACOSX" in name or file_name[0] in (".", "_", ):
             err.warning(
                 err_id=("testcases_content", "test_packed_packages",
                         "hidden_files"),
-                warning="Hidden files and folders flagged",
+                warning="Unused files or directories flagged.",
                 description="Hidden files and folders can make the review process "
                             "difficult and may contain sensitive information "
                             "about the system that generated the zip. Please "
                             "modify the packaging process so that these files "
                             "aren't included.",
                 filename=name)
+            garbage_files += file_size
             continue
         elif (any(name.endswith(ext) for ext in FLAGGED_EXTENSIONS) or
               name in FLAGGED_FILES):
             err.warning(
                 err_id=("testcases_content", "test_packaged_packages",
                         "flagged_files"),
-                warning="Flagged filename found",
+                warning="Garbage file detected",
                 description="Files were found that are either unnecessary "
                             "or have been included unintentionally. They "
                             "should be removed.",
                 filename=name)
-            continue
-
-        # Skip the file if it's in the pre-tested file resources.
-        if name in pretested_files:
+            garbage_files += file_size
             continue
 
         # Read the file from the archive if possible.
         file_data = u""
         try:
             file_data = package.read(name)
-        except KeyError:  # pragma: no cover
+        except KeyError:
             pass
 
         # Skip over whitelisted hashes unless we are checking for compatibility.
-        hash = hashlib.sha1(file_data).hexdigest()
-        if hash in hash_blacklist:
+        if hashlib.sha1(file_data).hexdigest() in hash_blacklist:
             continue
 
         # Process the file.
@@ -82,6 +81,16 @@ def test_packed_packages(err, package=None):
         # This aids in creating unit tests.
         processed_files += 1
 
+    if garbage_files >= MAX_GARBAGE:
+        err.error(
+            err_id=("testcases_content", "garbage"),
+            error="Too much garbage in package",
+            description="Your app contains too many unused or garbage files. "
+                        "These include temporary files, 'dot files', IDE and "
+                        "editor backup and configuration, and operating "
+                        "system hidden files. They must be removed before "
+                        "your app can be submitted.")
+
     return processed_files
 
 
@@ -90,24 +99,23 @@ def _process_file(err, package, name, file_data):
 
     name_lower = name.lower()
 
-    if name_lower.endswith((".css", ".js", ".xml", ".html", ".xhtml")):
+    if not name_lower.endswith((".css", ".js", ".xml", ".html", ".xhtml")):
+        return False
 
-        if not file_data:
-            return None
+    if not file_data:
+        return None
 
-        # Convert the file data to unicode
-        file_data = unicodehelper.decode(file_data)
+    # Convert the file data to unicode
+    file_data = unicodehelper.decode(file_data)
 
-        if name_lower.endswith(".css"):
-            testendpoint_css.test_css_file(err, name, file_data)
+    if name_lower.endswith(".css"):
+        testendpoint_css.test_css_file(err, name, file_data)
 
-        elif name_lower.endswith(".js"):
-            testendpoint_js.test_js_file(err, name, file_data)
+    elif name_lower.endswith(".js"):
+        testendpoint_js.test_js_file(err, name, file_data)
 
-        elif name_lower.endswith((".xml", ".html", ".xhtml")):
-            p = testendpoint_markup.MarkupParser(err)
-            p.process(name, file_data, package.info(name)["extension"])
+    elif name_lower.endswith((".xml", ".html", ".xhtml")):
+        p = testendpoint_markup.MarkupParser(err)
+        p.process(name, file_data, package.info(name)["extension"])
 
-        return True
-
-    return False
+    return True

appvalidator/testcases/packagelayout.py

@@ -17,6 +17,8 @@ blacklisted_magic_numbers = (
         (0x43, 0x57, 0x53),  # ZLIB compressed SWF
 )
 
+VC_DIRS = (".git", ".svn", )
+
 
 @register_test(tier=1)
 def test_blacklisted_files(err, package=None):
@@ -26,6 +28,7 @@ def test_blacklisted_files(err, package=None):
         return
 
     flagged_files = []
+    flagged_for_vc = False
 
     for name in package:
         file_ = package.info(name)
@@ -50,6 +53,22 @@ def test_blacklisted_files(err, package=None):
             flagged_files.append(name)
             continue
 
+        if any(x in VC_DIRS for x in name.lower().split("/")):
+            if flagged_for_vc:
+                continue
+
+            flagged_for_vc = True
+            err.error(
+                err_id=("packagelayout", "version_control"),
+                error="Version control detected in package",
+                description=["A version control directory was detected in "
+                             "your package. Version control may not be "
+                             "included as part of a packaged app due to size "
+                             "and potentially sensitive data.",
+                             "Detected file: %s" % name],
+                filename=name)
+            continue
+
         # Perform a deep inspection to detect magic numbers for known binary
         # and executable file types.
         try:
@@ -71,8 +90,7 @@ def test_blacklisted_files(err, package=None):
             # Note that there is binary content in the metadata
             err.metadata["contains_binary_content"] = True
             err.warning(
-                err_id=("testcases_packagelayout",
-                        "test_blacklisted_files",
+                err_id=("testcases_packagelayout", "test_blacklisted_files",
                         "disallowed_file_type"),
                 warning="Flagged file type found",
                 description=["A file was found to contain flagged content "
@@ -83,26 +101,20 @@ def test_blacklisted_files(err, package=None):
                 filename=name)
 
     if flagged_files:
-        # Detect Java JAR files:
         err.warning(
-            err_id=("testcases_packagelayout",
-                    "test_blacklisted_files",
+            err_id=("testcases_packagelayout", "test_blacklisted_files",
                     "disallowed_extension"),
             warning="Flagged file extensions found.",
             description=["Files whose names end with flagged extensions have "
-                         "been found in the add-on.",
+                         "been found in the app.",
                          "The extension of these files are flagged because "
-                         "they usually identify binary components. Please see "
-                         "http://addons.mozilla.org/developers/docs/"
-                             "policies/reviews#section-binary"
-                         " for more information on the binary content review "
-                         "process.", "\n".join(flagged_files)],
-            filename=name)
+                         "they usually identify binary components, which can "
+                         "contain malware.", "\n".join(flagged_files)])
 
 
 @register_test(tier=1)
 def test_layout_all(err, package):
-    """Tests the well-formedness of extensions."""
+    """Tests the well-formedness of apps."""
 
     if not package:
         return

tests/helper.py

@@ -158,12 +158,13 @@ class MockZipFile:
 
 class MockXPI:
 
-    def __init__(self, data=None):
+    def __init__(self, data=None, default_size=100):
         if not data:
             data = {}
         self.zf = MockZipFile()
         self.data = data
         self.filename = "mock_xpi.xpi"
+        self.default_size = default_size
 
     def test(self):
         return True
@@ -171,6 +172,7 @@ class MockXPI:
     def info(self, name):
         name = name.split('/')[-1]
         return {"name_lower": name.lower(),
+                "size": self.default_size,
                 "extension": name.lower().split(".")[-1]}
 
     def __iter__(self):
@@ -182,4 +184,3 @@ class MockXPI:
 
     def read(self, name):
         return open(self.data[name]).read()
-

tests/test_content.py

@@ -136,9 +136,28 @@ class TestContent(TestCase):
             mock_package = MockXPI(
                 dict([(structure, "tests/resources/content/junk.xpi")]))
             content.test_packed_packages(self.err, mock_package)
+            print structure
             print self.err.print_summary(verbose=True)
             self.assert_failed()
 
         for structure in (".hidden", "dir/__MACOSX/foo", "dir/.foo.swp",
                           "dir/file.old", "dir/file.xul~"):
             yield test_structure, structure
+
+    def test_too_much_garbage(self):
+        """Tests that hidden files are reported."""
+        self.setup_err()
+        mock_package = MockXPI(
+            {".junky": "tests/resources/content/junk.xpi"},
+            default_size=50 * 1024)
+
+        content.test_packed_packages(self.err, mock_package)
+        self.assert_failed(with_warnings=True)
+
+        mock_package = MockXPI(
+            {".junky": "tests/resources/content/junk.xpi",
+             ".morejunk": "tests/resources/content/junk.xpi",},
+            default_size=50 * 1024)
+
+        content.test_packed_packages(self.err, mock_package)
+        self.assert_failed(with_warnings=True, with_errors=True)

tests/test_packagelayout.py

@@ -40,6 +40,16 @@ def test_duplicate_files():
     assert err.failed()
 
 
+def test_version_control():
+    """Test that version control in a package are caught."""
+
+    package = MockXPI({".git/foo/bar": None})
+
+    err = ErrorBundle()
+    packagelayout.test_blacklisted_files(err, package)
+    assert err.failed()
+
+
 def test_spaces_in_names():
     """Test that spaces in filenames are errors."""