From d0be09fdd3b893be554f787c23d1cebe1ea7e50b Mon Sep 17 00:00:00 2001
From: Krishnan Parthasarathi <krisis@users.noreply.github.com>
Date: Sun, 26 Jun 2016 18:10:08 -0700
Subject: [PATCH] object: checkETag compares quoted ETags properly (#1997)

Previously, checkETag didn't handle ETags with leading and trailing
double quotes. e.g "abcdef1234" == "\"abcdef1234\"" would return false.
Now, checkETag function canonicalizes the ETags passed as arguments by
removing one leading/trailing double quote.
---
 object-handlers.go | 58 ++++++++++++++++++++++++++++------------------
 1 file changed, 35 insertions(+), 23 deletions(-)

diff --git a/object-handlers.go b/object-handlers.go
index 57287d563..bd4037248 100644
--- a/object-handlers.go
+++ b/object-handlers.go
@@ -107,16 +107,6 @@ func (api objectAPIHandlers) GetObjectHandler(w http.ResponseWriter, r *http.Req
 		return
 	}
 
-	// Verify 'If-Modified-Since' and 'If-Unmodified-Since'.
-	lastModified := objInfo.ModTime
-	if checkLastModified(w, r, lastModified) {
-		return
-	}
-	// Verify 'If-Match' and 'If-None-Match'.
-	if checkETag(w, r) {
-		return
-	}
-
 	var hrange *httpRange
 	hrange, err = getRequestedRange(r.Header.Get("Range"), objInfo.Size)
 	if err != nil {
@@ -130,6 +120,16 @@ func (api objectAPIHandlers) GetObjectHandler(w http.ResponseWriter, r *http.Req
 	// Set any additional requested response headers.
 	setGetRespHeaders(w, r.URL.Query())
 
+	// Verify 'If-Modified-Since' and 'If-Unmodified-Since'.
+	lastModified := objInfo.ModTime
+	if checkLastModified(w, r, lastModified) {
+		return
+	}
+	// Verify 'If-Match' and 'If-None-Match'.
+	if checkETag(w, r) {
+		return
+	}
+
 	// Get the object.
 	startOffset := hrange.start
 	length := hrange.length
@@ -191,25 +191,40 @@ func checkLastModified(w http.ResponseWriter, r *http.Request, modtime time.Time
 	return false
 }
 
+// canonicalizeETag returns ETag with leading and trailing double-quotes removed,
+// if any present
+func canonicalizeETag(etag string) string {
+	canonicalETag := strings.TrimPrefix(etag, "\"")
+	return strings.TrimSuffix(canonicalETag, "\"")
+}
+
+// isETagEqual return true if the canonical representations of two ETag strings
+// are equal, false otherwise
+func isETagEqual(left, right string) bool {
+	return canonicalizeETag(left) == canonicalizeETag(right)
+}
+
 // checkETag implements If-None-Match and If-Match checks.
 //
 // The ETag must have been previously set in the ResponseWriter's
 // headers. The return value is whether this request is now considered
 // done.
 func checkETag(w http.ResponseWriter, r *http.Request) bool {
+	// writer always has quoted string
+	// transform reader's etag to
+	if r.Method != "GET" && r.Method != "HEAD" {
+		return false
+	}
 	etag := w.Header().Get("ETag")
 	// Must know ETag.
 	if etag == "" {
 		return false
 	}
-	if inm := r.Header.Get("If-None-Match"); inm != "" {
+	if inm := r.Header.Get("If-None-Match"); !isETagEqual(inm, "") {
 		// Return the object only if its entity tag (ETag) is
 		// different from the one specified; otherwise, return a 304
 		// (not modified).
-		if r.Method != "GET" && r.Method != "HEAD" {
-			return false
-		}
-		if inm == etag || inm == "*" {
+		if isETagEqual(inm, etag) || isETagEqual(inm, "*") {
 			h := w.Header()
 			// Remove following headers if already set.
 			delete(h, "Content-Type")
@@ -218,13 +233,10 @@ func checkETag(w http.ResponseWriter, r *http.Request) bool {
 			w.WriteHeader(http.StatusNotModified)
 			return true
 		}
-	} else if im := r.Header.Get("If-Match"); im != "" {
+	} else if im := r.Header.Get("If-Match"); !isETagEqual(im, "") {
 		// Return the object only if its entity tag (ETag) is the same
 		// as the one specified; otherwise, return a 412 (precondition failed).
-		if r.Method != "GET" && r.Method != "HEAD" {
-			return false
-		}
-		if im != etag {
+		if !isETagEqual(im, etag) {
 			h := w.Header()
 			// Remove following headers if already set.
 			delete(h, "Content-Type")
@@ -275,6 +287,9 @@ func (api objectAPIHandlers) HeadObjectHandler(w http.ResponseWriter, r *http.Re
 		return
 	}
 
+	// Set standard object headers.
+	setObjectHeaders(w, objInfo, nil)
+
 	// Verify 'If-Modified-Since' and 'If-Unmodified-Since'.
 	lastModified := objInfo.ModTime
 	if checkLastModified(w, r, lastModified) {
@@ -286,9 +301,6 @@ func (api objectAPIHandlers) HeadObjectHandler(w http.ResponseWriter, r *http.Re
 		return
 	}
 
-	// Set standard object headers.
-	setObjectHeaders(w, objInfo, nil)
-
 	// Successfull response.
 	w.WriteHeader(http.StatusOK)
 }