Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

returning QueryDict with content-type application/json and empty body #5349

Closed
5 of 6 tasks
Reskov opened this issue Aug 22, 2017 · 3 comments · Fixed by #5351
Closed
5 of 6 tasks

returning QueryDict with content-type application/json and empty body #5349

Reskov opened this issue Aug 22, 2017 · 3 comments · Fixed by #5351

Comments

@Reskov
Copy link

Reskov commented Aug 22, 2017

Checklist

  • I have verified that that issue exists against the master branch of Django REST framework.
  • I have searched for similar issues in both open and closed tickets and cannot find a duplicate.
  • This is not a usage question. (Those should be directed to the discussion group instead.)
  • This cannot be dealt with as a third party library. (We prefer new functionality to be in the form of third party libraries where possible.)
  • I have reduced the issue to the simplest possible case.
  • I have included a failing test as a pull request. (If you are unable to do so we can still accept the issue.)

Steps to reproduce

Hi, is this line correct? 5677d06#diff-6dc9b019ec1d96c56e0e31dac3bba51cR302

This line of code was added at this pull request.

if media_type and not is_form_media_type(media_type):
empty_data = QueryDict('', encoding=self._request._encoding)
else:
empty_data = {}

Expected behavior

I suppose returning QueryDict should be only for form data type, not for application/json.

Actual behavior

In my case I was sending empty request with content-type application/json and received immutable QueryDict(). Is this correct behaviour?

@rpkilby
Copy link
Member

rpkilby commented Aug 22, 2017

Hi @Reskov. Yep - I think you're correct here. The condition should be

# this
if media_type and is_form_media_type(media_type):
    ...

# not this
if media_type and not is_form_media_type(media_type):
    ...

I did a little debugging, and it looks like the test client isn't setting the request's content_type. Because of this, the condition fails on the first part of the expression and not on the is_form_media_type check.

Digging further, this is a behavior of Django's builtin RequestFactory. The Content-Type header is only set if data is provided.


To be more explicit, this is an inconsistency between actual clients and the test client. It is possible for a client to specify a Content-Type without providing a message. Django's request factory will not set the header if no data is present.

@rpkilby
Copy link
Member

rpkilby commented Aug 31, 2017

Hi @Reskov - PR 5351 should fix the issue. If you have the time, could you install the corresponding branch and verify the expected behavior?

@Reskov
Copy link
Author

Reskov commented Aug 31, 2017

Hi @rpkilby, I already verified your branch localy, seems problem is gone, thank you

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants