Merge pull request #537 from harshavardhana/pr_out_add_proper_content_length_for_error_and_success_responses

pkg/api/api_bucket_handlers.go

@@ -18,6 +18,7 @@ package api
 
 import (
 	"net/http"
+	"strconv"
 
 	"github.com/gorilla/mux"
 	"github.com/minio-io/minio/pkg/iodine"
@@ -88,12 +89,15 @@ func (server *minioAPI) listObjectsHandler(w http.ResponseWriter, req *http.Requ
 	switch err := iodine.ToError(err).(type) {
 	case nil: // success
 		{
+			// generate response
+			response := generateListObjectsResponse(bucket, objects, resources)
+			encodedSuccessResponse := encodeSuccessResponse(response, acceptsContentType)
 			// write headers
 			setCommonHeaders(w, getContentTypeString(acceptsContentType))
+			// set content-length to the size of the body
+			w.Header().Set("Content-Length", strconv.Itoa(len(encodedSuccessResponse)))
 			w.WriteHeader(http.StatusOK)
 			// write body
-			response := generateObjectsListResult(bucket, objects, resources)
-			encodedSuccessResponse := encodeSuccessResponse(response, acceptsContentType)
 			w.Write(encodedSuccessResponse)
 		}
 	case drivers.ObjectNotFound:
@@ -128,12 +132,15 @@ func (server *minioAPI) listBucketsHandler(w http.ResponseWriter, req *http.Requ
 	switch err := iodine.ToError(err).(type) {
 	case nil:
 		{
-			response := generateBucketsListResult(buckets)
+			// generate response
+			response := generateListBucketsResponse(buckets)
+			encodedSuccessResponse := encodeSuccessResponse(response, acceptsContentType)
 			// write headers
 			setCommonHeaders(w, getContentTypeString(acceptsContentType))
+			// set content-length to the size of the body
+			w.Header().Set("Content-Length", strconv.Itoa(len(encodedSuccessResponse)))
 			w.WriteHeader(http.StatusOK)
 			// write response
-			encodedSuccessResponse := encodeSuccessResponse(response, acceptsContentType)
 			w.Write(encodedSuccessResponse)
 		}
 	default:
@@ -172,7 +179,9 @@ func (server *minioAPI) putBucketHandler(w http.ResponseWriter, req *http.Reques
 	switch iodine.ToError(err).(type) {
 	case nil:
 		{
-			writeSuccessResponse(w)
+			// Make sure to add Location information here only for bucket
+			w.Header().Set("Location", "/"+bucket)
+			writeSuccessResponse(w, acceptsContentType)
 		}
 	case drivers.TooManyBuckets:
 		{
@@ -217,7 +226,7 @@ func (server *minioAPI) putBucketACLHandler(w http.ResponseWriter, req *http.Req
 	switch iodine.ToError(err).(type) {
 	case nil:
 		{
-			writeSuccessResponse(w)
+			writeSuccessResponse(w, acceptsContentType)
 		}
 	case drivers.BucketNameInvalid:
 		{
@@ -254,5 +263,5 @@ func (server *minioAPI) headBucketHandler(w http.ResponseWriter, req *http.Reque
 	}
 
 	// Always a success if isValidOp succeeds
-	writeSuccessResponse(w)
+	writeSuccessResponse(w, acceptsContentType)
 }

pkg/api/api_definitions.go

@@ -25,8 +25,8 @@ const (
 	maxObjectList = 1000
 )
 
-// ObjectListResponse format
-type ObjectListResponse struct {
+// ListObjectsResponse - format for list objects response
+type ListObjectsResponse struct {
 	XMLName        xml.Name `xml:"ListBucketResult" json:"-"`
 	Name           string
 	Prefix         string
@@ -38,8 +38,8 @@ type ObjectListResponse struct {
 	CommonPrefixes []*Prefix
 }
 
-// BucketListResponse - bucket list response format
-type BucketListResponse struct {
+// ListBucketsResponse - format for list buckets response
+type ListBucketsResponse struct {
 	XMLName xml.Name `xml:"ListAllMyBucketsResult" json:"-"`
 	Owner   Owner
 	Buckets struct {
@@ -75,7 +75,7 @@ type Owner struct {
 }
 
 // List of not implemented bucket queries
-var unimplementedBucketResourceNames = map[string]bool{
+var notimplementedBucketResourceNames = map[string]bool{
 	"policy":         true,
 	"cors":           true,
 	"lifecycle":      true,
@@ -91,7 +91,7 @@ var unimplementedBucketResourceNames = map[string]bool{
 }
 
 // List of not implemented object queries
-var unimplementedObjectResourceNames = map[string]bool{
+var notimplementedObjectResourceNames = map[string]bool{
 	"uploadId": true,
 	"torrent":  true,
 	"uploads":  true,

pkg/api/api_generic_handlers.go

@@ -162,7 +162,7 @@ func (h resourceHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 		writeErrorResponse(w, r, NotAcceptable, acceptsContentType, r.URL.Path)
 		return
 	}
-	if ignoreUnImplementedObjectResources(r) || ignoreUnImplementedBucketResources(r) {
+	if ignoreNotImplementedObjectResources(r) || ignoreNotImplementedBucketResources(r) {
 		error := getErrorCode(NotImplemented)
 		errorResponse := getErrorResponse(error, "")
 		setCommonHeaders(w, getContentTypeString(acceptsContentType))
@@ -175,22 +175,22 @@ func (h resourceHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 
 //// helpers
 
-// Checks requests for unimplemented Bucket resources
-func ignoreUnImplementedBucketResources(req *http.Request) bool {
+// Checks requests for not implemented Bucket resources
+func ignoreNotImplementedBucketResources(req *http.Request) bool {
 	q := req.URL.Query()
 	for name := range q {
-		if unimplementedBucketResourceNames[name] {
+		if notimplementedBucketResourceNames[name] {
 			return true
 		}
 	}
 	return false
 }
 
-// Checks requests for unimplemented Object resources
-func ignoreUnImplementedObjectResources(req *http.Request) bool {
+// Checks requests for not implemented Object resources
+func ignoreNotImplementedObjectResources(req *http.Request) bool {
 	q := req.URL.Query()
 	for name := range q {
-		if unimplementedObjectResourceNames[name] {
+		if notimplementedObjectResourceNames[name] {
 			return true
 		}
 	}

pkg/api/api_object_handlers.go

@@ -175,7 +175,26 @@ func (server *minioAPI) putObjectHandler(w http.ResponseWriter, req *http.Reques
 	switch err := iodine.ToError(err).(type) {
 	case nil:
 		{
-			writeSuccessResponse(w)
+			metadata, err := server.driver.GetObjectMetadata(bucket, object, "")
+			switch err := iodine.ToError(err).(type) {
+			case nil:
+				w.Header().Set("ETag", metadata.Md5)
+				writeSuccessResponse(w, acceptsContentType)
+			case drivers.ObjectNotFound:
+				{
+					writeErrorResponse(w, req, NoSuchKey, acceptsContentType, req.URL.Path)
+				}
+			case drivers.ObjectNameInvalid:
+				{
+					writeErrorResponse(w, req, NoSuchKey, acceptsContentType, req.URL.Path)
+				}
+			default:
+				{
+					log.Error.Println(iodine.New(err, nil))
+					writeErrorResponse(w, req, InternalError, acceptsContentType, req.URL.Path)
+				}
+			}
+
 		}
 	case drivers.ObjectExists:
 		{

pkg/api/api_response.go

@@ -19,6 +19,7 @@ package api
 import (
 	"net/http"
 	"sort"
+	"strconv"
 
 	"github.com/minio-io/minio/pkg/storage/drivers"
 )
@@ -34,9 +35,9 @@ const (
 //
 // output:
 // populated struct that can be serialized to match xml and json api spec output
-func generateBucketsListResult(buckets []drivers.BucketMetadata) BucketListResponse {
+func generateListBucketsResponse(buckets []drivers.BucketMetadata) ListBucketsResponse {
 	var listbuckets []*Bucket
-	var data = BucketListResponse{}
+	var data = ListBucketsResponse{}
 	var owner = Owner{}
 
 	owner.ID = "minio"
@@ -70,11 +71,11 @@ func (b itemKey) Less(i, j int) bool { return b[i].Key < b[j].Key }
 //
 // output:
 // populated struct that can be serialized to match xml and json api spec output
-func generateObjectsListResult(bucket string, objects []drivers.ObjectMetadata, bucketResources drivers.BucketResourcesMetadata) ObjectListResponse {
+func generateListObjectsResponse(bucket string, objects []drivers.ObjectMetadata, bucketResources drivers.BucketResourcesMetadata) ListObjectsResponse {
 	var contents []*Item
 	var prefixes []*Prefix
 	var owner = Owner{}
-	var data = ObjectListResponse{}
+	var data = ListObjectsResponse{}
 
 	owner.ID = "minio"
 	owner.DisplayName = "minio"
@@ -110,20 +111,23 @@ func generateObjectsListResult(bucket string, objects []drivers.ObjectMetadata,
 }
 
 // writeSuccessResponse - write success headers
-func writeSuccessResponse(w http.ResponseWriter) {
-	w.Header().Set("Server", "Minio")
-	w.Header().Set("Connection", "close")
+func writeSuccessResponse(w http.ResponseWriter, acceptsContentType contentType) {
+	setCommonHeaders(w, getContentTypeString(acceptsContentType))
 	w.WriteHeader(http.StatusOK)
 }
 
 // writeErrorRespone - write error headers
 func writeErrorResponse(w http.ResponseWriter, req *http.Request, errorType int, acceptsContentType contentType, resource string) {
 	error := getErrorCode(errorType)
+	// generate error response
 	errorResponse := getErrorResponse(error, resource)
-	// set headers
+	encodedErrorResponse := encodeErrorResponse(errorResponse, acceptsContentType)
+	// set common headers
 	setCommonHeaders(w, getContentTypeString(acceptsContentType))
+	// set content-length to size of error response
+	w.Header().Set("Content-Length", strconv.Itoa(len(encodedErrorResponse)))
+	// set headers
 	w.WriteHeader(error.HTTPStatusCode)
 	// write body
-	encodedErrorResponse := encodeErrorResponse(errorResponse, acceptsContentType)
 	w.Write(encodedErrorResponse)
 }

pkg/api/api_test.go

@@ -168,6 +168,7 @@ func (s *MySuite) TestEmptyObject(c *C) {
 	}
 	typedDriver.On("CreateBucket", "bucket", "private").Return(nil).Once()
 	typedDriver.On("CreateObject", "bucket", "object", "", "", mock.Anything).Return(nil).Once()
+	typedDriver.On("GetObjectMetadata", "bucket", "object", "").Return(metadata, nil).Once()
 	typedDriver.On("GetBucketMetadata", "bucket").Return(drivers.BucketMetadata{}, nil).Twice()
 	typedDriver.On("GetObjectMetadata", "bucket", "object", "").Return(metadata, nil).Once()
 	typedDriver.On("GetObject", mock.Anything, "bucket", "object").Return(int64(0), nil).Once()
@@ -179,6 +180,7 @@ func (s *MySuite) TestEmptyObject(c *C) {
 	buffer := bytes.NewBufferString("")
 	driver.CreateBucket("bucket", "private")
 	driver.CreateObject("bucket", "object", "", "", buffer)
+	driver.GetObjectMetadata("bucket", "object", "")
 
 	request, err := http.NewRequest("GET", testServer.URL+"/bucket/object", nil)
 	c.Assert(err, IsNil)
@@ -250,6 +252,7 @@ func (s *MySuite) TestObject(c *C) {
 	}
 	typedDriver.On("CreateBucket", "bucket", "private").Return(nil).Once()
 	typedDriver.On("CreateObject", "bucket", "object", "", "", mock.Anything).Return(nil).Once()
+	typedDriver.On("GetObjectMetadata", "bucket", "object", "").Return(metadata, nil).Twice()
 	typedDriver.On("GetBucketMetadata", "bucket").Return(drivers.BucketMetadata{}, nil).Twice()
 	typedDriver.On("GetObjectMetadata", "bucket", "object", "").Return(metadata, nil).Twice()
 	typedDriver.SetGetObjectWriter("bucket", "object", []byte("hello world"))
@@ -262,6 +265,7 @@ func (s *MySuite) TestObject(c *C) {
 	buffer := bytes.NewBufferString("hello world")
 	driver.CreateBucket("bucket", "private")
 	driver.CreateObject("bucket", "object", "", "", buffer)
+	driver.GetObjectMetadata("bucket", "object", "")
 
 	request, err := http.NewRequest("GET", testServer.URL+"/bucket/object", nil)
 	c.Assert(err, IsNil)
@@ -325,11 +329,17 @@ func (s *MySuite) TestMultipleObjects(c *C) {
 	typedDriver.On("CreateBucket", "bucket", "private").Return(nil).Once()
 	driver.CreateBucket("bucket", "private")
 	typedDriver.On("CreateObject", "bucket", "object1", "", "", mock.Anything).Return(nil).Once()
+	typedDriver.On("GetObjectMetadata", "bucket", "object1", "").Return(metadata1, nil).Once()
 	driver.CreateObject("bucket", "object1", "", "", buffer1)
+	driver.GetObjectMetadata("bucket", "object1", "")
 	typedDriver.On("CreateObject", "bucket", "object2", "", "", mock.Anything).Return(nil).Once()
+	typedDriver.On("GetObjectMetadata", "bucket", "object2", "").Return(metadata2, nil).Once()
 	driver.CreateObject("bucket", "object2", "", "", buffer2)
+	driver.GetObjectMetadata("bucket", "object2", "")
 	typedDriver.On("CreateObject", "bucket", "object3", "", "", mock.Anything).Return(nil).Once()
+	typedDriver.On("GetObjectMetadata", "bucket", "object3", "").Return(metadata3, nil).Once()
 	driver.CreateObject("bucket", "object3", "", "", buffer3)
+	driver.GetObjectMetadata("bucket", "object3", "")
 
 	// test non-existant object
 	typedDriver.On("GetBucketMetadata", "bucket").Return(drivers.BucketMetadata{}, nil).Once()
@@ -494,11 +504,6 @@ func (s *MySuite) TestHeader(c *C) {
 
 	verifyError(c, response, "NoSuchKey", "The specified key does not exist.", http.StatusNotFound)
 
-	buffer := bytes.NewBufferString("hello world")
-	typedDriver.On("GetBucketMetadata", "foo").Return(bucketMetadata, nil).Once()
-	typedDriver.On("CreateObject", "bucket", "object", "", "", mock.Anything).Return(nil).Once()
-	driver.CreateObject("bucket", "object", "", "", buffer)
-
 	objectMetadata := drivers.ObjectMetadata{
 		Bucket:      "bucket",
 		Key:         "object",
@@ -508,6 +513,13 @@ func (s *MySuite) TestHeader(c *C) {
 		Size:        11,
 	}
 
+	buffer := bytes.NewBufferString("hello world")
+	typedDriver.On("GetBucketMetadata", "foo").Return(bucketMetadata, nil).Once()
+	typedDriver.On("CreateObject", "bucket", "object", "", "", mock.Anything).Return(nil).Once()
+	typedDriver.On("GetObjectMetadata", "bucket", "object", "").Return(objectMetadata, nil).Once()
+	driver.CreateObject("bucket", "object", "", "", buffer)
+	driver.GetObjectMetadata("bucket", "object", "")
+
 	typedDriver.On("GetBucketMetadata", "bucket").Return(bucketMetadata, nil).Once()
 	typedDriver.On("GetObjectMetadata", "bucket", "object", "").Return(objectMetadata, nil).Once()
 	typedDriver.SetGetObjectWriter("", "", []byte("hello world"))
@@ -608,15 +620,6 @@ func (s *MySuite) TestPutObject(c *C) {
 	c.Assert(err, IsNil)
 	c.Assert(response.StatusCode, Equals, http.StatusOK)
 
-	typedDriver.On("CreateObject", "bucket", "two", "", "", mock.Anything).Return(nil).Once()
-	request, err = http.NewRequest("PUT", testServer.URL+"/bucket/two", bytes.NewBufferString("hello world"))
-	c.Assert(err, IsNil)
-	setAuthHeader(request)
-
-	response, err = client.Do(request)
-	c.Assert(err, IsNil)
-	c.Assert(response.StatusCode, Equals, http.StatusOK)
-
 	twoMetadata := drivers.ObjectMetadata{
 		Bucket:      "bucket",
 		Key:         "two",
@@ -626,6 +629,16 @@ func (s *MySuite) TestPutObject(c *C) {
 		Size:        11,
 	}
 
+	typedDriver.On("CreateObject", "bucket", "two", "", "", mock.Anything).Return(nil).Once()
+	typedDriver.On("GetObjectMetadata", "bucket", "two", "").Return(twoMetadata, nil).Once()
+	request, err = http.NewRequest("PUT", testServer.URL+"/bucket/two", bytes.NewBufferString("hello world"))
+	c.Assert(err, IsNil)
+	setAuthHeader(request)
+
+	response, err = client.Do(request)
+	c.Assert(err, IsNil)
+	c.Assert(response.StatusCode, Equals, http.StatusOK)
+
 	date2 := time.Now()
 
 	resources.Maxkeys = 1000
@@ -731,8 +744,8 @@ func (s *MySuite) TestListBuckets(c *C) {
 	c.Assert(listResponse.Buckets.Bucket[1].Name, Equals, "foo")
 }
 
-func readListBucket(reader io.Reader) (BucketListResponse, error) {
-	var results BucketListResponse
+func readListBucket(reader io.Reader) (ListBucketsResponse, error) {
+	var results ListBucketsResponse
 	decoder := xml.NewDecoder(reader)
 	err := decoder.Decode(&results)
 	return results, err
@@ -893,8 +906,19 @@ func (s *MySuite) TestContentTypePersists(c *C) {
 		Created: time.Now(),
 		ACL:     drivers.BucketACL("private"),
 	}
+	// test head
+	oneMetadata := drivers.ObjectMetadata{
+		Bucket:      "bucket",
+		Key:         "one",
+		ContentType: "application/octet-stream",
+		Created:     time.Now(),
+		Md5:         "d41d8cd98f00b204e9800998ecf8427e",
+		Size:        0,
+	}
+
 	typedDriver.On("GetBucketMetadata", "bucket").Return(metadata, nil).Once()
 	typedDriver.On("CreateObject", "bucket", "one", "", "", mock.Anything).Return(nil).Once()
+	typedDriver.On("GetObjectMetadata", "bucket", "one", "").Return(oneMetadata, nil).Once()
 	request, err := http.NewRequest("PUT", testServer.URL+"/bucket/one", bytes.NewBufferString("hello world"))
 	delete(request.Header, "Content-Type")
 	c.Assert(err, IsNil)
@@ -905,15 +929,6 @@ func (s *MySuite) TestContentTypePersists(c *C) {
 	c.Assert(err, IsNil)
 	c.Assert(response.StatusCode, Equals, http.StatusOK)
 
-	// test head
-	oneMetadata := drivers.ObjectMetadata{
-		Bucket:      "bucket",
-		Key:         "one",
-		ContentType: "application/octet-stream",
-		Created:     time.Now(),
-		Md5:         "d41d8cd98f00b204e9800998ecf8427e",
-		Size:        0,
-	}
 	typedDriver.On("GetBucketMetadata", "bucket").Return(drivers.BucketMetadata{}, nil).Once()
 	typedDriver.On("GetObjectMetadata", "bucket", "one", "").Return(oneMetadata, nil).Once()
 	request, err = http.NewRequest("HEAD", testServer.URL+"/bucket/one", nil)
@@ -939,8 +954,19 @@ func (s *MySuite) TestContentTypePersists(c *C) {
 	c.Assert(response.StatusCode, Equals, http.StatusOK)
 	c.Assert(response.Header.Get("Content-Type"), Equals, "application/octet-stream")
 
+	twoMetadata := drivers.ObjectMetadata{
+		Bucket:      "bucket",
+		Key:         "one",
+		ContentType: "application/octet-stream",
+		Created:     time.Now(),
+		// Fix MD5
+		Md5:  "d41d8cd98f00b204e9800998ecf8427e",
+		Size: 0,
+	}
+
 	typedDriver.On("GetBucketMetadata", "bucket").Return(metadata, nil).Once()
 	typedDriver.On("CreateObject", "bucket", "two", "", "", mock.Anything).Return(nil).Once()
+	typedDriver.On("GetObjectMetadata", "bucket", "two", "").Return(twoMetadata, nil).Once()
 	request, err = http.NewRequest("PUT", testServer.URL+"/bucket/two", bytes.NewBufferString("hello world"))
 	delete(request.Header, "Content-Type")
 	request.Header.Add("Content-Type", "application/json")
@@ -951,15 +977,6 @@ func (s *MySuite) TestContentTypePersists(c *C) {
 	c.Assert(err, IsNil)
 	c.Assert(response.StatusCode, Equals, http.StatusOK)
 
-	twoMetadata := drivers.ObjectMetadata{
-		Bucket:      "bucket",
-		Key:         "one",
-		ContentType: "application/octet-stream",
-		Created:     time.Now(),
-		// Fix MD5
-		Md5:  "d41d8cd98f00b204e9800998ecf8427e",
-		Size: 0,
-	}
 	typedDriver.On("GetBucketMetadata", "bucket").Return(metadata, nil).Once()
 	typedDriver.On("GetObjectMetadata", "bucket", "two", "").Return(twoMetadata, nil).Once()
 	request, err = http.NewRequest("HEAD", testServer.URL+"/bucket/two", nil)
@@ -1008,10 +1025,12 @@ func (s *MySuite) TestPartialContent(c *C) {
 
 	typedDriver.On("CreateBucket", "foo", "private").Return(nil).Once()
 	typedDriver.On("CreateObject", "foo", "bar", "", "", mock.Anything).Return(nil).Once()
+	typedDriver.On("GetObjectMetadata", "foo", "bar", "").Return(metadata, nil).Once()
 	err := driver.CreateBucket("foo", "private")
 	c.Assert(err, IsNil)
 
 	driver.CreateObject("foo", "bar", "", "", bytes.NewBufferString("hello world"))
+	driver.GetObjectMetadata("foo", "bar", "")
 
 	// prepare for GET on range request
 	typedDriver.SetGetObjectWriter("foo", "bar", []byte("hello world"))

pkg/api/headers.go

@@ -40,6 +40,8 @@ func setCommonHeaders(w http.ResponseWriter, acceptsType string) {
 	w.Header().Set("Accept-Ranges", "bytes")
 	w.Header().Set("Content-Type", acceptsType)
 	w.Header().Set("Connection", "close")
+	// should be set to '0' by default
+	w.Header().Set("Content-Length", "0")
 }
 
 // Write error response headers