Bug in utils.encoding producing invalid JSON for request body

[Nick Sarbicki]

I've just posted a ticket for this here: https://code.djangoproject.com/ticket/29082#ticket

The general point is that when I'm using RequestFactory and making a post request with a JSON body, I'd expect to be able to pass in a dict for this - but I can't as the JSON it produces (it does try) is invalid (single instead of double quotes).

The fix is three lines in the file but another isinstance call among many.

I've been meaning to contribute for a long time and would like this to be my first ticket if I can get confirmation that it _is_ a bug and not intentional.

Reproducible example and exact location of offending code are in the ticket.

- Nick.

[Adam Johnson]

the JSON it produces (it does try)

I wouldn't say it "tries", it just calls str on any object that is passed in, which for a simple dict with strings in, looks close to JSON. 
force_bytes definitely shouldn't be trying to JSON-coerce things.

Thus I don't think it's a bug. Bugs are when behaviour doesn't match the documentation, and afaict coercing a dict to JSON isn't documented anywhere.

It should be possible to make a subclass of RequestFactory or Client that does json.dumps inside _encode_data before calling super(). It's possible to swap the self.client on your TestCase subclasses easily by setting client_class so you could make it the default in your project.

--
You received this message because you are subscribed to the Google Groups "Django developers (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email to django-developers+unsubscribe@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at https://groups.google.com/group/django-developers.
To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/77ab7248-3d6f-4cca-8ddc-27afc49c520c%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

--

Adam

[Nick Sarbicki]

There's been a longer discussion on the ticket about this. The point you raise I agree with. I also think _encode_data should be left alone.

I still think this should be in framework instead of having to subclass RequestFactory. Passing in a dict as data should not give invalid JSON.

Instead it would be better to implement an _encode_json. This would be called if we have the correct content_type within the .post, .patch and .delete methods. Question for me is how to pass the kwargs in for json.dumps to allow custom encoders. Or do we not want this (I think we do)?

On Mon, Jan 29, 2018 at 4:50 PM Adam Johnson <m...@adamj.eu> wrote:

the JSON it produces (it does try)

I wouldn't say it "tries", it just calls str on any object that is passed in, which for a simple dict with strings in, looks close to JSON. 
force_bytes definitely shouldn't be trying to JSON-coerce things.

Thus I don't think it's a bug. Bugs are when behaviour doesn't match the documentation, and afaict coercing a dict to JSON isn't documented anywhere.

It should be possible to make a subclass of RequestFactory or Client that does json.dumps inside _encode_data before calling super(). It's possible to swap the self.client on your TestCase subclasses easily by setting client_class so you could make it the default in your project.

On 29 January 2018 at 14:14, Nick Sarbicki <nick.a....@gmail.com> wrote:

I've just posted a ticket for this here: https://code.djangoproject.com/ticket/29082#ticket

The general point is that when I'm using RequestFactory and making a post request with a JSON body, I'd expect to be able to pass in a dict for this - but I can't as the JSON it produces (it does try) is invalid (single instead of double quotes).

The fix is three lines in the file but another isinstance call among many.

I've been meaning to contribute for a long time and would like this to be my first ticket if I can get confirmation that it _is_ a bug and not intentional.

Reproducible example and exact location of offending code are in the ticket.

- Nick.

--
You received this message because you are subscribed to the Google Groups "Django developers (Contributions to Django itself)" group.

To unsubscribe from this group and stop receiving emails from it, send an email to django-develop...@googlegroups.com.
To post to this group, send email to django-d...@googlegroups.com.

Visit this group at https://groups.google.com/group/django-developers.
To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/77ab7248-3d6f-4cca-8ddc-27afc49c520c%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

--

Adam

--

You received this message because you are subscribed to the Google Groups "Django developers (Contributions to Django itself)" group.

To unsubscribe from this group and stop receiving emails from it, send an email to django-develop...@googlegroups.com.
To post to this group, send email to django-d...@googlegroups.com.

Visit this group at https://groups.google.com/group/django-developers.

To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/CAMyDDM1biqF10RR9Ybm%3Dbs083PCSQbjwy1LCCM66nOS4B%3DW6rA%40mail.gmail.com.

[Adam Johnson]

Sorry, replying on ticket.

On 29 January 2018 at 16:53, Nick Sarbicki <nick.a....@gmail.com> wrote:

There's been a longer discussion on the ticket about this. The point you raise I agree with. I also think _encode_data should be left alone.

I still think this should be in framework instead of having to subclass RequestFactory. Passing in a dict as data should not give invalid JSON.

Instead it would be better to implement an _encode_json. This would be called if we have the correct content_type within the .post, .patch and .delete methods. Question for me is how to pass the kwargs in for json.dumps to allow custom encoders. Or do we not want this (I think we do)?

On Mon, Jan 29, 2018 at 4:50 PM Adam Johnson <m...@adamj.eu> wrote:

the JSON it produces (it does try)

I wouldn't say it "tries", it just calls str on any object that is passed in, which for a simple dict with strings in, looks close to JSON. 
force_bytes definitely shouldn't be trying to JSON-coerce things.

Thus I don't think it's a bug. Bugs are when behaviour doesn't match the documentation, and afaict coercing a dict to JSON isn't documented anywhere.

It should be possible to make a subclass of RequestFactory or Client that does json.dumps inside _encode_data before calling super(). It's possible to swap the self.client on your TestCase subclasses easily by setting client_class so you could make it the default in your project.

On 29 January 2018 at 14:14, Nick Sarbicki <nick.a....@gmail.com> wrote:

I've just posted a ticket for this here: https://code.djangoproject.com/ticket/29082#ticket

The general point is that when I'm using RequestFactory and making a post request with a JSON body, I'd expect to be able to pass in a dict for this - but I can't as the JSON it produces (it does try) is invalid (single instead of double quotes).

The fix is three lines in the file but another isinstance call among many.

I've been meaning to contribute for a long time and would like this to be my first ticket if I can get confirmation that it _is_ a bug and not intentional.

Reproducible example and exact location of offending code are in the ticket.

- Nick.

--
You received this message because you are subscribed to the Google Groups "Django developers (Contributions to Django itself)" group.

To unsubscribe from this group and stop receiving emails from it, send an email to django-developers+unsubscribe@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.

Visit this group at https://groups.google.com/group/django-developers.
To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/77ab7248-3d6f-4cca-8ddc-27afc49c520c%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

--

Adam

--
You received this message because you are subscribed to the Google Groups "Django developers (Contributions to Django itself)" group.

To unsubscribe from this group and stop receiving emails from it, send an email to django-developers+unsubscribe@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.

Visit this group at https://groups.google.com/group/django-developers.

To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/CAMyDDM1biqF10RR9Ybm%3Dbs083PCSQbjwy1LCCM66nOS4B%3DW6rA%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.

--

You received this message because you are subscribed to the Google Groups "Django developers (Contributions to Django itself)" group.

To unsubscribe from this group and stop receiving emails from it, send an email to django-developers+unsubscribe@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.

Visit this group at https://groups.google.com/group/django-developers.

To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/CAGuvt9216aoAbTSukC0a9VxGitxcx4zhmkqfS%2BFu0oVg%2BYNz4A%40mail.gmail.com.

For more options, visit https://groups.google.com/d/optout.

--

Adam