Bind() should guess deserialization method without reading body

[mholt]

tl;dr If a request comes in without a Content-Type, what do we do?

When the Bind() method is looking at the request to see which deserialization method to use, there is, I think, a bug if the "else" block is entered... suppose JSON was POSTed without a Content-Type. That "else" block is entered and, first, a JSON deserialization is attempted. If there are any errors, it uses form deserialization.

Two problems:

1. The JSON deserializer reads the body, so the stream is empty if and when the form deserializer tries to read it
2. The JSON deserializer may succeed, even if the Validation middleware later adds errors to the context (for example: "cannot be a negative number" for one of the fields...), so form deserialization proceeds when it should not, because there is at least one error on the context.

I'm willing to submit a fix for these, but would like some feedback first. How do we guess the Content-Type correctly and elegantly? Peek at the first character of the request body for a [ or { character to detect JSON? That seems kind of unreliable.

Thoughts?

[gravis]

I have started to write a small framework on top of martini, to have something closer to Ruby on Rails. What about:

package utils

import (
        "github.com/codegangsta/martini"
        "net/http"
        "regexp"
)

type RespondTo struct {
        ContentType string
}

func (r *RespondTo) Json() bool {
        if r.ContentType == "application/json" {
                return true
        }
        return false
}

func (r *RespondTo) Xml() bool {
        matched, err := regexp.MatchString("(application|text)/xml", r.ContentType)
        if err != nil {
                panic(err)
        }
        if matched {
                return true
        }
        return false
}

func (r *RespondTo) Html() bool {
        if !r.Xml() && !r.Json() {
                // defaults to html
                return true
        }
        return false
}

func RespondToFormat() martini.Handler {
        return func(context martini.Context, request *http.Request) {
                header := request.Header.Get("Content-Type")
                respond_to := &RespondTo{ContentType: header}
                context.Map(respond_to)
        }
}

[mholt]

Cool, yeah something like that! But what if the Content-Type isn't specified? Don't we have to read the body to figure out whether it's Form- or JSON-encoded?

[gravis]

Sending Json without a proper Content-Type header would be silly, and I vote for defaulting to HTML then (which is a decent default).

[gravis]

As bonus, here is the corresponding test:

package utils

import (
        "github.com/codegangsta/martini"
        "net/http"
        "net/http/httptest"
        "testing"
)

func TestRespondToJson(t *testing.T) {
}

func TestRespondToXml(t *testing.T) {
}

func TestRespondTo(t *testing.T) {
        var respondToTests = []struct {
                contentType string
                expected    string
        }{
                {"application/json", "json"},
                {"application/xml", "xml"},
                {"text/xml", "xml"},
                {"text/html", "html"},
                {"application/x-www-form-urlencoded", "html"},
                {"multipart/form-data", "html"},
                {"blah", "html"},
        }
        for _, tt := range respondToTests {
                m := martini.Classic()
                m.Use(RespondToFormat())
                var format string
                m.Use(func(res http.ResponseWriter, r *RespondTo) {
                        if r.Json() {
                                format = "json"
                        }
                        if r.Xml() {
                                format = "xml"
                        }
                        if r.Html() {
                                format = "html"
                        }
                        res.Write([]byte(""))
                        if tt.expected != format {
                                t.Errorf("Expected format to be %v, got %v", tt.expected, format)
                        }
                })
                response := httptest.NewRecorder()
                req, err := http.NewRequest("GET", "/", nil)
                if err != nil {
                        t.Fatal(err)
                }
                req.Header.Add("Content-Type", tt.contentType)
                m.ServeHTTP(response, req)
        }

}

Can't wait to spend more time on this :)

[mholt]

Much appreciated! But remember that this binding middleware is for requests, not responses -- for incoming data, not outgoing. In other words, all we have to figure out is whether to do JSON or Form deserialization. Fortunately we don't have to worry about receiving HTML content.

I like your idea though... maybe we shouldn't try any magic. Maybe we should just map an Overall error to the context if there is no Content-Type. After all, that is part of the protocol/spec, and the contract should be kept by both sides. That'll simplify the code and remove this whole issue.

I will default to this proposed behavior soon unless there is feedback to the contrary!

[jakejscott]

You should have a look at the way it's done with asp.net web api:

In ASP.NET Web API, a media-type formatter is an object that can read objects from an HTTP message body and write objects into an HTTP message body. The media type determines how Web API serializes and deserializes the HTTP message body. Web API has built-in support for XML, JSON, BSON, and form-urlencoded data, and you can support additional media types by writing a media formatter.

http://www.asp.net/web-api/overview/formats-and-model-binding/content-negotiation http://www.asp.net/web-api/overview/formats-and-model-binding/media-formatters http://www.asp.net/web-api/overview/formats-and-model-binding/json-and-xml-serialization

Default Content Negotiator

The DefaultContentNegotiator class provides the default implementation of IContentNegotiator. It uses several criteria to select a formatter.

First, the formatter must be able to serialize the type. This is verified by calling MediaTypeFormatter.CanWriteType.

Next, the content negotiator looks at each formatter and evaluates how well it matches the HTTP request. To evaluate the match, the content negotiator looks at two things on the formatter:

The SupportedMediaTypes collection, which contains a list of supported media types. The content negotiator tries to match this list against the request Accept header. Note that the Accept header can include ranges. For example, “text/plain” is a match for text/* or */*.
The MediaTypeMappings collection, which contains a list of MediaTypeMapping objects. The MediaTypeMapping class provides a generic way to match HTTP requests with media types. For example, it could map a custom HTTP header to a particular media type.
If there are multiple matches, the match with the highest quality factor wins. For example:

Accept: application/json, application/xml; q=0.9, */*; q=0.1
In this example, application/json has an implied quality factor of 1.0, so it is preferred over application/xml.

If no matches are found, the content negotiator tries to match on the media type of the request body, if any. For example, if the request contains JSON data, the content negotiator looks for a JSON formatter.

If there are still no matches, the content negotiator simply picks the first formatter that can serialize the type.

[mholt]

@superlogical Great feedback! Glad to see your comment here, I'm looking at that. Very interesting.

Along with a comment from the mailing list, I'm strongly considering just taking out the "guessing" feature. It's not as much a feature as a cover-up for a client-side bug: Content-Type should be specified.

[jakejscott]

@mholt You can read the code for it here: https://github.com/mono/aspnetwebstack/tree/master/src/System.Net.Http.Formatting/Formatting