Prevent URLs from being added to ratings text (#22604)

src/olympia/addons/serializers.py

@@ -32,6 +32,7 @@ from olympia.api.fields import (
 )
 from olympia.api.serializers import AMOModelSerializer, BaseESSerializer
 from olympia.api.utils import is_gate_active
+from olympia.api.validators import NoURLsValidator
 from olympia.applications.models import AppVersion
 from olympia.bandwagon.models import Collection
 from olympia.constants.applications import APP_IDS, APPS_ALL
@@ -85,7 +86,6 @@ from .validators import (
     MatchingGuidValidator,
     NoFallbackDefaultLocaleValidator,
     NoThemesValidator,
-    NoURLsValidator,
     ReviewedSourceFileValidator,
     VerifyMozillaTrademark,
     VersionAddonMetadataValidator,

src/olympia/addons/validators.py

@@ -5,7 +5,7 @@ from django.utils.translation import gettext
 from rest_framework import exceptions, fields
 
 from olympia import amo
-from olympia.amo.utils import find_language, verify_no_urls
+from olympia.amo.utils import find_language
 from olympia.versions.models import License
 
 from .models import Addon
@@ -318,14 +318,6 @@ class CanSetCompatibilityValidator:
             )
 
 
-class NoURLsValidator:
-    def __call__(self, value):
-        try:
-            verify_no_urls(value)
-        except forms.ValidationError as exc:
-            raise exceptions.ValidationError(exc.message)
-
-
 class NoThemesValidator:
     requires_context = True
 

src/olympia/api/validators.py

@@ -1,7 +1,8 @@
 from django.core.exceptions import ValidationError as DjangoValidationError
 
-from rest_framework import serializers
+from rest_framework import exceptions
 
+from olympia.amo.utils import verify_no_urls
 from olympia.amo.validators import OneOrMorePrintableCharacterValidator
 
 
@@ -13,4 +14,12 @@ class OneOrMorePrintableCharacterAPIValidator(OneOrMorePrintableCharacterValidat
         try:
             return super().__call__(value)
         except DjangoValidationError:
-            raise serializers.ValidationError(self.message)
+            raise exceptions.ValidationError(self.message)
+
+
+class NoURLsValidator:
+    def __call__(self, value):
+        try:
+            verify_no_urls(value)
+        except DjangoValidationError as exc:
+            raise exceptions.ValidationError(exc.message)

src/olympia/ratings/serializers.py

@@ -1,11 +1,9 @@
 import re
 from collections import OrderedDict
-from urllib.parse import unquote
 
 from django.conf import settings
 from django.utils.translation import gettext, ngettext
 
-from bleach.linkifier import TLDS
 from rest_framework import serializers
 from rest_framework.relations import PrimaryKeyRelatedField
 
@@ -13,23 +11,13 @@ from olympia.accounts.serializers import BaseUserSerializer
 from olympia.addons.serializers import SimpleAddonSerializer, SimpleVersionSerializer
 from olympia.api.serializers import AMOModelSerializer
 from olympia.api.utils import is_gate_active
+from olympia.api.validators import NoURLsValidator
 from olympia.users.utils import RestrictionChecker
 from olympia.versions.models import Version
 
 from .models import DeniedRatingWord, Rating, RatingFlag
 
 
-# This matches the following three types of patterns:
-# http://... or https://..., generic domain names, and IPv4 octets. It does not
-# match IPv6 addresses or long strings such as "example dot com".
-link_pattern = re.compile(
-    r'((://)|'  # Protocols (e.g.: http://)
-    r'((\d{1,3}\.){3}(\d{1,3}))|'
-    r'([0-9a-z\-%%]+\.(%s)))' % '|'.join(TLDS),
-    (re.I | re.U | re.M),
-)
-
-
 class RatingAddonSerializer(SimpleAddonSerializer):
     def get_attribute(self, obj):
         # Avoid database queries if possible by re-using the addon object from
@@ -39,6 +27,12 @@ class RatingAddonSerializer(SimpleAddonSerializer):
 
 class BaseRatingSerializer(AMOModelSerializer):
     addon = RatingAddonSerializer(read_only=True)
+    body = serializers.CharField(
+        allow_blank=True,
+        allow_null=True,
+        required=False,
+        validators=[NoURLsValidator()],
+    )
     is_deleted = serializers.BooleanField(read_only=True, source='deleted')
     is_developer_reply = serializers.SerializerMethodField()
     is_latest = serializers.BooleanField(read_only=True)
@@ -149,12 +143,10 @@ class BaseRatingSerializer(AMOModelSerializer):
                 f'{user.email} or {{{ip_address}}} matched a UserRestriction',
             )
 
-        # Flag the review if there was a word match or a URL was in it.
+        if hasattr(self, '_rating_flag_to_save'):
-        # Unquote when searching for links, in case someone tries 'example%2ecom'.
+            # If we have the _rating_flag_to_save set, we should be set the
-        if (
+            # flag and editorreview attributes on the instance we're about to
-            hasattr(self, '_rating_flag_to_save')
+            # save.
-            or link_pattern.search(unquote(data.get('body') or '')) is not None
-        ):
             data['flag'] = True
             data['editorreview'] = True
 
@@ -188,7 +180,12 @@ class BaseRatingSerializer(AMOModelSerializer):
 class RatingSerializerReply(BaseRatingSerializer):
     """Serializer used for replies only."""
 
-    body = serializers.CharField(allow_null=False, required=True, allow_blank=False)
+    body = serializers.CharField(
+        allow_null=False,
+        required=True,
+        allow_blank=False,
+        validators=[NoURLsValidator()],
+    )
 
     def to_representation(self, obj):
         should_access_deleted = getattr(

src/olympia/ratings/tests/test_views.py

@@ -1824,7 +1824,6 @@ class TestRatingViewSetPost(TestCase):
         self.client.login_api(self.user)
         assert not Rating.objects.exists()
         body = 'Trying to spam <br> http://éxample.com'
-        cleaned_body = 'Trying to spam \n http://éxample.com'
         response = self.client.post(
             self.url,
             {
@@ -1834,21 +1833,8 @@ class TestRatingViewSetPost(TestCase):
                 'version': self.addon.current_version.pk,
             },
         )
-        assert response.status_code == 201
+        assert response.status_code == 400
-        review = Rating.objects.latest('pk')
+        assert response.data['body'] == ['URLs are not allowed.']
-        assert review.pk == response.data['id']
-        assert str(review.body) == response.data['body'] == cleaned_body
-        assert review.rating == response.data['score'] == 5
-        assert review.user == self.user
-        assert review.reply_to is None
-        assert review.addon == self.addon
-        assert review.version == self.addon.current_version
-        assert response.data['version'] == {
-            'id': review.version.id,
-            'version': review.version.version,
-        }
-        assert review.flag
-        assert review.editorreview
 
     def test_post_rating_float(self):
         self.user = user_factory()