Validation improvements

ide/models/build.py

@@ -6,9 +6,10 @@
 from django.conf import settings
 from django.db import models
 from ide.models.project import Project
-from django.utils.translation import ugettext as _
+from django.utils.translation import ugettext_lazy as _
 
 from ide.models.meta import IdeModel
+from ide.utils.regexes import regexes
 
 import utils.s3 as s3
 __author__ = 'katharine'
@@ -34,7 +35,7 @@ class BuildResult(IdeModel):
     DEBUG_WORKER = 1
 
     project = models.ForeignKey(Project, related_name='builds')
-    uuid = models.CharField(max_length=36, default=lambda:str(uuid.uuid4()))
+    uuid = models.CharField(max_length=36, default=lambda: str(uuid.uuid4()), validators=regexes.validator('uuid', _('Invalid UUID.')))
     state = models.IntegerField(choices=STATE_CHOICES, default=STATE_WAITING)
     started = models.DateTimeField(auto_now_add=True, db_index=True)
     finished = models.DateTimeField(blank=True, null=True)

ide/models/files.py

@@ -11,6 +11,7 @@
 from ide.models.s3file import S3File
 from ide.models.textfile import TextFile
 from ide.models.meta import IdeModel
+from ide.utils.regexes import regexes
 
 __author__ = 'katharine'
 
@@ -28,7 +29,7 @@ class ResourceFile(IdeModel):
         ('pbi', _('1-bit Pebble image')),
     )
 
-    file_name = models.CharField(max_length=100, validators=[RegexValidator(r"^[/a-zA-Z0-9_(). -]+$", message=_("Invalid filename."))])
+    file_name = models.CharField(max_length=100, validators=regexes.validator('resource_file_name', _("Invalid filename.")))
     kind = models.CharField(max_length=9, choices=RESOURCE_KINDS)
     is_menu_icon = models.BooleanField(default=False)
 
@@ -141,7 +142,6 @@ def get_tags_string(self):
         return "".join(self.get_tag_names())
 
     def save(self, *args, **kwargs):
-        self.full_clean()
         self.resource_file.save()
         super(ResourceVariant, self).save(*args, **kwargs)
 
@@ -168,7 +168,7 @@ class Meta(S3File.Meta):
 
 class ResourceIdentifier(IdeModel):
     resource_file = models.ForeignKey(ResourceFile, related_name='identifiers')
-    resource_id = models.CharField(max_length=100)
+    resource_id = models.CharField(max_length=100, validators=regexes.validator('c_identifier', _("Invalid resource ID.")))
     character_regex = models.CharField(max_length=100, blank=True, null=True)
     tracking = models.IntegerField(blank=True, null=True)
     compatibility = models.CharField(max_length=10, blank=True, null=True)
@@ -225,7 +225,7 @@ def save(self, *args, **kwargs):
 
 class SourceFile(TextFile):
     project = models.ForeignKey('Project', related_name='source_files')
-    file_name = models.CharField(max_length=100, validators=[RegexValidator(r"^[/a-zA-Z0-9_.-]+\.(c|h|js)$", message=_("Invalid filename."))])
+    file_name = models.CharField(max_length=100, validators=regexes.validator('source_file_name', _('Invalid file name.')))
     folder = 'sources'
 
     TARGETS = (

ide/models/meta.py

@@ -1,7 +1,15 @@
 from django.db import models
-
+from django.db.models.signals import pre_save
+from django.dispatch import receiver
 
 class IdeModel(models.Model):
     class Meta:
         abstract = True
         app_label = "ide"
+
+
+@receiver(pre_save)
+def pre_save_full_clean_handler(sender, instance, *args, **kwargs):
+    """ Force IdeModels to call full_clean before save """
+    if isinstance(instance, IdeModel):
+        instance.full_clean()

ide/models/project.py

@@ -4,13 +4,14 @@
 
 from django.contrib.auth.models import User
 from django.db import models, transaction
-from django.utils.translation import ugettext as _
+from django.utils.translation import ugettext_lazy as _
 from django.core.exceptions import ValidationError
 
 from ide.models.files import ResourceFile, ResourceIdentifier, SourceFile, ResourceVariant
 from ide.models.dependency import Dependency
 from ide.models.meta import IdeModel
 from ide.utils import generate_half_uuid
+from ide.utils.regexes import regexes
 from ide.utils.version import version_to_semver, semver_to_version, parse_sdk_version
 
 __author__ = 'katharine'
@@ -42,7 +43,7 @@ class Project(IdeModel):
     sdk_version = models.CharField(max_length=6, choices=SDK_VERSIONS, default='2')
 
     # New settings for 2.0
-    app_uuid = models.CharField(max_length=36, blank=True, null=True, default=generate_half_uuid)
+    app_uuid = models.CharField(max_length=36, blank=True, null=True, default=generate_half_uuid, validators=regexes.validator('uuid', _('Invalid UUID.')))
     app_company_name = models.CharField(max_length=100, blank=True, null=True)
     app_short_name = models.CharField(max_length=100, blank=True, null=True)
     app_long_name = models.CharField(max_length=100, blank=True, null=True)
@@ -85,7 +86,6 @@ def set_dependencies(self, dependencies):
             Dependency.objects.filter(project=self).delete()
             for name, version in dependencies.iteritems():
                 dep = Dependency.objects.create(project=self, name=name, version=version)
-                dep.full_clean()
                 dep.save()
 
     @property
@@ -118,7 +118,7 @@ def get_parsed_appkeys(self):
         else:
             parsed_keys = []
             for appkey in app_keys:
-                parsed = re.match(r'^([a-zA-Z_][_a-zA-Z\d]*)(?:\[(\d+)\])?$', appkey)
+                parsed = re.match(regexes.C_IDENTIFIER_WITH_INDEX, appkey)
                 if not parsed:
                     raise ValueError("Bad Appkey %s" % appkey)
                 parsed_keys.append((parsed.group(1), parsed.group(2) or 1))

ide/static/ide/js/resources.js

@@ -179,7 +179,10 @@ CloudPebble.Resources = (function() {
             var identifier = resource.identifiers[0];
             resource.identifiers = [identifier + '_WHITE', identifier + '_BLACK'];
         }
-        CloudPebble.Sidebar.SetPopover('resource-' + resource.id, ngettext('Identifier', 'Identifiers', resource.identifiers.length), resource.identifiers.join('<br>'));
+        var popover_content = _.map(resource.identifiers, function(identifier) {
+            return $('<span>').text(identifier).html()
+        }).join('<br>');
+        CloudPebble.Sidebar.SetPopover('resource-' + resource.id, ngettext('Identifier', 'Identifiers', resource.identifiers.length), popover_content);
         // We need to update code completion so it can include these identifiers.
         // However, don't do this during initial setup; the server handle it for us.
         if(CloudPebble.Ready) {
@@ -297,7 +300,7 @@ CloudPebble.Resources = (function() {
         }
 
         // Validate the file name
-        if (!/^[a-zA-Z0-9_(). -]+$/.test(name)) {
+        if (!REGEXES.resource_file_name.test(name)) {
             throw new Error(gettext("You must provide a valid filename. Only alphanumerics and characters in the set \"_(). -\" are allowed."));
         }
 
@@ -887,7 +890,7 @@ CloudPebble.Resources = (function() {
     };
 
     var validate_resource_id = function(id) {
-        return !/[^a-zA-Z0-9_]/.test(id);
+        return REGEXES.c_identifier.test(id);
 
     };
 

ide/static/ide/js/settings.js

@@ -94,10 +94,10 @@ CloudPebble.Settings = (function() {
             // This is not an appropriate use of a regex, but we have to have it for the HTML5 pattern attribute anyway,
             // so we may as well reuse the effort here.
             // It validates that the format matches x[.y] with x, y in [0, 255].
-            if(!version_label.match(VERSION_REGEX)) {
+            if(!version_label.match(REGEXES.sdk_version)) {
                 throw new Error(gettext("You must specify a valid version number."));
             }
-            if(!app_uuid.match(/[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-4[0-9a-fA-F]{3}-[89abAB][0-9a-fA-F]{3}-[0-9a-fA-F]{12}/)) {
+            if(!app_uuid.match(REGEXES.uuid)) {
                 throw new Error(gettext("You must specify a valid UUID (of the form 00000000-0000-0000-0000-000000000000)"));
             }
 

ide/tasks/archive.py

@@ -303,7 +303,6 @@ def make_valid_filename(zip_entry):
                     for root_file_name, loaded in file_exists_for_root.iteritems():
                         if not loaded:
                             raise KeyError("No file was found to satisfy the manifest filename: {}".format(root_file_name))
-                    project.full_clean()
                     project.save()
                     send_td_event('cloudpebble_zip_import_succeeded', project=project)
 

ide/templates/ide/project.html

@@ -480,7 +480,8 @@ <h3>{% trans 'Compass and Accelerometer' %}</h3>
 };
 var DOC_JSON = "{% static 'ide/documentation.json' %}";
 var LIBPEBBLE_PROXY = {{ libpebble_proxy | safe }};
-var VERSION_REGEX = "{{version_regex|escapejs}}";
+var REGEXES = _.mapObject(JSON.parse('{{regexes_json|escapejs}}'), function(v) { return new RegExp(v)});
+
 </script>
 <script src="{% static 'CodeMirror/lib/codemirror.js' %}"></script>
 <script src="{% static 'CodeMirror/addon/dialog/dialog.js' %}"></script>

ide/templates/ide/project/resource.html

@@ -28,7 +28,7 @@
             <div class="control-group" id="edit-resource-file-name_section">
                 <label class="control-label" for="edit-resource-file-name">{% trans 'File name' %}</label>
                 <div class="controls">
-                    <input type="text" id="edit-resource-file-name" name="file_name">
+                    <input type="text" id="edit-resource-file-name" name="file_name" pattern="{{regexes.resource_file_name}}">
                 </div>
             </div>
         </div>
@@ -40,7 +40,7 @@
                     <div class="controls">
                         {# TODO: placeholder/pattern thingybobs #}
                         <input type="text" class="span6 edit-resource-id font-only" placeholder="FONT_EXAMPLE_BOLD_SUBSET_24" pattern="[A-Za-z0-9_]*[0-9]+">
-                        <input type="text" class="span6 edit-resource-id non-font-only" placeholder="IMAGE_EXAMPLE_IDENTIFIER" pattern="[A-Za-z0-9_]+">
+                        <input type="text" class="span6 edit-resource-id non-font-only" placeholder="IMAGE_EXAMPLE_IDENTIFIER" pattern="{{ regexes.c_identifier }}">
                         <span class="help-block">{% trans 'This is used in your code and must be a valid C identifier.' %}<br>
                             <span class="font-only">{% trans 'It must end with the desired font size.' %}</span>
                         </span>

ide/templates/ide/project/settings.html

@@ -120,7 +120,7 @@
                     <label class="control-label" for="settings-version-label">{% trans "Version label" %}</label>
                     <div class="controls">
                         <input type="text" id="settings-version-label" placeholder="0.1" value="{{project.app_version_label}}"
-                                pattern={{version_regex}}>
+                                pattern="{{ regexes.sdk_version }}">
                         <span class="help-block">
                             {% trans "App version. Takes the format major[.minor], where major and minor are between 0 and 255." %}
                         </span>
@@ -130,7 +130,7 @@
                     <label class="control-label" for="settings-uuid">{% trans "App UUID" %}</label>
                     <div class="controls">
                         <div class="settings-uuid-control">
-                            <input type="text" id="settings-uuid" pattern="[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-4[0-9a-fA-F]{3}-[89abAB][0-9a-fA-F]{3}-[0-9a-fA-F]{12}" placeholder="00000000-0000-0000-0000-000000000000" value="{{project.app_uuid}}">
+                            <input type="text" id="settings-uuid" pattern="{{ regexes.uuid }}" placeholder="00000000-0000-0000-0000-000000000000" value="{{project.app_uuid}}">
                             <button id="uuid-generate" class="btn btn-small">{% trans "Generate" %}</button>
                         </div>
                         <span class="help-block">

ide/tests/test_import_archive.py

@@ -12,22 +12,24 @@
 
 fake_s3 = FakeS3()
 
-RESOURCE_SPEC = {
-    'resources': {
-        'media': [{
-            'file': 'images/blah.png',
-            'name': 'IMAGE_BLAH',
-            'type': 'bitmap'
-        }]
-    }
-}
-
 
 @mock.patch('ide.models.s3file.s3', fake_s3)
 class TestImportProject(CloudpebbleTestCase):
     def setUp(self):
         self.login()
 
+    @staticmethod
+    def make_resource_spec(name='IMAGE_BLAH'):
+        return {
+            'resources': {
+                'media': [{
+                    'file': 'images/blah.png',
+                    'name': name,
+                    'type': 'bitmap'
+                }]
+            }
+        }
+
     def test_import_basic_bundle_with_appinfo(self):
         """ Check that a minimal bundle imports without error """
         bundle = build_bundle({
@@ -97,7 +99,7 @@ def test_import_appinfo_with_resources(self):
         bundle = build_bundle({
             'src/main.c': '',
             'resources/images/blah.png': 'contents!',
-            'appinfo.json': make_appinfo(options=RESOURCE_SPEC)
+            'appinfo.json': make_appinfo(options=self.make_resource_spec())
         })
         do_import_archive(self.project_id, bundle)
         project = Project.objects.get(pk=self.project_id)
@@ -108,7 +110,7 @@ def test_import_package_with_resources(self):
         bundle = build_bundle({
             'src/main.c': '',
             'resources/images/blah.png': 'contents!',
-            'package.json': make_package(pebble_options=RESOURCE_SPEC)
+            'package.json': make_package(pebble_options=self.make_resource_spec())
         })
         do_import_archive(self.project_id, bundle)
         project = Project.objects.get(pk=self.project_id)
@@ -151,3 +153,13 @@ def test_throws_if_importing_array_appkeys_without_npm_manifest_support(self):
         })
         with self.assertRaises(InvalidProjectArchiveException):
             do_import_archive(self.project_id, bundle)
+
+    def test_invalid_resource_id(self):
+        bundle = build_bundle({
+            'src/main.c': '',
+            'resources/images/blah.png': 'contents!',
+            'package.json': make_package(pebble_options=self.make_resource_spec("<>"))
+        })
+
+        with self.assertRaises(ValidationError):
+            do_import_archive(self.project_id, bundle)

ide/utils/cloudpebble_test.py

@@ -56,7 +56,7 @@ def make_appinfo(options=None):
         },
         "sdkVersion": "3",
         "shortName": "test",
-        "uuid": "666x6666-x66x-66x6-x666-666666666666",
+        "uuid": "123e4567-e89b-42d3-a456-426655440000",
         "versionLabel": "1.0",
         "watchapp": {
             "watchface": False
@@ -92,7 +92,7 @@ def make_package(package_options=None, pebble_options=None, no_pebble=False):
                 "media": []
             },
             "sdkVersion": "3",
-            "uuid": '666x6666-x66x-66x6-x666-666666666666',
+            "uuid": '123e4567-e89b-42d3-a456-426655440000',
             "watchapp": {
                 "watchface": False
             }

ide/utils/regexes.py

@@ -0,0 +1,35 @@
+from django.core.validators import RegexValidator
+
+
+class RegexHolder(object):
+    regex_dictionary = {
+        # Match major[.minor], where major and minor are numbers between 0 and 255 with no leading 0s
+        'sdk_version': r'^(0|[1-9]\d?|1\d{2}|2[0-4]\d|25[0-5])(\.(0|[1-9]\d?|1\d{2}|2[0-4]\d|25[0-5]))?$',
+
+        # Match major.minor.0, where major and minor are numbers between 0 and 255 with no leading 0s
+        'semver': r'^(0|[1-9]\d?|1\d{2}|2[0-4]\d|25[0-5])\.(0|[1-9]\d?|1\d{2}|2[0-4]\d|25[0-5])\.0$',
+
+        # Match a string of letters and numbers separated with underscores but not starting with a digit
+        'c_identifier': r'^\w+$',
+
+        # Match a C identifier optionally followed by an [array index]
+        'c_identifier_with_index': r'^(\w+)(?:\[(\d+)\])?$',
+
+        # Match a UUID4
+        'uuid': r'^[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-4[0-9a-fA-F]{3}-[89abAB][0-9a-fA-F]{3}-[0-9a-fA-F]{12}$',
+
+        # Match a valid resource file name
+        'resource_file_name': r'^[/a-zA-Z0-9_(). -]+$',
+
+        # Match a valid c/h/js file name
+        'source_file_name': r'^[/a-zA-Z0-9_.-]+\.(c|h|js)$'
+    }
+
+    def validator(self, key, message):
+        return [RegexValidator(self.regex_dictionary[key], message=message)]
+
+    def __getattr__(self, key):
+        return self.regex_dictionary[key.lower()]
+
+
+regexes = RegexHolder()

ide/utils/version.py

@@ -1,18 +1,14 @@
 import re
 
-# Match major[.minor], where major and minor are numbers between 0 and 255 with no leading 0s
-SDK_VERSION_REGEX = r"^(0|[1-9]\d?|1\d{2}|2[0-4]\d|25[0-5])(\.(0|[1-9]\d?|1\d{2}|2[0-4]\d|25[0-5]))?$"
-
-# Match major.minor.0, where major and minor are numbers between 0 and 255 with no leading 0s
-SEMVER_REGEX = r"^(0|[1-9]\d?|1\d{2}|2[0-4]\d|25[0-5])\.(0|[1-9]\d?|1\d{2}|2[0-4]\d|25[0-5])\.0$"
+from ide.utils.regexes import regexes
 
 
 def parse_sdk_version(version):
     """ Parse an SDK compatible version string
     :param version: should be "major[.minor]"
     :return: (major, minor)
     """
-    parsed = re.match(SDK_VERSION_REGEX, version)
+    parsed = re.match(regexes.SDK_VERSION, version)
     if not parsed:
         raise ValueError("Invalid version {}".format(version))
     major = parsed.group(1)
@@ -33,7 +29,7 @@ def parse_semver(semver):
     :param semver: should be "major.minor.0"
     :return: (major, minor)
     """
-    parsed = re.match(SEMVER_REGEX, semver)
+    parsed = re.match(regexes.SEMVER, semver)
     if not parsed:
         raise ValueError("Invalid semver {}".format(semver))
     return parsed.group(1), parsed.group(2)

ide/views/project.py

@@ -12,7 +12,8 @@
 from ide.tasks.git import hooked_commit
 from ide.utils import generate_half_uuid
 from utils.td_helper import send_td_event
-from ide.utils.version import SDK_VERSION_REGEX
+from ide.utils.regexes import regexes
+
 __author__ = 'katharine'
 
 
@@ -49,7 +50,8 @@ def view_project(request, project_id):
         'token': token,
         'phone_shorturl': settings.PHONE_SHORTURL,
         'supported_platforms': supported_platforms,
-        'version_regex': SDK_VERSION_REGEX,
+        'regexes': regexes,
+        'regexes_json': json.dumps(regexes.regex_dictionary),
         'npm_manifest_support_enabled': settings.NPM_MANIFEST_SUPPORT
     })