handlers: Ignore malformatted datetime type header (#4097)

Ignore headers, such as If-Modified-Since, If-Unmodified-Since, etc.. when they are received with a format other than HTTP date.
7 years ago · e4bd882f11
parent 4448285a83
commit e4bd882f11
3 changed files with 125 additions and 34 deletions
--- a/cmd/object-handlers-common.go
+++ b/cmd/object-handlers-common.go
@ -66,11 +66,13 @@ func checkCopyObjectPreconditions(w http.ResponseWriter, r *http.Request, objInf
 	// 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, ErrPreconditionFailed, r.URL)
-			return true
+		if givenTime, err := time.Parse(http.TimeFormat, ifModifiedSinceHeader); err == nil {
+			if !ifModifiedSince(objInfo.ModTime, givenTime) {
+				// If the object is not modified since the specified time.
+				writeHeaders()
+				writeErrorResponse(w, ErrPreconditionFailed, r.URL)
+				return true
+			}
 		}
 	}

@ -78,11 +80,13 @@ func checkCopyObjectPreconditions(w http.ResponseWriter, r *http.Request, objInf
 	// 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, ErrPreconditionFailed, r.URL)
-			return true
+		if givenTime, err := time.Parse(http.TimeFormat, ifUnmodifiedSinceHeader); err == nil {
+			if ifModifiedSince(objInfo.ModTime, givenTime) {
+				// If the object is modified since the specified time.
+				writeHeaders()
+				writeErrorResponse(w, ErrPreconditionFailed, r.URL)
+				return true
+			}
 		}
 	}

@ -147,11 +151,13 @@ func checkPreconditions(w http.ResponseWriter, r *http.Request, objInfo ObjectIn
 	// otherwise return a 304 (not modified).
 	ifModifiedSinceHeader := r.Header.Get("If-Modified-Since")
 	if ifModifiedSinceHeader != "" {
-		if !ifModifiedSince(objInfo.ModTime, ifModifiedSinceHeader) {
-			// If the object is not modified since the specified time.
-			writeHeaders()
-			w.WriteHeader(http.StatusNotModified)
-			return true
+		if givenTime, err := time.Parse(http.TimeFormat, ifModifiedSinceHeader); err == nil {
+			if !ifModifiedSince(objInfo.ModTime, givenTime) {
+				// If the object is not modified since the specified time.
+				writeHeaders()
+				w.WriteHeader(http.StatusNotModified)
+				return true
+			}
 		}
 	}

@ -159,11 +165,13 @@ func checkPreconditions(w http.ResponseWriter, r *http.Request, objInfo ObjectIn
 	// time, otherwise return a 412 (precondition failed).
 	ifUnmodifiedSinceHeader := r.Header.Get("If-Unmodified-Since")
 	if ifUnmodifiedSinceHeader != "" {
-		if ifModifiedSince(objInfo.ModTime, ifUnmodifiedSinceHeader) {
-			// If the object is modified since the specified time.
-			writeHeaders()
-			writeErrorResponse(w, ErrPreconditionFailed, r.URL)
-			return true
+		if givenTime, err := time.Parse(http.TimeFormat, ifUnmodifiedSinceHeader); err == nil {
+			if ifModifiedSince(objInfo.ModTime, givenTime) {
+				// If the object is modified since the specified time.
+				writeHeaders()
+				writeErrorResponse(w, ErrPreconditionFailed, r.URL)
+				return true
+			}
 		}
 	}

@ -195,11 +203,7 @@ func checkPreconditions(w http.ResponseWriter, r *http.Request, objInfo ObjectIn
 }

 // returns true if object was modified after givenTime.
-func ifModifiedSince(objTime time.Time, givenTimeStr string) bool {
-	givenTime, err := time.Parse(http.TimeFormat, givenTimeStr)
-	if err != nil {
-		return true
-	}
+func ifModifiedSince(objTime time.Time, givenTime time.Time) bool {
 	// The Date-Modified header truncates sub-second precision, so
 	// use mtime < t+1s instead of mtime <= t to check for unmodified.
 	if objTime.After(givenTime.Add(1 * time.Second)) {
--- a/cmd/object-handlers_test.go
+++ b/cmd/object-handlers_test.go
@ -1473,15 +1473,17 @@ func testAPICopyObjectHandler(obj ObjectLayer, instanceType, bucketName string,

 	// test cases with inputs and expected result for Copy Object.
 	testCases := []struct {
-		bucketName       string
-		newObjectName    string // name of the newly copied object.
-		copySourceHeader string // data for "X-Amz-Copy-Source" header. Contains the object to be copied in the URL.
-		metadataGarbage  bool
-		metadataReplace  bool
-		metadataCopy     bool
-		metadata         map[string]string
-		accessKey        string
-		secretKey        string
+		bucketName           string
+		newObjectName        string // name of the newly copied object.
+		copySourceHeader     string // data for "X-Amz-Copy-Source" header. Contains the object to be copied in the URL.
+		copyModifiedHeader   string // data for "X-Amz-Copy-Source-If-Modified-Since" header
+		copyUnmodifiedHeader string // data for "X-Amz-Copy-Source-If-Unmodified-Since" header
+		metadataGarbage      bool
+		metadataReplace      bool
+		metadataCopy         bool
+		metadata             map[string]string
+		accessKey            string
+		secretKey            string
 		// expected output.
 		expectedRespStatus int
 	}{
@ -1624,6 +1626,66 @@ func testAPICopyObjectHandler(obj ObjectLayer, instanceType, bucketName string,

 			expectedRespStatus: http.StatusForbidden,
 		},
+		// Test case - 11, copy metadata from newObject1 with satisfying modified header.
+		{
+			bucketName:         bucketName,
+			newObjectName:      "newObject1",
+			copySourceHeader:   url.QueryEscape("/" + bucketName + "/" + objectName),
+			copyModifiedHeader: "Mon, 02 Jan 2006 15:04:05 GMT",
+			accessKey:          credentials.AccessKey,
+			secretKey:          credentials.SecretKey,
+			expectedRespStatus: http.StatusOK,
+		},
+		// Test case - 12, copy metadata from newObject1 with unsatisfying modified header.
+		{
+			bucketName:         bucketName,
+			newObjectName:      "newObject1",
+			copySourceHeader:   url.QueryEscape("/" + bucketName + "/" + objectName),
+			copyModifiedHeader: "Mon, 02 Jan 2217 15:04:05 GMT",
+			accessKey:          credentials.AccessKey,
+			secretKey:          credentials.SecretKey,
+			expectedRespStatus: http.StatusPreconditionFailed,
+		},
+		// Test case - 13, copy metadata from newObject1 with wrong modified header format
+		{
+			bucketName:         bucketName,
+			newObjectName:      "newObject1",
+			copySourceHeader:   url.QueryEscape("/" + bucketName + "/" + objectName),
+			copyModifiedHeader: "Mon, 02 Jan 2217 15:04:05 +00:00",
+			accessKey:          credentials.AccessKey,
+			secretKey:          credentials.SecretKey,
+			expectedRespStatus: http.StatusOK,
+		},
+		// Test case - 14, copy metadata from newObject1 with satisfying unmodified header.
+		{
+			bucketName:           bucketName,
+			newObjectName:        "newObject1",
+			copySourceHeader:     url.QueryEscape("/" + bucketName + "/" + objectName),
+			copyUnmodifiedHeader: "Mon, 02 Jan 2217 15:04:05 GMT",
+			accessKey:            credentials.AccessKey,
+			secretKey:            credentials.SecretKey,
+			expectedRespStatus:   http.StatusOK,
+		},
+		// Test case - 15, copy metadata from newObject1 with unsatisfying unmodified header.
+		{
+			bucketName:           bucketName,
+			newObjectName:        "newObject1",
+			copySourceHeader:     url.QueryEscape("/" + bucketName + "/" + objectName),
+			copyUnmodifiedHeader: "Mon, 02 Jan 2007 15:04:05 GMT",
+			accessKey:            credentials.AccessKey,
+			secretKey:            credentials.SecretKey,
+			expectedRespStatus:   http.StatusPreconditionFailed,
+		},
+		// Test case - 16, copy metadata from newObject1 with incorrect unmodified header format.
+		{
+			bucketName:           bucketName,
+			newObjectName:        "newObject1",
+			copySourceHeader:     url.QueryEscape("/" + bucketName + "/" + objectName),
+			copyUnmodifiedHeader: "Mon, 02 Jan 2007 15:04:05 +00:00",
+			accessKey:            credentials.AccessKey,
+			secretKey:            credentials.SecretKey,
+			expectedRespStatus:   http.StatusOK,
+		},
 	}

 	for i, testCase := range testCases {
@ -1642,6 +1704,12 @@ func testAPICopyObjectHandler(obj ObjectLayer, instanceType, bucketName string,
 		if testCase.copySourceHeader != "" {
 			req.Header.Set("X-Amz-Copy-Source", testCase.copySourceHeader)
 		}
+		if testCase.copyModifiedHeader != "" {
+			req.Header.Set("X-Amz-Copy-Source-If-Modified-Since", testCase.copyModifiedHeader)
+		}
+		if testCase.copyUnmodifiedHeader != "" {
+			req.Header.Set("X-Amz-Copy-Source-If-Unmodified-Since", testCase.copyUnmodifiedHeader)
+		}
 		// Add custom metadata.
 		for k, v := range testCase.metadata {
 			req.Header.Set(k, v)
@ -1687,6 +1755,12 @@ func testAPICopyObjectHandler(obj ObjectLayer, instanceType, bucketName string,
 		if testCase.copySourceHeader != "" {
 			reqV2.Header.Set("X-Amz-Copy-Source", testCase.copySourceHeader)
 		}
+		if testCase.copyModifiedHeader != "" {
+			reqV2.Header.Set("X-Amz-Copy-Source-If-Modified-Since", testCase.copyModifiedHeader)
+		}
+		if testCase.copyUnmodifiedHeader != "" {
+			reqV2.Header.Set("X-Amz-Copy-Source-If-Unmodified-Since", testCase.copyUnmodifiedHeader)
+		}

 		// Add custom metadata.
 		for k, v := range testCase.metadata {
--- a/cmd/server_test.go
+++ b/cmd/server_test.go
@ -1342,6 +1342,19 @@ func (s *TestSuiteCommon) TestHeadOnObjectLastModified(c *C) {
 	response, err = client.Do(request)
 	c.Assert(err, IsNil)
 	c.Assert(response.StatusCode, Equals, http.StatusPreconditionFailed)
+
+	// make HTTP request to obtain object info.
+	// But this time set a date with unrecognized format to the "If-Modified-Since" header
+	request, err = newTestSignedRequest("HEAD", getHeadObjectURL(s.endPoint, bucketName, objectName),
+		0, nil, s.accessKey, s.secretKey, s.signer)
+	c.Assert(err, IsNil)
+	request.Header.Set("If-Unmodified-Since", "Mon, 02 Jan 2006 15:04:05 +00:00")
+	response, err = client.Do(request)
+	c.Assert(err, IsNil)
+	// Since the "If-Modified-Since" header was ahead in time compared to the actual
+	// modified time of the object expecting the response status to be http.StatusNotModified.
+	c.Assert(response.StatusCode, Equals, http.StatusOK)
+
 }

 // TestHeadOnBucket - Validates response for HEAD on the bucket.