Merge branch 'purity'

apps/addons/models.py

@@ -10,7 +10,8 @@
 import amo.models
 from amo.urlresolvers import reverse
 from reviews.models import Review
-from translations.fields import TranslatedField, translations_with_fallback
+from translations.fields import (TranslatedField, PurifiedField,
+                                 LinkifiedField, translations_with_fallback)
 from users.models import UserProfile
 from search import utils as search_utils
 
@@ -115,10 +116,10 @@ class Addon(amo.models.ModelBase):
     homepage = TranslatedField()
     support_email = TranslatedField(db_column='supportemail')
     support_url = TranslatedField(db_column='supporturl')
-    description = TranslatedField()
+    description = PurifiedField()
 
-    summary = TranslatedField()
-    developer_comments = TranslatedField(db_column='developercomments')
+    summary = LinkifiedField()
+    developer_comments = PurifiedField(db_column='developercomments')
     eula = TranslatedField()
     privacy_policy = TranslatedField(db_column='privacypolicy')
     the_reason = TranslatedField()

apps/api/templates/api/includes/addon.xml

@@ -1,27 +1,27 @@
 <addon>
-  <name>{{addon.name}}</name>
-  <type id="{{addon.type.id}}">{{amo.ADDON_TYPE[addon.type.id]}}</type>
-  <guid>{{addon.guid}}</guid>
-  <version>{{addon.current_version.version}}</version>
-  <status id="{{addon.status}}">{{amo.STATUS_CHOICES[addon.status]}}</status>
+  <name>{{ addon.name }}</name>
+  <type id="{{ addon.type.id }}">{{ amo.ADDON_TYPE[addon.type.id] }}</type>
+  <guid>{{ addon.guid }}</guid>
+  <version>{{ addon.current_version.version }}</version>
+  <status id="{{ addon.status }}">{{ amo.STATUS_CHOICES[addon.status] }}</status>
   <authors>
     {% for author in addon.authors.filter(addonuser__listed=True) %}
-    <author>{{author.display_name}}</author>
+    <author>{{ author.display_name }}</author>
     {% endfor %}
   </authors>
-  <summary>{{addon.summary}}</summary>
-  <description>{{addon.description}}</description>
-  <icon>{{addon.icon_url}}</icon>
+  <summary>{{ addon.summary }}</summary>
+  <description>{{ addon.description }}</description>
+  <icon>{{ addon.icon_url }}</icon>
   <compatible_applications>
     {% if addon.current_version %}
     {% for app in addon.current_version.applicationsversions_set.all() %}
     {% if amo.APP_IDS.get(app.application_id) %}
     <application>
-      <name>{{amo.APP_IDS[app.application_id].pretty}}</name>
-      <application_id>{{app.application_id}}</application_id>
-      <min_version>{{app.min}}</min_version>
-      <max_version>{{app.max}}</max_version>
-      <appID>{{amo.APP_IDS[app.application_id].guid}}</appID>
+      <name>{{ amo.APP_IDS[app.application_id].pretty }}</name>
+      <application_id>{{ app.application_id }}</application_id>
+      <min_version>{{ app.min }}</min_version>
+      <max_version>{{ app.max }}</max_version>
+      <appID>{{ amo.APP_IDS[app.application_id].guid }}</appID>
     </application>
     {% endif %}
     {% endfor %}
@@ -30,18 +30,18 @@
   <all_compatible_os>
     {% if addon.current_version %}
     {% for os in addon.current_version.supported_platforms %}
-    <os>{{os}}</os>
+    <os>{{ os }}</os>
     {% endfor %}
     {% endif %}
   </all_compatible_os>
-  <eula>{{addon.eula|hide_none}}</eula>
-  <thumbnail>{{addon.thumbnail_url}}</thumbnail>
-  <rating>{{addon.bayesian_rating|wround(0, 'ceil')}}</rating>
-  <learnmore>{{settings.SITE_URL+addon.get_absolute_url()+'?src=api'}}</learnmore>
+  <eula>{{ addon.eula|hide_none }}</eula>
+  <thumbnail>{{ addon.thumbnail_url }}</thumbnail>
+  <rating>{{ addon.bayesian_rating|wround(0, 'ceil') }}</rating>
+  <learnmore>{{ settings.SITE_URL+addon.get_absolute_url()+'?src=api' }}</learnmore>
   {% if addon.current_version %}
   {% for file in addon.current_version.files.all() %}
-  <install hash="{{file.hash}}" os="{{file.platform.name}}">
-    {{file.get_absolute_url('api')}}
+  <install hash="{{ file.hash }}" os="{{ file.platform.name }}">
+    {{ file.get_absolute_url('api') }}
   </install>
   {% endfor %}
   {% endif %}

apps/bandwagon/models.py

@@ -4,14 +4,14 @@
 from addons.models import Addon, AddonCategory
 from applications.models import Application
 from users.models import UserProfile
-from translations.fields import TranslatedField
+from translations.fields import TranslatedField, LinkifiedField
 
 
 class Collection(amo.models.ModelBase):
     uuid = models.CharField(max_length=36, blank=True, unique=True)
     name = TranslatedField()
     nickname = models.CharField(max_length=30, blank=True, unique=True)
-    description = TranslatedField()
+    description = LinkifiedField()
     defaultlocale = models.CharField(max_length=10, default='en-US')
     collection_type = models.PositiveIntegerField(default=0)
     icontype = models.CharField(max_length=25, blank=True)

apps/translations/fields.py

@@ -5,19 +5,20 @@
 from django.utils import translation as translation_utils
 from django.utils.translation.trans_real import to_language
 
-from .models import Translation
+from .models import Translation, PurifiedTranslation, LinkifiedTranslation
 from .widgets import TranslationWidget
 
 
 class TranslatedField(models.ForeignKey):
     """A foreign key to the translations table."""
+    model = Translation
 
     def __init__(self, **kwargs):
         # to_field: The field on the related object that the relation is to.
         # Django wants to default to translations.autoid, but we need id.
         options = dict(null=True, to_field='id', unique=True, blank=True)
         kwargs.update(options)
-        super(TranslatedField, self).__init__(Translation, **kwargs)
+        super(TranslatedField, self).__init__(self.model, **kwargs)
 
     @property
     def db_column(self):
@@ -59,11 +60,29 @@ def validate(self, value, model_instance):
         return models.Field.validate(self, value, model_instance)
 
 
+class PurifiedField(TranslatedField):
+    model = PurifiedTranslation
+
+
+class LinkifiedField(TranslatedField):
+    model = LinkifiedTranslation
+
+
+def switch(obj, new_model):
+    """Switch between Translation and Purified/Linkified Translations."""
+    fields = [(f.name, getattr(obj, f.name)) for f in new_model._meta.fields]
+    return new_model(**dict(fields))
+
+
 class TranslationDescriptor(related.ReverseSingleRelatedObjectDescriptor):
     """
     Descriptor that handles creating and updating Translations given strings.
     """
 
+    def __init__(self, field):
+        super(TranslationDescriptor, self).__init__(field)
+        self.model = field.model
+
     def __get__(self, instance, instance_type=None):
         if instance is None:
             return self
@@ -88,6 +107,11 @@ def __set__(self, instance, value):
         # Don't let this be set to None, because Django will then blank out the
         # foreign key for this object.  That's incorrect for translations.
         if value is not None:
+            # We always get these back from the database as Translations, but
+            # we may want them to be a more specific Purified/Linkified child
+            # class.
+            if not isinstance(value, self.model):
+                value = switch(value, self.model)
             super(TranslationDescriptor, self).__set__(instance, value)
 
     def translation_from_string(self, instance, lang, string):
@@ -98,18 +122,18 @@ def translation_from_string(self, instance, lang, string):
             if trans is None and trans_id is not None:
                 # This locale doesn't have a translation set, but there are
                 # translations in another locale, so we have an id already.
-                return Translation.new(string, lang, id=trans_id)
+                return self.model.new(string, lang, id=trans_id)
             elif to_language(trans.locale) == lang.lower():
                 # Replace the translation in the current language.
                 trans.localized_string = string
                 trans.save()
                 return trans
             else:
                 # We already have a translation in a different language.
-                return Translation.new(string, lang, id=trans.id)
+                return self.model.new(string, lang, id=trans.id)
         except AttributeError:
             # Create a brand new translation.
-            return Translation.new(string, lang)
+            return self.model.new(string, lang)
 
     def translation_from_dict(self, instance, lang, dict_):
         """

apps/translations/models.py

@@ -1,6 +1,11 @@
 from django.db import models, connection
 
+from bleach import Bleach
 import caching.base
+import jinja2
+
+
+bleach = Bleach()
 
 
 class Translation(caching.base.CachingMixin, models.Model):
@@ -15,6 +20,7 @@ class Translation(caching.base.CachingMixin, models.Model):
     id = models.IntegerField()
     locale = models.CharField(max_length=10)
     localized_string = models.TextField(null=True)
+    localized_string_clean = models.TextField(null=True)
 
     # These are normally from amo.models.ModelBase, but we don't want to have
     # weird circular dependencies between ModelBase and Translations.
@@ -65,15 +71,50 @@ def new(cls, string, locale, id=None):
         # Update if one exists, otherwise create a new one.
         q = {'id': id, 'locale': locale}
         try:
-            trans = Translation.objects.get(**q)
+            trans = cls.objects.get(**q)
             trans.localized_string = string
             trans.save(force_update=True)
-        except Translation.DoesNotExist:
-            trans = Translation.objects.create(localized_string=string, **q)
+        except cls.DoesNotExist:
+            trans = cls.objects.create(localized_string=string, **q)
 
         return trans
 
 
+class PurifiedTranslation(Translation):
+    """Run the string through bleach to get a safe, linkified version."""
+
+    class Meta:
+        proxy = True
+
+    def __unicode__(self):
+        if not self.localized_string_clean:
+            self.clean()
+        return unicode(self.localized_string_clean)
+
+    def __html__(self):
+        return self
+
+    def clean(self):
+        self.localized_string_clean = bleach.bleach(self.localized_string)
+
+    def save(self, **kwargs):
+        self.clean()
+        return super(PurifiedTranslation, self).save(**kwargs)
+
+
+class LinkifiedTranslation(PurifiedTranslation):
+    """Run the string through bleach to get a linkified version."""
+
+    class Meta:
+        proxy = True
+
+    def clean(self):
+        linkified = bleach.linkify(self.localized_string)
+        clean = bleach.clean(linkified, tags=['a'],
+                             attributes={'a': ['href', 'rel']})
+        self.localized_string_clean = clean
+
+
 class TranslationSequence(models.Model):
     """
     The translations_seq table, so syncdb will create it during testing.

apps/translations/tests/test_models.py

@@ -2,11 +2,11 @@
 from django.core.cache import cache
 from django.utils import translation
 
+import jinja2
 from nose.tools import eq_
-
 from test_utils import ExtraAppTestCase, trans_eq
 
-from testapp.models import TranslatedModel, UntranslatedModel
+from testapp.models import TranslatedModel, UntranslatedModel, FancyModel
 from translations.models import Translation
 from translations import widgets
 from translations.query import order_by_translation
@@ -20,9 +20,6 @@ class TranslationTestCase(ExtraAppTestCase):
     fixtures = ['testapp/test_models.json']
     extra_apps = ['translations.tests.testapp']
 
-    def setUp(self):
-        cache.clear()
-
     def test_fetch_translations(self):
         """Basic check of fetching translations in the current locale."""
         o = TranslatedModel.objects.get(id=1)
@@ -187,6 +184,65 @@ def test_sorting_by_field(self):
 
         del TranslatedModel.get_fallback
 
+    def test_new_purified_field(self):
+        # This is not a full test of the html sanitizing.  We expect the
+        # underlying bleach library to have full tests.
+        s = '<a id=xx href="http://xxx.com">yay</a> <i>http://yyy.com</i>'
+        m = FancyModel.objects.create(purified=s)
+        eq_(m.purified.localized_string_clean,
+            '<a href="http://xxx.com" rel="nofollow">yay</a> '
+            '<i><a href="http://yyy.com" rel="nofollow">http://yyy.com</a></i>')
+        eq_(m.purified.localized_string, s)
+
+    def test_new_linkified_field(self):
+        s = '<a id=xx href="http://xxx.com">yay</a> <i>http://yyy.com</i>'
+        m = FancyModel.objects.create(linkified=s)
+        eq_(m.linkified.localized_string_clean,
+            '<a href="http://xxx.com" rel="nofollow">yay</a> '
+            '&lt;i&gt;<a href="http://yyy.com" rel="nofollow">http://yyy.com</a>&lt;/i&gt;')
+        eq_(m.linkified.localized_string, s)
+
+    def test_update_purified_field(self):
+        m = FancyModel.objects.get(id=1)
+        s = '<a id=xx href="http://xxx.com">yay</a> <i>http://yyy.com</i>'
+        m.purified = s
+        m.save()
+        eq_(m.purified.localized_string_clean,
+            '<a href="http://xxx.com" rel="nofollow">yay</a> '
+            '<i><a href="http://yyy.com" rel="nofollow">http://yyy.com</a></i>')
+        eq_(m.purified.localized_string, s)
+
+    def test_update_linkified_field(self):
+        m = FancyModel.objects.get(id=1)
+        s = '<a id=xx href="http://xxx.com">yay</a> <i>http://yyy.com</i>'
+        m.linkified = s
+        m.save()
+        eq_(m.linkified.localized_string_clean,
+            '<a href="http://xxx.com" rel="nofollow">yay</a> '
+            '&lt;i&gt;<a href="http://yyy.com" rel="nofollow">http://yyy.com</a>&lt;/i&gt;')
+        eq_(m.linkified.localized_string, s)
+
+
+    def test_purified_field_str(self):
+        m = FancyModel.objects.get(id=1)
+        eq_(u'%s' % m.purified,
+            '<i>x</i> '
+            '<a href="http://yyy.com" rel="nofollow">http://yyy.com</a>')
+
+    def test_linkified_field_str(self):
+        m = FancyModel.objects.get(id=1)
+        eq_(u'%s' % m.linkified,
+            '&lt;i&gt;x&lt;/i&gt; '
+            '<a href="http://yyy.com" rel="nofollow">http://yyy.com</a>')
+
+    def test_purifed_linkified_fields_in_template(self):
+        m = FancyModel.objects.get(id=1)
+        env = jinja2.Environment()
+        t = env.from_string('{{ m.purified }}=={{ m.linkified }}')
+        s = t.render(m=m)
+        eq_(s, u'%s==%s' % (m.purified.localized_string_clean,
+                            m.linkified.localized_string_clean))
+
 
 def test_translation_bool():
     t = lambda s: Translation(localized_string=s)