Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed #22804 -- Warned on unsafe value of 'sep=' in Signer #2784

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions django/core/signing.py
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
import base64 import base64
import json import json
import time import time
import warnings
import zlib import zlib


from django.conf import settings from django.conf import settings
Expand All @@ -46,6 +47,10 @@
from django.utils.encoding import force_bytes, force_str, force_text from django.utils.encoding import force_bytes, force_str, force_text
from django.utils.module_loading import import_string from django.utils.module_loading import import_string


_SEP_UNSAFE = set(
"0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ"
"abcdefghijklmnopqrstuvwxyz-_="
)


class BadSignature(Exception): class BadSignature(Exception):
""" """
Expand Down Expand Up @@ -150,6 +155,10 @@ class Signer(object):
def __init__(self, key=None, sep=':', salt=None): def __init__(self, key=None, sep=':', salt=None):
# Use of native strings in all versions of Python # Use of native strings in all versions of Python
self.sep = force_str(sep) self.sep = force_str(sep)
if self.sep in _SEP_UNSAFE:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

won't work if sep is more than a single character.

warnings.warn(
"Unsafe Signer separator: %r (cannot be in %r)"
% (self.sep, "".join(_SEP_UNSAFE)))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

probably want something like: sorted(_SEP_UNSAFE) so the order isn't random. Might be clearer to output something like "0-9A-Za-z-_=" though.

self.key = key or settings.SECRET_KEY self.key = key or settings.SECRET_KEY
self.salt = force_str(salt or self.salt = force_str(salt or
'%s.%s' % (self.__class__.__module__, self.__class__.__name__)) '%s.%s' % (self.__class__.__module__, self.__class__.__name__))
Expand Down
9 changes: 9 additions & 0 deletions tests/signing/tests.py
Original file line number Original file line Diff line number Diff line change
@@ -1,6 +1,7 @@
from __future__ import unicode_literals from __future__ import unicode_literals


import time import time
import warnings


from django.core import signing from django.core import signing
from django.test import TestCase from django.test import TestCase
Expand Down Expand Up @@ -111,6 +112,14 @@ def test_works_with_non_ascii_keys(self):
s = signing.Signer(binary_key) s = signing.Signer(binary_key)
self.assertEqual('foo:6NB0fssLW5RQvZ3Y-MTerq2rX7w', s.sign('foo')) self.assertEqual('foo:6NB0fssLW5RQvZ3Y-MTerq2rX7w', s.sign('foo'))


def test_issues_warning_on_invalid_sep(self):
with warnings.catch_warnings(record=True) as recorded:
warnings.simplefilter('always')
signing.Signer(sep="-")
self.assertEqual(len(recorded), 1)
msg = str(recorded[0].message)
self.assertTrue(msg.startswith("Unsafe Signer separator"))



class TestTimestampSigner(TestCase): class TestTimestampSigner(TestCase):


Expand Down