Code

Opened 6 years ago

Closed 6 years ago

#9814 closed (fixed)

Python 2.6's sqlite refuses 8-bit SafeStrings

Reported by: kmtracey Owned by: nobody
Component: Database layer (models, ORM) Version: 1.0
Severity: Keywords:
Cc: Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

r9467/r9469 modified a test in modeltests/field_defaults to set a field value to a utf-8 encoded SafeString. This test now fails on Python 2.6 when using sqlite as the backend:

======================================================================
FAIL: Doctest: modeltests.field_defaults.models.__test__.API_TESTS
----------------------------------------------------------------------
Traceback (most recent call last):
  File "d:\u\kmt\django\trunk\django\test\_doctest.py", line 2180, in runTest
    raise self.failureException(self.format_failure(new.getvalue()))
AssertionError: Failed doctest test for modeltests.field_defaults.models.__test__.API_TESTS
  File "D:\u\kmt\django\trunk\tests\modeltests\field_defaults\models.py", line unknown line number, in API_TESTS

----------------------------------------------------------------------
File "D:\u\kmt\django\trunk\tests\modeltests\field_defaults\models.py", line ?, in modeltests.field_defaults.models.__test__.API_TESTS
Failed example:
    a.save()
Exception raised:
    Traceback (most recent call last):
      File "d:\u\kmt\django\trunk\django\test\_doctest.py", line 1267, in __run
        compileflags, 1) in test.globs
      File "<doctest modeltests.field_defaults.models.__test__.API_TESTS[13]>", line 1, in <module>
        a.save()
      File "d:\u\kmt\django\trunk\django\db\models\base.py", line 328, in save
        self.save_base(force_insert=force_insert, force_update=force_update)
      File "d:\u\kmt\django\trunk\django\db\models\base.py", line 379, in save_base
        rows = manager.filter(pk=pk_val)._update(values)
      File "d:\u\kmt\django\trunk\django\db\models\query.py", line 435, in _update
        return query.execute_sql(None)
      File "d:\u\kmt\django\trunk\django\db\models\sql\subqueries.py", line 117, in execute_sql
        cursor = super(UpdateQuery, self).execute_sql(result_type)
      File "d:\u\kmt\django\trunk\django\db\models\sql\query.py", line 1756, in execute_sql
        cursor.execute(sql, params)
      File "d:\u\kmt\django\trunk\django\db\backends\sqlite3\base.py", line 169, in execute
        return Database.Cursor.execute(self, query, params)
    ProgrammingError: You must not use 8-bit bytestrings unless you use a text_factory that can interpret 8-bit bytestrings (like text_factory = str). It is highly recommended that you instead just switch your application to Unicode strings.

----------------------------------------------------------------------
Ran 1 test in 0.030s

FAILED (failures=1)

This can be fixed as regular str was fixed for Python 2.6 (r8276) by adding Database.register_adapter(SafeString, lambda s:s.decode('utf-8')) to the appropriate spot in django/db/sqlite3/base.py. That seems correct but just in case I'm missing something I figured I'd check first before just doing it. (I'm not entirely sure when app code would be sending SafeStrings to the DB, but I suppose it can happen and should be supported?) Feedback?

Attachments (1)

ss.diff (945 bytes) - added by kmtracey 6 years ago.

Download all attachments as: .zip

Change History (4)

Changed 6 years ago by kmtracey

comment:1 Changed 6 years ago by mtredinnick

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

This looks like another case like MySQL, where it's doing a bad type check. isinstance(SafeString, str) is True, so it should just work with safe strings as strings if it needs strings, like everything else does.

It's not illegal to use SafeString wherever all good strings are accepted (much like American Express :-) ), so we have to fix this. However, if it's only SafeString that's the problem, we don't need any decoding or encoding. It's already required to be a bytestring, since it's a "str" subclass, not a "unicode" subclass. Thus, simple str() on the object will be the right thing, I suspect.

Don't have time to test this out now. Only wanted to note that I think (a) we should add the adaptor if that's really required, (b) I'm still not loving the Python 2.6 developers because this should be completely unnecessary as we're already giving them something that acts as a str, and (c) we should check SafeUnicode for equivalent issues (and if the fact that SafeUnicode is a unicode means it gets a free ride, please refer to point (b)).

comment:2 Changed 6 years ago by kmtracey

Trying Database.register_adapter(SafeString, lambda s:str(s)) doesn't work. I'm thinking it doesn't then go through the registered adapters again and see that str's have their own adapter (which does a utf-8 decode)?

It has no problem with SafeUnicode, apparently, since a couple of lines prior to the test line that is causing the current failure is saving of a field set to a SafeUnicode value. It may be just when it is searching through adapters that it isn't really matching properly (we don't have to register an adapter for unicode). I'll look at it again when I'm more awake in the AM.

comment:3 Changed 6 years ago by kmtracey

  • Resolution set to fixed
  • Status changed from new to closed

Fixed in r9654/r9655 on trunk and 1.0.X. I don't know why this wasn't auto-closed.

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.