implement "415 Unsupported Media Type"

[amotl]

This implements validation of the Content-Type header sent in requests against a list of allowed content types and responds with "415 Unsupported Media Type", if there's a mismatch. Could resolve #57. ;]

[almet]

I would name this valid_content_type

[amotl]

Done, with pleasure. ;] Also made it return a boolean value, so somebody else could ask if valid_content_type(...).

[almet]

Thanks for working on that, I made some comments about it.

[almet]

Any update on this issue?

[almet]

Hi Andreas,

Do you think you can find some time to review my comments and make the necessary changes to this pull request in order to make it upstream?

[amotl]

Hi Alex,

sorry for not following up in time, right now going back to this. I think the implementation is feasible, but clutters the details around more places than the current one (see implementation for "Accept"). Especially the unrolling logic in "_fallback_view" and the duplicate/redundant match in there gives headaches from a design perspective, at least from my point of view.

I'm halfway through, so you'll receive an updated changeset soon.

Nonetheless i really question if this is too much overkill. Maybe we should just skip all that jazz wiring it into the innards of Cornice, but provide an additional/standard validator which can be optionally used if desired. At least from our point of view this is what it should be in Cornice: A validator which checks for valid Content-Type and otherwise returns a nice formatted response in line with the style of request.errors.

On the other hand we see the value if this would be implemented using Pyramid predicates regarding interoperability with Pyramid applications which would rely on this. We just can't estimate the cost-benefit ratio.

FYI - our current "validator"-style code in production is:

def valid_content_type(request):

    # check for existence of 'Content-Type' header
    try:
        request.headers['Content-Type']
    except KeyError:
        request.errors.add('headers', 'Content-Type', 'Content-Type header must be given')
        return

    # check for proper value of 'Content-Type' header
    try:
        content_type = request.headers['Content-Type']
        assert content_type.startswith('application/json')
    except AssertionError:
        request.errors.add('headers', 'Content-Type', "Content-Type header should start with 'application/json', not '{0}'".format(content_type))
        return

We could put this into cornice.validators and would be fine. ;-)

Cheers,
Andreas.

P.S.: Stay tuned...

I'm halfway through, so you'll receive an updated changeset soon.

I'll finish the predicate-style implementation if nothing gets in the way, then maybe we can decide whether to provide both implementation flavors.

[amotl]

Hi Alexis,

just updated the code to follow the predicate path, you can have a look. Didn't pull from master yet, let's get things conceptually sorted first, ok?

Best wishes,
Andreas.

[almet]

Oh, you probably don't want to put that in ext. The "ext" repository is meant for "extensions".

Unless I'm missing something, this thing will be an addition to the "normal" cornice behaviors. I would rather put that in pyramid_hooks for instance, or in utils.

[amotl]

Alright!

[amotl]

Hi Alexis,

i've put all your suggestions into code. You can have a look as soon as you can catch an opportunity.

Cheers,
Andreas.

[almet]

I love this explanation. That's making things far better to understand (even if the ingress and egress terms are not that much used)

[almet]

I was off for some time, so I'm just catching things up now. That sounds great. Do you think you can rebase your work on top of the latest master so I can merge it?

[amotl]

Rebased on top of master. Travis fails on Python 3.3, but it wasn't me. ;-) Could you have a look?

[almet]

yeah, we have intermitent failures with travis and I don't know why. I'm having a look right now.

[amotl]

Thanks, had the opportunity to install a Py3k environment yesterday: repoze/repoze.debug#3 (comment)

Cornice tests on branch "master" succeed like a charm locally, but unfortunately fail on branch "content_types" with Python 3.3. I'm totally AFK today, but may be able to have a look tomorrow.

Cheers,
Andreas.

[almet]

ok, awesome.

[amotl]

Should now be working on recent Python 2.x and Python 3.x, at least it looks good to Travis.

[almet]

Awesome. We're close to there.

Can you squash the commits into something that makes more sense while reading it? That would be much appreciated and could let the history cleaner. This feature is interesting, and the history of it as well, but I don't think that (the history, not the code) is valuable enough to appear in the changelog.

You can do so by using the git rebase -i command. I can give you some help if that's needed.

[amotl]

Rebased and squashed on top of current master. Unfortunately Travis fails on downloading docutils-0.8.1.tar.gz ;[

[almet]

Yeah, Travis does what he wants when he wants. I'm thinking more and more about removing it from this project since it doesn't serve any purpose at the moment.

[almet]

Thanks for your work on this, and sorry for the delay it took to integrate!

[amotl]

Hey Alex, thanks for merging, we finally made it. :-) And thanks to @tseaver for the chance to lose my Py3k virginity.