search

derek-schaefer / django-json-field

Generic JSON model and form fields.
BSD 3-Clause "New" or "Revised" License
122 stars 86 forks source link

DecoderError on basestring #7

Closed mjtorn closed 12 years ago

mjtorn commented 12 years ago

Hi!

Noticed my tests started failing and I found (with the help of bisect) that "77da4d283236fd949906c8fb1367df03500c5baa is the first bad commit"

The behavior turned out that I couldn't assign like 'a', to a json field anymore;

/home/mjt/src/git_checkouts/django-json-field/json_field/fields.pyc in to_python(self, value)
     89             return None
     90         if isinstance(value, basestring):
---> 91             value = json.loads(value, **self.decoder_kwargs)
     92         return value
     93 
[...]
JSONDecodeError: No JSON object could be decoded: line 1 column 0 (char 0)

Sure, JSON is identified as a basestring but this is not necessarily what we mean by it ;)

Not sure if this is a problem or not, as we'd usually assign 'a' to it, but if we assign '"a"' we do not end up with 'a' as the value. Instead it gets parsed into a, apparently ends up in JSONDecoder.decode -> isinstance(obj, basestring) and eventually _dateparser.parse(obj) which interprets it as a valid date.

Assigning 'ab' and '"ab"' work the same.

Assigning '1' makes it an integer but '"1"' a Decimal for the above reasons.

If these findings should be considered problems they should probably be fixed separately.

This pull request is about a real-world problem anyway with assigning strings :)

derek-schaefer commented 12 years ago

Yes, that is definitely the case. Thanks for pointing it out. I was attempting to force strict adherence to the JSON spec (i.e. only values are allowed) but to_python() is used both for assignment and loading from the database so this was the incorrect place to do it.

My pending commit reverts that broken commit, adds some tests, and greatly improves the way JSON deserialization is handled. However, there is still an issue with assigning number strings ('1') to the field as they will be interpreted as, say, an integer. I will be looking into this separately.