Fixed #22804 -- Added warning for unsafe value of 'sep' in Signer

Thanks Jaap Roes for completing the patch.

4 files changed, 33 insertions, 1 deletions

diff --git a/django/core/signing.py b/django/core/signing.py
index d46a386c6d..61b43316db 100644
--- a/django/core/signing.py
+++ b/django/core/signing.py
@@ -38,15 +38,20 @@ from __future__ import unicode_literals
 import base64
 import datetime
 import json
+import re
 import time
+import warnings
 import zlib
 
 from django.conf import settings
 from django.utils import baseconv
 from django.utils.crypto import constant_time_compare, salted_hmac
+from django.utils.deprecation import RemovedInDjango110Warning
 from django.utils.encoding import force_bytes, force_str, force_text
 from django.utils.module_loading import import_string
 
+_SEP_UNSAFE = re.compile(r'^[A-z0-9-_=]*$')
+
 
 class BadSignature(Exception):
     """
@@ -150,8 +155,11 @@ class Signer(object):
 
     def __init__(self, key=None, sep=':', salt=None):
         # Use of native strings in all versions of Python
-        self.sep = force_str(sep)
         self.key = key or settings.SECRET_KEY
+        self.sep = force_str(sep)
+        if _SEP_UNSAFE.match(self.sep):
+            warnings.warn('Unsafe Signer separator: %r (cannot be empty or consist of only A-z0-9-_=)' % sep,
+                          RemovedInDjango110Warning)
         self.salt = force_str(salt or
             '%s.%s' % (self.__class__.__module__, self.__class__.__name__))
 
diff --git a/docs/internals/deprecation.txt b/docs/internals/deprecation.txt
index 67f2982292..63171d09b5 100644
--- a/docs/internals/deprecation.txt
+++ b/docs/internals/deprecation.txt
@@ -247,6 +247,9 @@ details on these changes.
 * Support for the syntax of ``{% cycle %}`` that uses comma-separated arguments
   will be removed.
 
+* The warning that :class:`~django.core.signing.Signer` issues when given an
+  invalid separator will become an exception.
+
 .. _deprecation-removed-in-1.9:
 
 1.9
diff --git a/docs/releases/1.9.txt b/docs/releases/1.9.txt
index b5c515db09..4ad98e05a2 100644
--- a/docs/releases/1.9.txt
+++ b/docs/releases/1.9.txt
@@ -974,6 +974,9 @@ Miscellaneous
   ``django.utils.feedgenerator.RssFeed.mime_type`` attributes are deprecated in
   favor of ``content_type``.
 
+* :class:`~django.core.signing.Signer` now issues a warning if an invalid
+  separator is used. This will become an exception in Django 1.10.
+
 .. removed-features-1.9:
 
 Features removed in 1.9
diff --git a/tests/signing/tests.py b/tests/signing/tests.py
index 80dadee7f4..a8de1b0cbc 100644
--- a/tests/signing/tests.py
+++ b/tests/signing/tests.py
@@ -1,6 +1,7 @@
 from __future__ import unicode_literals
 
 import datetime
+import warnings
 
 from django.core import signing
 from django.test import SimpleTestCase
@@ -112,6 +113,23 @@ class TestSigner(SimpleTestCase):
         s = signing.Signer(binary_key)
         self.assertEqual('foo:6NB0fssLW5RQvZ3Y-MTerq2rX7w', s.sign('foo'))
 
+    def test_valid_sep(self):
+        separators = ['/', '*sep*', ',']
+        for sep in separators:
+            signer = signing.Signer('predictable-secret', sep=sep)
+            self.assertEqual('foo%ssH9B01cZcJ9FoT_jEVkRkNULrl8' % sep, signer.sign('foo'))
+
+    def test_invalid_sep(self):
+        """should warn on invalid separator"""
+        separators = ['', '-', 'abc']
+        for sep in separators:
+            with warnings.catch_warnings(record=True) as recorded:
+                warnings.simplefilter('always')
+                signing.Signer(sep=sep)
+                self.assertEqual(len(recorded), 1)
+                msg = str(recorded[0].message)
+                self.assertTrue(msg.startswith('Unsafe Signer separator'))
+
 
 class TestTimestampSigner(SimpleTestCase):