Opened 8 years ago

Closed 8 years ago

Last modified 8 years ago

#4187 closed (wontfix)

manage.py loaddata fails when loading python serialized code

Reported by: sandro.turriate@… Owned by: nobody
Component: Core (Other) Version: master
Severity: Keywords: serializers sprintdec01
Cc: django@… Triage Stage: Design decision needed
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

I dumped my database using the python format serializer:
./manage.py dumpdata app --format=python > app/fixtures/1.python

When loading the app I get the error message:

$ ./manage.py loaddata 1.python
Loading '2' fixtures...
Installing python fixture '2' from 'app/fixtures'.
Problem installing fixture 'app/fixtures/2.python': string indices must be integers

The problem is that the python deserializer is trying to loop over a string and not python objects.

I've attached a hack to fix the problem, I was unsure of the best place to put the patch so I added it to __init__.py, please advise on better placement.

Attachments (1)

__init__.py.diff (861 bytes) - added by sandro.turriate@… 8 years ago.
fix python deserialization for manage.py loaddata

Download all attachments as: .zip

Change History (12)

Changed 8 years ago by sandro.turriate@…

fix python deserialization for manage.py loaddata

comment:1 Changed 8 years ago by James Wheare

  • Cc django@… added
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

comment:2 Changed 8 years ago by russellm

  • Owner changed from adrian to russellm

comment:3 Changed 8 years ago by empty <mtrier@…>

  • Owner changed from nobody to empty

comment:4 Changed 8 years ago by empty <mtrier@…>

  • Keywords sprintdec01 added

comment:5 follow-up: Changed 8 years ago by mtredinnick

  • Triage Stage changed from Unreviewed to Design decision needed

I would be -1 on including anything like this. At the moment, loading fixtures is reasonably safe: they include data that doesn't already exist into the database, but cannot delete anything. This patch, however, means that arbitrary Python code can be executed. Okay, you are probably only loading fixtures that you created (or come from somebody you trust), but why take the risk? We have the xml, json and yaml formats for serialising to storage in any case. I don't think we need the Python one for that purpose.

Russ: any reason we need to be able to load Python format from disk?

comment:6 Changed 8 years ago by empty <mtrier@…>

  • Owner changed from empty to nobody

If it's decided to go on with this I just want to note that the patch itself is not in the right place. If we’re changing the deserialization logic for the python serializer then that code should be self contained in the Deserializer class for python and not placed in the general deserialization routine.

comment:7 in reply to: ↑ 5 Changed 8 years ago by russellm

Replying to mtredinnick:

Russ: any reason we need to be able to load Python format from disk?

Absolutely none. The primary reason for its existence is as an internal utility format. I'm -1 on this patch too.

IMHO, we should be removing all reference to a "python" serializer; as you noted, it's a potential (albeit small) security issue, and doesn't add anything that JSON, XML and YAML already provide.

comment:8 Changed 8 years ago by mtredinnick

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

comment:9 follow-up: Changed 8 years ago by empty <mtrier@…>

Would it make sense then to open a ticket to not allow the python option in the loaddata dumpdata commands? The docs are a bit confusing because they point to the serializers page, which then goes on to say that python is an available serializer, although discourages it's use directly.

comment:10 in reply to: ↑ 9 Changed 8 years ago by russellm

Replying to empty <mtrier@gmail.com>:

Would it make sense then to open a ticket to not allow the python option in the loaddata dumpdata commands?

Yes :-)

Want to take care of this for us?

comment:11 Changed 8 years ago by empty

I'll get on it. Thanks. Ticket #6110.

Note: See TracTickets for help on using tickets.
Back to Top