Opened 9 years ago

Closed 9 years ago

Last modified 9 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@… 9 years ago.
fix python deserialization for manage.py loaddata

Download all attachments as: .zip

Change History (12)

Changed 9 years ago by sandro.turriate@…

Attachment: __init__.py.diff added

fix python deserialization for manage.py loaddata

comment:1 Changed 9 years ago by James Wheare

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

comment:2 Changed 9 years ago by Russell Keith-Magee

Owner: changed from Adrian Holovaty to Russell Keith-Magee

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

Owner: changed from nobody to empty

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

Keywords: sprintdec01 added

comment:5 Changed 9 years ago by Malcolm Tredinnick

Triage Stage: UnreviewedDesign 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 9 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 9 years ago by Russell Keith-Magee

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 9 years ago by Malcolm Tredinnick

Resolution: wontfix
Status: newclosed

comment:9 Changed 9 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 9 years ago by Russell Keith-Magee

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 9 years ago by empty

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

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