go-macaron / binding

Package binding is a middleware that provides request data binding and validation for Macaron.
Apache License 2.0
23 stars 17 forks source link

GET requests with content-type header set result in errors #29

Closed Dieterbe closed 5 years ago

Dieterbe commented 5 years ago

hello, a GET request has no body, yet the HTTP RFC allows to set content-type on GET requests (https://tools.ietf.org/html/rfc7231#section-3.1.1.5) in this case it simply does not apply. We see this commonly with customers that switch requests between GET and POST, but leaving the content-type header specified. For GET requests, it should simply have no effect.

However, the binding middleware will, upon detection of a non-zero content-type header, switch to trying to use the json decoder on the body, even when there is no body such as in a GET request. This results in errors such as https://github.com/grafana/metrictank/issues/1465 :

http  'http://localhost:6060/render?target=stats.docker-env.response.200&from=-6s' 'Content-Type:application/json'
HTTP/1.1 422 Unprocessable Entity
Content-Encoding: gzip
Content-Length: 96
Content-Type: application/json; charset=utf-8
Date: Wed, 18 Sep 2019 14:29:00 GMT
Vary: Accept-Encoding

[
    {
        "classification": "RequiredError",
        "fieldNames": [
            "target"
        ],
        "message": "Required"
    }
]

the correct behavior would be to process the request based on its method (e.g. GET requests use GET parameters)

unknwon commented 5 years ago

Should I tag a new release or it will be fine as it is?

Dieterbe commented 5 years ago

hey, a new release would be helpful. thanks

unknwon commented 5 years ago

Just tagged: https://github.com/go-macaron/binding/releases/tag/v1.0.0