API/CopyObject: Refactor the code and handle if-modified-since as well. (#2183)

This completes the S3 spec behavior for CopyObject API as reported
by `s3verify`.

object-handlers-common.go

@@ -22,12 +22,104 @@ import (
 	"time"
 )
 
-// Validates the preconditions. Returns true if object content need not be written to http.ResponseWriter.
+// Validates the preconditions for CopyObject, returns true if CopyObject operation should not proceed.
-func checkPreconditions(r *http.Request, w http.ResponseWriter, objInfo ObjectInfo) bool {
+// Preconditions supported are:
+//  x-amz-copy-source-if-modified-since
+//  x-amz-copy-source-if-unmodified-since
+//  x-amz-copy-source-if-match
+//  x-amz-copy-source-if-none-match
+func checkCopyObjectPreconditions(w http.ResponseWriter, r *http.Request, objInfo ObjectInfo) bool {
+	// Return false for methods other than GET and HEAD.
+	if r.Method != "PUT" {
+		return false
+	}
+	// If the object doesn't have a modtime (IsZero), or the modtime
+	// is obviously garbage (Unix time == 0), then ignore modtimes
+	// and don't process the If-Modified-Since header.
+	if objInfo.ModTime.IsZero() || objInfo.ModTime.Equal(time.Unix(0, 0)) {
+		return false
+	}
+
+	// Headers to be set of object content is not going to be written to the client.
+	writeHeaders := func() {
+		// set common headers
+		setCommonHeaders(w)
+
+		// set object-related metadata headers
+		w.Header().Set("Last-Modified", objInfo.ModTime.UTC().Format(http.TimeFormat))
+
+		if objInfo.MD5Sum != "" {
+			w.Header().Set("ETag", "\""+objInfo.MD5Sum+"\"")
+		}
+	}
+	// x-amz-copy-source-if-modified-since: Return the object only if it has been modified
+	// since the specified time otherwise return 412 (precondition failed).
+	ifModifiedSinceHeader := r.Header.Get("x-amz-copy-source-if-modified-since")
+	if ifModifiedSinceHeader != "" {
+		if !ifModifiedSince(objInfo.ModTime, ifModifiedSinceHeader) {
+			// If the object is not modified since the specified time.
+			writeHeaders()
+			writeErrorResponse(w, r, ErrPreconditionFailed, r.URL.Path)
+			return true
+		}
+	}
+
+	// x-amz-copy-source-if-unmodified-since : Return the object only if it has not been
+	// modified since the specified time, otherwise return a 412 (precondition failed).
+	ifUnmodifiedSinceHeader := r.Header.Get("x-amz-copy-source-if-unmodified-since")
+	if ifUnmodifiedSinceHeader != "" {
+		if ifModifiedSince(objInfo.ModTime, ifUnmodifiedSinceHeader) {
+			// If the object is modified since the specified time.
+			writeHeaders()
+			writeErrorResponse(w, r, ErrPreconditionFailed, r.URL.Path)
+			return true
+		}
+	}
+
+	// x-amz-copy-source-if-match : Return the object only if its entity tag (ETag) is the
+	// same as the one specified; otherwise return a 412 (precondition failed).
+	ifMatchETagHeader := r.Header.Get("x-amz-copy-source-if-match")
+	if ifMatchETagHeader != "" {
+		if !isETagEqual(objInfo.MD5Sum, ifMatchETagHeader) {
+			// If the object ETag does not match with the specified ETag.
+			writeHeaders()
+			writeErrorResponse(w, r, ErrPreconditionFailed, r.URL.Path)
+			return true
+		}
+	}
+
+	// If-None-Match : Return the object only if its entity tag (ETag) is different from the
+	// one specified otherwise, return a 304 (not modified).
+	ifNoneMatchETagHeader := r.Header.Get("x-amz-copy-source-if-none-match")
+	if ifNoneMatchETagHeader != "" {
+		if isETagEqual(objInfo.MD5Sum, ifNoneMatchETagHeader) {
+			// If the object ETag matches with the specified ETag.
+			writeHeaders()
+			writeErrorResponse(w, r, ErrPreconditionFailed, r.URL.Path)
+			return true
+		}
+	}
+	// Object content should be written to http.ResponseWriter
+	return false
+}
+
+// Validates the preconditions. Returns true if GET/HEAD operation should not proceed.
+// Preconditions supported are:
+//  If-Modified-Since
+//  If-Unmodified-Since
+//  If-Match
+//  If-None-Match
+func checkPreconditions(w http.ResponseWriter, r *http.Request, objInfo ObjectInfo) bool {
 	// Return false for methods other than GET and HEAD.
 	if r.Method != "GET" && r.Method != "HEAD" {
 		return false
 	}
+	// If the object doesn't have a modtime (IsZero), or the modtime
+	// is obviously garbage (Unix time == 0), then ignore modtimes
+	// and don't process the If-Modified-Since header.
+	if objInfo.ModTime.IsZero() || objInfo.ModTime.Equal(time.Unix(0, 0)) {
+		return false
+	}
 
 	// Headers to be set of object content is not going to be written to the client.
 	writeHeaders := func() {
@@ -94,12 +186,6 @@ func checkPreconditions(r *http.Request, w http.ResponseWriter, objInfo ObjectIn
 
 // returns true if object was modified after givenTime.
 func ifModifiedSince(objTime time.Time, givenTimeStr string) bool {
-	if objTime.IsZero() || objTime.Equal(time.Unix(0, 0)) {
-		// If the object doesn't have a modtime (IsZero), or the modtime
-		// is obviously garbage (Unix time == 0), then ignore modtimes
-		// and don't process the If-Modified-Since header.
-		return false
-	}
 	givenTime, err := time.Parse(http.TimeFormat, givenTimeStr)
 	if err != nil {
 		return true
@@ -124,100 +210,3 @@ func canonicalizeETag(etag string) string {
 func isETagEqual(left, right string) bool {
 	return canonicalizeETag(left) == canonicalizeETag(right)
 }
-
-// checkCopySource implements x-amz-copy-source-if-modified-since and
-// x-amz-copy-source-if-unmodified-since checks.
-//
-// modtime is the modification time of the resource to be served, or
-// IsZero(). return value is whether this request is now complete.
-func checkCopySourceLastModified(w http.ResponseWriter, r *http.Request, modtime time.Time) bool {
-	// writer always has quoted string
-	// transform reader's etag to
-	if r.Method != "PUT" {
-		return false
-	}
-	if modtime.IsZero() || modtime.Equal(time.Unix(0, 0)) {
-		// If the object doesn't have a modtime (IsZero), or the modtime
-		// is obviously garbage (Unix time == 0), then ignore modtimes
-		// and don't process the If-Modified-Since header.
-		return false
-	}
-	// The Date-Modified header truncates sub-second precision, so
-	// use mtime < t+1s instead of mtime <= t to check for unmodified.
-	if _, ok := r.Header["x-amz-copy-source-if-modified-since"]; ok {
-		// Return the object only if it has been modified since the
-		// specified time, otherwise return a 304 error (not modified).
-		t, err := time.Parse(http.TimeFormat, r.Header.Get("x-amz-copy-source-if-modified-since"))
-		if err == nil && modtime.Before(t.Add(1*time.Second)) {
-			h := w.Header()
-			// Remove Content headers if set
-			delete(h, "Content-Type")
-			delete(h, "Content-Length")
-			delete(h, "Content-Range")
-			w.WriteHeader(http.StatusNotModified)
-			return true
-		}
-	} else if _, ok := r.Header["x-amz-copy-source-if-unmodified-since"]; ok {
-		// Return the object only if it has not been modified since the
-		// specified time, otherwise return a 412 error (precondition failed).
-		t, err := time.Parse(http.TimeFormat, r.Header.Get("x-amz-copy-source-if-unmodified-since"))
-		if err == nil && modtime.After(t.Add(1*time.Second)) {
-			h := w.Header()
-			// Remove Content headers if set
-			delete(h, "Content-Type")
-			delete(h, "Content-Length")
-			delete(h, "Content-Range")
-			writeErrorResponse(w, r, ErrPreconditionFailed, r.URL.Path)
-			return true
-		}
-	}
-	w.Header().Set("Last-Modified", modtime.UTC().Format(http.TimeFormat))
-	return false
-}
-
-// checkCopySourceETag implements x-amz-copy-source-if-match and
-// x-amz-copy-source-if-none-match checks.
-//
-// The ETag must have been previously set in the ResponseWriter's
-// headers. The return value is whether this request is now considered
-// complete.
-func checkCopySourceETag(w http.ResponseWriter, r *http.Request) bool {
-	// writer always has quoted string
-	// transform reader's etag to
-	if r.Method != "PUT" {
-		return false
-	}
-	etag := w.Header().Get("ETag")
-	// Tag must be provided...
-	if etag == "" {
-		return false
-	}
-	if inm := r.Header.Get("x-amz-copy-source-if-none-match"); inm != "" {
-		// Return the object only if its entity tag (ETag) is
-		// different from the one specified; otherwise, the
-		// request returns a 412 HTTP status code error (failed precondition).
-		if isETagEqual(inm, etag) || isETagEqual(inm, "*") {
-			h := w.Header()
-			// Remove Content headers if set
-			delete(h, "Content-Type")
-			delete(h, "Content-Length")
-			delete(h, "Content-Range")
-			writeErrorResponse(w, r, ErrPreconditionFailed, r.URL.Path)
-			return true
-		}
-	} else if inm := r.Header.Get("x-amz-copy-source-if-match"); !isETagEqual(inm, "") {
-		// Return the object only if its entity tag (ETag) is the same
-		// as the one specified; otherwise, return a 412 (precondition failed).
-		if !isETagEqual(inm, etag) {
-			h := w.Header()
-			// Remove Content headers if set
-			delete(h, "Content-Type")
-			delete(h, "Content-Length")
-			delete(h, "Content-Range")
-			writeErrorResponse(w, r, ErrPreconditionFailed, r.URL.Path)
-			return true
-		}
-	}
-	return false
-
-}

object-handlers.go

@@ -130,7 +130,7 @@ func (api objectAPIHandlers) GetObjectHandler(w http.ResponseWriter, r *http.Req
 	}
 
 	// Validate pre-conditions if any.
-	if checkPreconditions(r, w, objInfo) {
+	if checkPreconditions(w, r, objInfo) {
 		return
 	}
 
@@ -217,7 +217,7 @@ func (api objectAPIHandlers) HeadObjectHandler(w http.ResponseWriter, r *http.Re
 	}
 
 	// Validate pre-conditions if any.
-	if checkPreconditions(r, w, objInfo) {
+	if checkPreconditions(w, r, objInfo) {
 		return
 	}
 
@@ -295,21 +295,8 @@ func (api objectAPIHandlers) CopyObjectHandler(w http.ResponseWriter, r *http.Re
 		return
 	}
 
-	// Verify before writing.
+	// Verify before x-amz-copy-source preconditions before continuing with CopyObject.
-
+	if checkCopyObjectPreconditions(w, r, objInfo) {
-	// Verify x-amz-copy-source-if-modified-since and
-	// x-amz-copy-source-if-unmodified-since.
-	lastModified := objInfo.ModTime
-	if checkCopySourceLastModified(w, r, lastModified) {
-		return
-	}
-
-	if objInfo.MD5Sum != "" {
-		w.Header().Set("ETag", "\""+objInfo.MD5Sum+"\"")
-	}
-
-	// Verify x-amz-copy-source-if-match and x-amz-copy-source-if-none-match.
-	if checkCopySourceETag(w, r) {
 		return
 	}