Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[0.12 / master] POST to /write does not write points if request has header 'Content-Type: application/x-www-form-urlencoded' #6061

Closed
mark-rushakoff opened this issue Mar 20, 2016 · 5 comments
Assignees
Milestone

Comments

@mark-rushakoff
Copy link
Contributor

tl;dr: POST to /write with Content-Type: application/x-www-form-urlencoded appears as a request with an empty body to the httpd service. The client can't distinguish between a response where no points were written and a response where all points were successfully written.

I'm working from d61d75f and I've made the following modifications to my working tree while debugging why my writes have been silently failing:

diff --git a/services/httpd/handler.go b/services/httpd/handler.go
index d969b91..6fd4fcc 100644
--- a/services/httpd/handler.go
+++ b/services/httpd/handler.go
@@ -401,6 +401,7 @@ func (h *Handler) serveWrite(w http.ResponseWriter, r *http.Request, user *meta.

    // Handle gzip decoding of the body
    body := r.Body
+   fmt.Printf("Body: %#v\n", body)
    if r.Header.Get("Content-encoding") == "gzip" {
        b, err := gzip.NewReader(r.Body)
        if err != nil {
@@ -412,6 +413,7 @@ func (h *Handler) serveWrite(w http.ResponseWriter, r *http.Request, user *meta.
    }

    var bs []byte
+   fmt.Printf("Headers: %#v\n", r.Header)
    if clStr := r.Header.Get("Content-Length"); clStr != "" {
        if length, err := strconv.Atoi(clStr); err == nil {
            // This will just be an initial hint for the gzip reader, as the
@@ -436,6 +438,9 @@ func (h *Handler) serveWrite(w http.ResponseWriter, r *http.Request, user *meta.
    }

    points, parseError := models.ParsePointsWithPrecision(buf.Bytes(), time.Now().UTC(), r.FormValue("precision"))
+   fmt.Printf("Req body: %#v\n", string(buf.Bytes()))
+   fmt.Printf("HTTP points: %#v\n", points)
+   fmt.Printf("parseError: %#v\n", parseError)
    // Not points parsed correctly so return the error now
    if parseError != nil && len(points) == 0 {
        if parseError.Error() == "EOF" {

First, a write via curl like the examples show:

→ curl -vvv -X POST 'http://localhost:8086/write?db=tmp' --data-binary 'test,src=curl_NoCustomHeaders x=1'
*   Trying ::1...
* Connected to localhost (::1) port 8086 (#0)
> POST /write?db=tmp HTTP/1.1
> Host: localhost:8086
> User-Agent: curl/7.43.0
> Accept: */*
> Content-Length: 33
> Content-Type: application/x-www-form-urlencoded
>
* upload completely sent off: 33 out of 33 bytes
< HTTP/1.1 204 No Content
< Request-Id: 9327baba-eeb4-11e5-8006-000000000000
< X-Influxdb-Version: unknown
< Date: Sun, 20 Mar 2016 15:58:23 GMT
<
* Connection #0 to host localhost left intact

A 204 response here implies to the client that all the points have been written successfully, but SELECT * FROM tmp returns no results, and the manually added log shows that the HTTP server did not see any points because it detected an empty request body:

[http] 2016/03/20 08:57:27 ::1 - - [20/Mar/2016:08:57:27 -0700] GET /query?db=tmp&epoch=ns&q=show+series HTTP/1.1 200 93 - InfluxDBShell/0.9 7198b29e-eeb4-11e5-8005-000000000000 477.06µs
Body: &http.body{src:(*io.LimitedReader)(0xc8208f28e0), hdr:interface {}(nil), r:(*bufio.Reader)(nil), closing:false, doEarlyClose:true, mu:sync.Mutex{state:0, sema:0x0}, sawEOF:true, closed:false, earlyClose:false, onHitEOF:(func())(nil)}
Headers: http.Header{"User-Agent":[]string{"curl/7.43.0"}, "Accept":[]string{"*/*"}, "Content-Length":[]string{"33"}, "Content-Type":[]string{"application/x-www-form-urlencoded"}, "Request-Id":[]string{"9327baba-eeb4-11e5-8006-000000000000"}}
Req body: ""
HTTP points: []models.Point{}
parseError: <nil>
[http] 2016/03/20 08:58:23 ::1 - - [20/Mar/2016:08:58:23 -0700] POST /write?db=tmp HTTP/1.1 204 0 - curl/7.43.0 9327baba-eeb4-11e5-8006-000000000000 251.714µs

Repeating the same curl command with the addition of a Content-Type header:

curl -vvv -X POST 'http://localhost:8086/write?db=tmp' --data-binary 'test,src=curl_CustomContentType x=1' -H 'Content-Type: text/plain'
*   Trying ::1...
* Connected to localhost (::1) port 8086 (#0)
> POST /write?db=tmp HTTP/1.1
> Host: localhost:8086
> User-Agent: curl/7.43.0
> Accept: */*
> Content-Type: text/plain
> Content-Length: 35
>
* upload completely sent off: 35 out of 35 bytes
< HTTP/1.1 204 No Content
< Request-Id: 7b6ccdbb-eeb5-11e5-8008-000000000000
< X-Influxdb-Version: unknown
< Date: Sun, 20 Mar 2016 16:04:53 GMT
<
* Connection #0 to host localhost left intact

The HTTP log now indicates there was a point present and that the request body was not empty:

[http] 2016/03/20 08:58:23 ::1 - - [20/Mar/2016:08:58:23 -0700] POST /write?db=tmp HTTP/1.1 204 0 - curl/7.43.0 9327baba-eeb4-11e5-8006-000000000000 251.714µs
[query] 2016/03/20 09:02:23 SELECT * FROM tmp."default".tmp
[http] 2016/03/20 09:02:23 ::1 - - [20/Mar/2016:09:02:23 -0700] GET /query?db=tmp&epoch=ns&q=select+%2A+from+tmp HTTP/1.1 200 40 - InfluxDBShell/0.9 2231a45c-eeb5-11e5-8007-000000000000 773.867µs
Body: &http.body{src:(*io.LimitedReader)(0xc8204ad920), hdr:interface {}(nil), r:(*bufio.Reader)(nil), closing:false, doEarlyClose:true, mu:sync.Mutex{state:0, sema:0x0}, sawEOF:false, closed:false, earlyClose:false, onHitEOF:(func())(nil)}
Headers: http.Header{"Content-Length":[]string{"35"}, "Request-Id":[]string{"7b6ccdbb-eeb5-11e5-8008-000000000000"}, "User-Agent":[]string{"curl/7.43.0"}, "Accept":[]string{"*/*"}, "Content-Type":[]string{"text/plain"}}
Req body: "test,src=curl_CustomContentType x=1"
HTTP points: []models.Point{(*models.point)(0xc8202a3710)}
parseError: <nil>

Inserting a point via the influx CLI still works, apparently because it sets Content-Type header to an empty string:

[http] 2016/03/20 09:06:48 ::1 - - [20/Mar/2016:09:06:48 -0700] GET /query?db=tmp&epoch=ns&q=select+%2A+from+test HTTP/1.1 200 158 - InfluxDBShell/0.9 c0115f6b-eeb5-11e5-800a-000000000000 605.561µs
Body: &http.body{src:(*io.LimitedReader)(0xc8208c0e00), hdr:interface {}(nil), r:(*bufio.Reader)(nil), closing:false, doEarlyClose:true, mu:sync.Mutex{state:0, sema:0x0}, sawEOF:false, closed:false, earlyClose:false, onHitEOF:(func())(nil)}
Headers: http.Header{"Request-Id":[]string{"0bdda8fe-eeb6-11e5-800b-000000000000"}, "User-Agent":[]string{"InfluxDBShell/0.9"}, "Content-Length":[]string{"25"}, "Content-Type":[]string{""}, "Accept-Encoding":[]string{"gzip"}}
Req body: " test,src=influx_cli x=2\n"
HTTP points: []models.Point{(*models.point)(0xc8203c1200)}
parseError: <nil>

There are at least two issues here:

  • First, failure to parse points when a request has Content-Type: application/x-www-form-urlencoded is a regression from previous behavior. That Content-Type is almost certainly the wrong one to use, but it's the default for most HTTP clients' POSTs if the user doesn't specify otherwise, so it seems important that InfluxDB accepts line protocol with that header. (I dug through net/http source for a bit but wasn't able to conclude anything; it might be related to the early call to (*http.Request).FormValue() before evaluating (*http.Request).Body. This blog post might also be a pointer in the right direction.)
  • Second, the server response for 0 points written is indistinguishable from the response for all points written. A 204 response still seems appropriate, but maybe a custom header in the response like X-Influxdb-Points-Written: 123 would allow the client to confirm that the request completed as expected.
@joelegasse
Copy link
Contributor

@mark-rushakoff Try replacing calls to FormValue() with r.URL.Query().Get(), since we never actually want to attempt to decode the body as a url-encoded form.

@mark-rushakoff
Copy link
Contributor Author

@joelegasse Yep, that fixes it. There's a few other uses of FormValue in serveWrite, and it's probably worth writing a test to make sure we don't regress on Content-Type: application/x-www-form-urlencoded, since it's common for HTTP client libraries to default to that.

Any thoughts on /write reporting back how many points were written?

@gunnaraasen
Copy link
Member

@mark-rushakoff I like the idea of returning the points written and a custom header seems like appropriate. How about a Influxdb-Points-Failed header as well? I imagine most people looking at that header will be trying to account for missing points.

@jwilder jwilder added this to the 0.12.0 milestone Mar 28, 2016
@joelegasse joelegasse self-assigned this Mar 30, 2016
joelegasse added a commit that referenced this issue Mar 30, 2016
FormValue() would attempt to parse the body of a request when the
content-type is set to `application/x-www-form-urlencoded`. The write
handler never wants url-encoded forms, and should only ever check the
URL for query parameters.

Fixes #6061
@ReinsBrain
Copy link

it would be nice if it was specified what Content-Type should be for posts... I'm searching all over and I can't find any mention of it aside what it should not be...

@karanrajpal14
Copy link

@ReinsBrain I agree. It took me so long to get it to send data to the database and now no data is being written. I keep getting "No Content Found" errors. It's sad that the documentation of the HTTP API is so poor.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants