From e4bd882f11239700c3a62b6f7cfc71aa51b0f466 Mon Sep 17 00:00:00 2001 From: Anis Elleuch Date: Wed, 12 Apr 2017 21:34:57 +0200 Subject: [PATCH] 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. --- cmd/object-handlers-common.go | 54 ++++++++++---------- cmd/object-handlers_test.go | 92 +++++++++++++++++++++++++++++++---- cmd/server_test.go | 13 +++++ 3 files changed, 125 insertions(+), 34 deletions(-) diff --git a/cmd/object-handlers-common.go b/cmd/object-handlers-common.go index dee33a0d5..1ed15b80d 100644 --- 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)) { diff --git a/cmd/object-handlers_test.go b/cmd/object-handlers_test.go index eed7d2cb1..f6b10539d 100644 --- 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 { diff --git a/cmd/server_test.go b/cmd/server_test.go index 6a8e0a843..53e075858 100644 --- 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.