determine_format: handle malformed HTTP Accept values

[acdha]

Some clients cause tastypie to return a 500 error because they send a request
with an HTTP Accept header which contains an invalid MIME type (e.g. any
value without a "/" other than "*", which mimeparse handles).

This patch simply causes tastypie to catch the ValueError and fail down to
the default format

[acdha]

You can test the current behaviour trivially:

curl -i  -H 'Accept: invalid' http://example.org/api/foo/

From what I've been able to tell, the primary source of this is a web spider run by Sogou, the Chinese search engine, with this user-agent:

Mozilla/4.0 (compatible; MSIE 7.0; Windows NT 6.1; Trident/5.0; SLCC2; .NET CLR 2.0.50727; .NET CLR 3.5.30729; .NET CLR 3.0.30729; Media Center PC 6.0; .NET4.0C; SE 2.X MetaSr 1.0)

[issackelly]

What is the accept header that they're using?

[acdha]

I didn't have that value logged before this morning so all I know is that it didn't have a /. If it happens again before I deploy the patched version of tastypie I should be able to record it.

[issackelly]

Mostly I ask because I think that there's a difference between the "right" technical thing, and the "right" user-facing behavior. Returning json for "application/xlm" is different than returning json for "".

I'm not sure that a 500 is incorrect really.

[issackelly]

Another hint: http://www.aqtronix.com/useragents/?Action=ShowAgentDetails&Name=Fake+Sogou+Search

[acdha]

I was thinking about that earlier this afternoon - arguably the best response would be a 400 since it's only possible to get this by sending an incorrectly formed request.

[issackelly]

Yup, you're right, this isn't arguable, something in the 4xx series is definitely correct. User Screwed Up.

[acdha]

Excellent - I'll be sure to return exactly that message ;)

[acdha]

I've pushed out some changes which now return an HTTP 400.

This also appears to fix another potential problem in the top_level view - while testing to confirm that the exception raised on https://github.com/acdha/django-tastypie/blob/master/tastypie/api.py#L139 was handled I noticed that there's a separate raise on https://github.com/acdha/django-tastypie/blob/master/tastypie/api.py#L146 which would similarly not be caught without the change in acdha/django-tastypie@f9e9af4

[issackelly]

LGTM thanks. I'll grab this next time I get a chance to run a couple commits.

[toastdriven]

Sorry for the delay on this one. Any chance you could update it to work post-SHA: 3ee47fb please? Otherwise looks good to merge to me.

[acdha]

Ah, I'd forgotten about this one. I'll rebase my branch against the current master

[acdha]

I just rebased it, updated for some subsequent API changes in Resource, and split the test into two tests (one for resource lists, the other for the resource itself). At this point the tests pass for me.

cd631ae is an unrelated compulsive moment but I got tired of excluding the one flake8 error in mime.py

[acdha]

@issackelly Is there anything I can do to help get this merged in? I'd like to make the next release if possible so I can stop deploying a fork.

[toastdriven]

I hate to be that guy, but there really ought to be a test in https://github.com/toastdriven/django-tastypie/blob/master/tests/core/tests/utils.py#L17 to assert what happens when a bad format is passed. Then I'd be all for shipping it.

[acdha]

Good point - the implicit checks at the view level aren't as focused.

Chris

On Friday, February 15, 2013 at 2:45 PM, Daniel Lindsley wrote:

I hate to be that guy, but there really ought to be a test in https://github.com/toastdriven/django-tastypie/blob/master/tests/core/tests/utils.py#L17 to assert what happens when a bad format is passed. Then I'd be all for shipping it.

—
Reply to this email directly or view it on GitHub (https://github.com/toastdriven/django-tastypie/pull/519#issuecomment-13624428).

[acdha]

I've updated the branch with a low-level test to confirm directly that determine_format raises a BadRequest