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

net/http/httputil: DumpResponse does not print response headers for PUT and DELETE methods. #13942

Closed
harshavardhana opened this issue Jan 13, 2016 · 10 comments

Comments

@harshavardhana
Copy link
Contributor

------------
PUT /i9ap2pe5jminnyumwykxo5fnk3yby3-resumable?partNumber=3&uploadId=T5co25TsHJjHiJXvgohhwpmhh_pmgsrsyeVVePSuIOyiwLlUMM91nfNnICROAMYV5aEWfiSytYKFYUlC6bLMPC3HozM.LwpzCCpLNnBDT_EpgFdY9us06aMrDsZgMOS9 HTTP/1.1
Host: i9ap2pe5jminnyumwykxo5fnk3yby3.s3.amazonaws.com
User-Agent: Minio (darwin; amd64) minio-go/0.2.5 Minio-go-FunctionalTest/0.1.0
Content-Length: 1048576
Authorization: AWS4-HMAC-SHA256 Credential=**REDACTED**/20160113/us-east-1/s3/aws4_request, SignedHeaders=content-md5;host;x-amz-content-sha256;x-amz-date, Signature=**REDACTED**
Content-Md5: 6y753xYMK8gkKwNnaB9Fcg==
X-Amz-Content-Sha256: 0decd2605c3aa43569b6669ff36f4b06700ed24afef5c2508028b84deb2b75cb
X-Amz-Date: 20160113T223000Z
Accept-Encoding: gzip

HTTP/1.1 200 OK
---------
POST /i9ap2pe5jminnyumwykxo5fnk3yby3-resumable?uploadId=T5co25TsHJjHiJXvgohhwpmhh_pmgsrsyeVVePSuIOyiwLlUMM91nfNnICROAMYV5aEWfiSytYKFYUlC6bLMPC3HozM.LwpzCCpLNnBDT_EpgFdY9us06aMrDsZgMOS9 HTTP/1.1
Host: i9ap2pe5jminnyumwykxo5fnk3yby3.s3.amazonaws.com
User-Agent: Minio (darwin; amd64) minio-go/0.2.5 Minio-go-FunctionalTest/0.1.0
Content-Length: 495
Authorization: AWS4-HMAC-SHA256 Credential=**REDACTED**/20160113/us-east-1/s3/aws4_request, SignedHeaders=host;x-amz-content-sha256;x-amz-date, Signature=**REDACTED**
X-Amz-Content-Sha256: abaeeb4bf38deca95412cf12176cd19e56686e48234ea9ee6075ac4cce20f201
X-Amz-Date: 20160113T223001Z
Accept-Encoding: gzip

HTTP/1.1 200 OK
Transfer-Encoding: chunked
Content-Type: application/xml
Date: Wed, 13 Jan 2016 22:30:02 GMT
Server: AmazonS3
X-Amz-Id-2: FJBpP8GYLapHA0nWyxCCnbly91A7Y+MrEx96H5e3YvFJSb0dx7CfZ2VBIbPkm/za
X-Amz-Request-Id: 5B725F657747BECC
-----------------

-----------------
DELETE /i9ap2pe5jminnyumwykxo5fnk3yby3-resumable HTTP/1.1
Host: i9ap2pe5jminnyumwykxo5fnk3yby3.s3.amazonaws.com
User-Agent: Minio (darwin; amd64) minio-go/0.2.5 Minio-go-FunctionalTest/0.1.0
Authorization: AWS4-HMAC-SHA256 Credential=**REDACTED**/20160113/us-east-1/s3/aws4_request, SignedHeaders=host;x-amz-content-sha256;x-amz-date, Signature=**REDACTED**
X-Amz-Content-Sha256: e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855
X-Amz-Date: 20160113T223001Z
Accept-Encoding: gzip

HTTP/1.1 204 No Content
-----------------
@harshavardhana
Copy link
Contributor Author

$ git diff
diff --git a/src/net/http/httputil/dump.go b/src/net/http/httputil/dump.go
index 6fe8fea..b0f6289 100644
--- a/src/net/http/httputil/dump.go
+++ b/src/net/http/httputil/dump.go
@@ -267,7 +267,7 @@ var errNoBody = errors.New("sentinel error value")
 // from reading the dummy body.
 type failureToReadBody struct{}

-func (failureToReadBody) Read([]byte) (int, error) { return 0, errNoBody }
+func (failureToReadBody) Read([]byte) (int, error) { return 0, io.EOF }
 func (failureToReadBody) Close() error             { return nil }

 var emptyBody = ioutil.NopCloser(strings.NewReader(""))

@harshavardhana
Copy link
Contributor Author

A simple fix fixes this, wanted to know if this is correct. or Should i change this failuretoReadBody{} to eofReader{}

@bradfitz bradfitz changed the title net/http: httputil.DumpResponse does not print response headers for PUT and DELETE methods. net/http/httputil: DumpResponse does not print response headers for PUT and DELETE methods. Jan 13, 2016
@bradfitz
Copy link
Contributor

I don't understand what the actual bug report is from your paste dump above. Please describe in words & also provide full sample code to reproduce, and explain your expected results. I don't know what you hope to see, and don't know how you arrived at the thing you're apparently unhappy with.

@bradfitz bradfitz added this to the Unplanned milestone Jan 13, 2016
@harshavardhana
Copy link
Contributor Author

I don't understand what the actual bug report is from your paste dump above. Please describe in words & also provide full sample code to reproduce, and explain your expected results. I don't know what you hope to see, and don't know how you arrived at the thing you're apparently unhappy with.

Sure let me do that. Sorry about not pasting enough info.

@harshavardhana
Copy link
Contributor Author

Here is a sample code and output.

package main

import (
    "bytes"
    "fmt"
    "log"
    "net/http"
    "net/http/httputil"
)

func main() {
    clnt := &http.Client{}
    req, err := http.NewRequest("PUT", "https://s3.amazonaws.com/miniocloud/new-file", bytes.NewReader([]byte("Hello world")))
    if err != nil {
        log.Fatalln(err)
    }
    resp, err := clnt.Do(req)
    if err != nil {
        log.Fatalln(err)
    }
    b, err := httputil.DumpRequestOut(req, false)
    if err != nil {
        log.Fatalln(err)
    }
    fmt.Println(string(b))
    b, err = httputil.DumpResponse(resp, false)
    if err != nil {
        log.Fatalln(err)
    }
    fmt.Println(string(b))

    req, err = http.NewRequest("GET", "https://s3.amazonaws.com/miniocloud/new-file", nil)
    if err != nil {
        log.Fatalln(err)
    }
    resp, err = clnt.Do(req)
    if err != nil {
        log.Fatalln(err)
    }
    b, err = httputil.DumpRequestOut(req, false)
    if err != nil {
        log.Fatalln(err)
    }
    fmt.Println(string(b))

    b, err = httputil.DumpResponse(resp, false)
    if err != nil {
        log.Fatalln(err)
    }
    fmt.Println(string(b))
}
$ go run put.go
PUT /miniocloud/new-file HTTP/1.1
Host: s3.amazonaws.com
User-Agent: Go-http-client/1.1
Content-Length: 11
Accept-Encoding: gzip


HTTP/1.1 200 OK

GET /miniocloud/new-file HTTP/1.1
Host: s3.amazonaws.com
User-Agent: Go-http-client/1.1
Accept-Encoding: gzip


HTTP/1.1 200 OK
Content-Length: 11
Accept-Ranges: bytes
Content-Type: binary/octet-stream
Date: Wed, 13 Jan 2016 23:02:41 GMT
Etag: "3e25960a79dbc69b674cd4ec67a72c62"
Last-Modified: Wed, 13 Jan 2016 23:02:41 GMT
Server: AmazonS3
X-Amz-Id-2: 0m6vLlu/8EgEGPvOK+j05C+o3J5zm+/C9HSyHmdIqAB6MqlaebFeSX5Kid2+b83TgKTrw3X2tFU=
X-Amz-Request-Id: 60B0326178C85B1A

@harshavardhana
Copy link
Contributor Author

If you observe PUT is not printing its response headers at all.

@harshavardhana
Copy link
Contributor Author

This actually happens because of few conditions

  • A successful PUT with response Content-Length set to '0'
  • func (failureToReadBody) Read([]byte) (int, error) { return 0, errNoBody }- This return upon Read.
  • Third this condition in response.Write()
   if r1.ContentLength == 0 && r1.Body != nil {
        // Is it actually 0 length? Or just unknown?
        var buf [1]byte
        n, err := r1.Body.Read(buf[:])
        if err != nil && err != io.EOF {
            return err  ---> Returns right here. 
        }

@harshavardhana
Copy link
Contributor Author

Just changing

  • func (failureToReadBody) Read([]byte) (int, error) { return 0, errNoBody }

errNoBody to io.EOF fixes this problem, but trying to fix the tests which are failing.

@gopherbot
Copy link

CL https://golang.org/cl/18624 mentions this issue.

@harshavardhana
Copy link
Contributor Author

With this change fixes our problem, here is the output for the sample program now.

$ ./bin/go run ~/put.go
PUT /miniocloud/new-file HTTP/1.1
Host: s3.amazonaws.com
User-Agent: Go-http-client/1.1
Content-Length: 11
Accept-Encoding: gzip


HTTP/1.1 200 OK
Content-Length: 0
Date: Thu, 14 Jan 2016 03:04:42 GMT
Etag: "3e25960a79dbc69b674cd4ec67a72c62"
Server: AmazonS3
X-Amz-Id-2: qnXyH6sknlovV0Myy3emFAXTNtI/sQIcu1ZXNq/6wd17K32tQ7WNGB1qb3nzCpW2DhfeZ/MbWfw=
X-Amz-Request-Id: 8422EACB0CC492BD


GET /miniocloud/new-file HTTP/1.1
Host: s3.amazonaws.com
User-Agent: Go-http-client/1.1
Accept-Encoding: gzip


HTTP/1.1 200 OK
Content-Length: 11
Accept-Ranges: bytes
Content-Type: binary/octet-stream
Date: Thu, 14 Jan 2016 03:04:42 GMT
Etag: "3e25960a79dbc69b674cd4ec67a72c62"
Last-Modified: Thu, 14 Jan 2016 03:04:42 GMT
Server: AmazonS3
X-Amz-Id-2: tBddirbFYZ4LOTodUoyqJlVfvhJOqshtJ/YCO+6TZ8s9ZRtocnvGRkZ4uYTwctSXd84XS95hCds=
X-Amz-Request-Id: 1EF96AD337008115

harshavardhana pushed a commit to harshavardhana/mc that referenced this issue May 7, 2016
Due to a golang bug need to add specific code to handle this
properly when the response body has ContentLength of '0'

Ref for more info : golang/go#13942.
abperiasamy pushed a commit to minio/mc that referenced this issue May 7, 2016
Due to a golang bug need to add specific code to handle this
properly when the response body has ContentLength of '0'

Ref for more info : golang/go#13942.
@golang golang locked and limited conversation to collaborators Feb 28, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants