App icon limit (bug 815444)

appvalidator/constants.py

@@ -24,6 +24,8 @@ MAX_STR_SIZE = 1024 * 24  # 24KB
 
 MAX_RESOURCE_SIZE = 2 * 1024 * 1024
 
+ICON_LIMIT = 10
+
 # Graciously provided by @kumar in bug 614574
 if (not SPIDERMONKEY_INSTALLATION or
     not os.path.exists(SPIDERMONKEY_INSTALLATION)):

appvalidator/testcases/webappbase.py

@@ -190,13 +190,33 @@ def test_app_resources(err, package):
     # Test the icons in the manifest. The manifest validator should have thrown
     # a hard error if this isn't a dict, so this won't ever be reached if it'll
     # otherwise fail with subscript errors.
-    for icon_size, url in manifest.get("icons", {}).items():
+    icon_urls = set()
+    icons = manifest.get("icons", {}).items()
+    for icon_size, url in icons:
+        # Don't test the same icon URL twice.
+        if url in icon_urls:
+            continue
+        elif len(icon_urls) == constants.ICON_LIMIT:
+            # If we're going in and there are already ten icons, that's a
+            # problem.
+            err.warning(
+                err_id=("resources", "icon", "count"),
+                warning="Too man icon resources.",
+                description=["The app's manifest file contains more than %d "
+                             "icons. Including too many icons could put "
+                             "unnecessary load on a web server." %
+                                 constants.ICON_LIMIT,
+                             "Found %d icons." % len(icons)],
+                filename="webapp.manifest")
+            break
+
         icon_data = try_get_resource(err, package,
                                      url, "webapp.manifest", "icon")
         if not icon_data:
             continue
 
         test_icon(err, data=StringIO(icon_data), url=url, size=icon_size)
+        icon_urls.add(url)
 
     if "launch_path" in manifest:
         try_get_resource(err, package, manifest["launch_path"],

tests/test_webapp_resources.py

@@ -5,9 +5,10 @@ from mock import Mock, patch
 from nose.tools import eq_, raises
 import requests.exceptions as reqexc
 
-from helper import TestCase
-from appvalidator.errorbundle import ErrorBundle
 import appvalidator.testcases.webappbase as appbase
+from helper import TestCase
+from appvalidator.constants import ICON_LIMIT
+from appvalidator.errorbundle import ErrorBundle
 
 
 class TestWebappDataURL(TestCase):
@@ -174,11 +175,30 @@ class TestResourcePolling(TestCase):
     @patch("appvalidator.testcases.webappbase.try_get_resource")
     def test_icons(self, tgr, test_icon):
         tgr.return_value = "foobar"
+
         self.setup_manifest()["icons"] = {"32": "fizz"}
         appbase.test_app_resources(self.err, None)
         eq_(tgr.call_args[0][2], "fizz")
         eq_(test_icon.call_args[1]["data"].getvalue(), "foobar")
 
+    @patch("appvalidator.testcases.webappbase.test_icon", Mock())
+    @patch("appvalidator.testcases.webappbase.try_get_resource",
+           Mock(return_value="this is an icon."))
+    def test_too_many_icons(self):
+        self.setup_manifest()["icons"] = dict(
+            [(str(i), "http://foo%d.jpg" % i) for i in range(ICON_LIMIT + 1)])
+        appbase.test_app_resources(self.err, None)
+        self.assert_failed(with_warnings=True)
+
+    @patch("appvalidator.testcases.webappbase.test_icon", Mock())
+    @patch("appvalidator.testcases.webappbase.try_get_resource",
+           Mock(return_value="this is an icon."))
+    def test_many_icons_same_url(self):
+        self.setup_manifest()["icons"] = dict(
+            [(str(i), "foo.jpg") for i in range(ICON_LIMIT + 1)])
+        appbase.test_app_resources(self.err, None)
+        self.assert_silent()
+
     @patch("appvalidator.testcases.webappbase.try_get_resource")
     def test_appcache_path(self, tgr):
         self.setup_manifest()["appcache_path"] = "fizz"