Backwards-incompatible change in django.core.signing

[Aymeric Augustin]

(django-dev)myk@myk django % git checkout stable/1.4.x
Switched to branch 'stable/1.4.x'

(django-dev)myk@myk django % python
Python 2.7.1 (r271:86832, Jun 16 2011, 16:59:05) 
[GCC 4.2.1 (Based on Apple Inc. build 5658) (LLVM build 2335.15.00)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> from django.core import signing
>>> signing.b64_encode('foo')
'Zm9v'
>>> ^D

(django-dev)myk@myk django % git checkout master
Switched to branch 'master'

(django-dev)myk@myk django % python
Python 2.7.1 (r271:86832, Jun 16 2011, 16:59:05) 
[GCC 4.2.1 (Based on Apple Inc. build 5658) (LLVM build 2335.15.00)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> from django.core import signing
>>> signing.b64_encode('foo')
u'Zm9v'
>>> ^D

A string is returned under 1.4.x, a unicode in master.

[Aymeric Augustin]

The regression was introduced in [92b2dec918].

This commit also unecessarily uses smart_bytes: we don't care about preserving lazy functions here.

[Aymeric Augustin]

I used a private API in the bug report, however, the same problem happens in the public APIs:

>>> from django.core.signing import Signer
>>> signer = Signer()
>>> value = signer.sign('My string')
>>> value
u'My string:ChWrz0sf2O07KCdM9iP-xb50tuY'

Notice that a unicode value is returned here, where the docs ​show a bytestring.

[Claude Paroz]

I'm not sure it's a regression from 1.4. For example, in 1.4, replace 'q;wjmbk;wkmb' by 'q;wjmbk;wkmb\xc3\xa0' (utf-8 encoded bytestring) in line 41 (test_sign_unsign) of tests/regressiontests/signing/tests.py. This will fail as the returned string is also unicode.

I would be more enclined to fix the docs, unless you could elaborate about real issues it could bring.

[Aymeric Augustin]

I noticed the issue by accident, because it breaks ​https://github.com/aaugustin/django-sesame.

The issues brought by a backwards-incompatible changes in public APIs are obvious. We have an ​API stability policy.

[Aymeric Augustin]

Here's a patch that :

• 1) uses byte strings in b64_encode and b64_decode for consistency with the functions provided by Python's base64 module
• 2) uses native strings (str objects) as the output of sign for backwards compatibility with previous versions of Django on Python 2, and because I believe it's the right thing to do under Python 3,
• 3) uses unicode strings as the output of unsign for backwards compatibility — there was an explicit call to force_unicode, which was replaced by force_text during the Python 3 migration, so the intent was clearly to return unicode there.

[Claude Paroz]

Sign() returns bytestring on python 2

[Claude Paroz]

Here is a minimal patch to fix the type of the returned value of sign() on Python 2. I would not opposed your approach, but I generally try to avoid manipulating bytestrings, which seems to me a little more Python-3 friendly. Would my patch fix your issue in django-sesame?

[Aymeric Augustin]

I agree about avoiding bytestrings when possible. At the lower level, crypto operates on bytes. The question is, where we draw the frontier?

My proposal moves it back "upwards" a bit — above the b64_decode/encode functions.

It is easier to read (and makes less changes) if you also take into account your previous commit. I'm attaching an aggregated diff that shows the net result.

[Aymeric Augustin <aymeric.augustin@…>]

In [28ea4d4b07d98385e0bbfd54058e7eb42b246d26]:

Fixed #18852 -- Restored backwards compatibility

in django.core.signing. Specifically, kept the same return types
(str/unicode) under Python 2. Related to [92b2dec918].