Issue #88 open

@validate decorator is broken by translate_mime

James Fisher
created an issue

The Invalidate decorator currently works on the GET, POST, PUT variables of the request object. This is currently depending on which is given as a parameter to the decorator.

We may POST/PUT not just with a standard attr/val string, but also with structured data: XML, json, etc. The way this is currently handled is to parse this request.raw_post_data and put it in request.data as a standard dictionary.

For example, I cURL the following:

{{{

!shell

curl -X POST -H 'Content-Type: application/json' -d '{"name": "James", "age": 22}' http://localhost:8000/people }}}

The first significant thing that happens to my data (see resource.py 109) is that translate_mime() is called on it. This successfully interprets the request.raw_post_data as JSON, and places the corresponding dictionary in request.data.

In my handlers.py, I indicate that the request should be validated and that it is POST data:

{{{

!python

@validate(PersonForm, 'POST')
def update(self, request, slug):
        ...

}}}

Invalidate then faithfully looks in request.POST. Except, of course, the data to validate isn't there, because request.POST actually looks like a dictionary in the form

{{{

!python

{ 'some_encoded_json_data': '' } }}}

Invalidate should in fact be looking at request.data, because that's where translate_mime put it. validate() currently looks like: {{{

!python

def validate(v_form, operation='POST'): decorator def wrap(f, self, request, a, *kwa): form = v_form(getattr(request, operation)) ... }}}

And if I alter it to:

{{{

!python

def validate(v_form): decorator def wrap(f, self, request, a, *kwa): form = v_form(request.data) ... }}}

Then everything works just dandy for me. The problem that I've created by doing that, though, is that not everyone is going to be passing their POST/PUT data in a structured format. So I think the function should first check for request.data, and only if it isn't there, then use the operation parameter given. So:

{{{

!python

def validate(v_form, operation='POST'): decorator def wrap(f, self, request, a, *kwa): try: dictionary = request.data except AttributeError: dictionary = getattr(request, operation) form = v_form(dictionary) }}}

However, this still seems like a lazy solution. Why? Because in my own create() function, I still have to be aware whether to use request.POST or request.data, which in turn depends on whether the client has decided to submit their new object in JSON encoding or in a standard GET string encoding (application/x-www-form-urlencoded).

To solve this I suggest populating request.data with the same data as that held in request.POST or request.PUT. This requires a bit of playing around with Mimer.translate() to become:

{{{

!python

def translate(self): 
    ctype = self.content_type()
    loadee = self.loader_for_type(ctype)

    self.request.content_type = ctype

    if not self.is_multipart() and loadee:
        try:
            self.request.data = loadee(self.request.raw_post_data)

            # Reset both POST and PUT from request, as its
            # misleading having their presence around.
            self.request.POST = self.request.PUT = dict()
        except (TypeError, ValueError):
            # This also catches if loadee is None.
            raise MimerDataException
    else:
        # Load self.data with the standard dictionary from request.POST or request.PUT, as appropriate
        self.request.data = getattr(self.request, self.request.method)

    return self.request

}}}

This in turn requires a slight adjustment of Mimer.loader_for_type, as it could operate on a ctype of value None. It is now:

{{{

!python

def loader_for_type(self, ctype):
    if not ctype:
        return None
    for loadee, mimes in Mimer.TYPES.iteritems():
        for mime in mimes:
            if ctype.startswith(mime):
                return loadee

}}}

Does all this make sense? Is there a massive thing I don't get?

Best.

Comments (2)

  1. Jesper Nøhr repo owner
    • changed status to open

    Good observations, well done. I would prefer if you apply your changes in a fork, so we can merge them in easily. That'll also allow you to run the testsuite on your changes, making sure we don't break anything in the process.

  2. Log in to comment