From ee202b76f3b9a6765399dd8c2157b1ac089b3af0 Mon Sep 17 00:00:00 2001 From: Ben McClelland Date: Mon, 26 Aug 2024 21:08:22 -0700 Subject: [PATCH] fix: move RFC 3339 time formatting to s3response It is better if we let the s3response module handle the xml formatting spec specifics, and let the backends not worry about how to format the time fields. This should help to prevent any future backend modifications or additions from accidental incorrect time formatting. --- backend/azure/azure.go | 8 +++--- backend/common.go | 5 ---- backend/posix/posix.go | 12 ++++----- backend/s3proxy/s3.go | 7 +++-- backend/scoutfs/scoutfs.go | 6 +++-- backend/walk_test.go | 6 +++-- s3response/s3response.go | 53 +++++++++++++++++++++++++++++++++++--- 7 files changed, 71 insertions(+), 26 deletions(-) diff --git a/backend/azure/azure.go b/backend/azure/azure.go index 625b9347..f9752ef1 100644 --- a/backend/azure/azure.go +++ b/backend/azure/azure.go @@ -559,7 +559,7 @@ Pager: objects = append(objects, s3response.Object{ ETag: (*string)(v.Properties.ETag), Key: v.Name, - LastModified: backend.GetStringPtr(v.Properties.LastModified.UTC().Format(backend.RFC3339TimeFormat)), + LastModified: v.Properties.LastModified, Size: v.Properties.ContentLength, StorageClass: types.ObjectStorageClass(*v.Properties.AccessTier), }) @@ -620,7 +620,7 @@ Pager: objects = append(objects, s3response.Object{ ETag: (*string)(v.Properties.ETag), Key: v.Name, - LastModified: backend.GetStringPtr(v.Properties.LastModified.UTC().Format(backend.RFC3339TimeFormat)), + LastModified: v.Properties.LastModified, Size: v.Properties.ContentLength, StorageClass: types.ObjectStorageClass(*v.Properties.AccessTier), }) @@ -868,7 +868,7 @@ func (az *Azure) ListParts(ctx context.Context, input *s3.ListPartsInput) (s3res Size: *el.Size, ETag: *el.Name, PartNumber: partNumber, - LastModified: time.Now().Format(backend.RFC3339TimeFormat), + LastModified: time.Now(), }) if len(parts) >= int(maxParts) { nextPartNumberMarker = partNumber @@ -923,7 +923,7 @@ func (az *Azure) ListMultipartUploads(ctx context.Context, input *s3.ListMultipa } uploads = append(uploads, s3response.Upload{ Key: *el.Name, - Initiated: el.Properties.CreationTime.Format(backend.RFC3339TimeFormat), + Initiated: *el.Properties.CreationTime, }) } } diff --git a/backend/common.go b/backend/common.go index 57ebe5ce..2321d9eb 100644 --- a/backend/common.go +++ b/backend/common.go @@ -30,11 +30,6 @@ import ( "github.com/versity/versitygw/s3response" ) -var ( - // RFC3339TimeFormat RFC3339 time format - RFC3339TimeFormat = "2006-01-02T15:04:05.999Z" -) - func IsValidBucketName(name string) bool { return true } type ByBucketName []s3response.ListAllMyBucketsEntry diff --git a/backend/posix/posix.go b/backend/posix/posix.go index e9839f65..f9f7d015 100644 --- a/backend/posix/posix.go +++ b/backend/posix/posix.go @@ -943,7 +943,7 @@ func (p *Posix) ListMultipartUploads(_ context.Context, mpu *s3.ListMultipartUpl uploads = append(uploads, s3response.Upload{ Key: objectName, UploadID: uploadID, - Initiated: fi.ModTime().Format(backend.RFC3339TimeFormat), + Initiated: fi.ModTime(), }) } } @@ -1088,7 +1088,7 @@ func (p *Posix) ListParts(_ context.Context, input *s3.ListPartsInput) (s3respon parts = append(parts, s3response.Part{ PartNumber: pn, ETag: etag, - LastModified: fi.ModTime().Format(backend.RFC3339TimeFormat), + LastModified: fi.ModTime(), Size: fi.Size(), }) } @@ -2155,12 +2155,12 @@ func (p *Posix) fileToObj(bucket string) backend.GetObjFunc { } size := int64(0) - mDate := fi.ModTime().UTC().Format(backend.RFC3339TimeFormat) + mtime := fi.ModTime() return s3response.Object{ ETag: &etag, Key: &path, - LastModified: &mDate, + LastModified: &mtime, Size: &size, }, nil } @@ -2187,12 +2187,12 @@ func (p *Posix) fileToObj(bucket string) backend.GetObjFunc { } size := fi.Size() - mDate := fi.ModTime().UTC().Format(backend.RFC3339TimeFormat) + mtime := fi.ModTime() return s3response.Object{ ETag: &etag, Key: &path, - LastModified: &mDate, + LastModified: &mtime, Size: &size, }, nil } diff --git a/backend/s3proxy/s3.go b/backend/s3proxy/s3.go index 55cb406a..c87dd841 100644 --- a/backend/s3proxy/s3.go +++ b/backend/s3proxy/s3.go @@ -204,7 +204,7 @@ func (s *S3Proxy) ListMultipartUploads(ctx context.Context, input *s3.ListMultip DisplayName: *u.Owner.DisplayName, }, StorageClass: string(u.StorageClass), - Initiated: u.Initiated.Format(backend.RFC3339TimeFormat), + Initiated: *u.Initiated, }) } @@ -241,7 +241,7 @@ func (s *S3Proxy) ListParts(ctx context.Context, input *s3.ListPartsInput) (s3re for _, p := range output.Parts { parts = append(parts, s3response.Part{ PartNumber: int(*p.PartNumber), - LastModified: p.LastModified.Format(backend.RFC3339TimeFormat), + LastModified: *p.LastModified, ETag: *p.ETag, Size: *p.Size, }) @@ -776,11 +776,10 @@ func convertObjects(objs []types.Object) []s3response.Object { result := make([]s3response.Object, len(objs)) for _, obj := range objs { - lastModified := obj.LastModified.UTC().Format(backend.RFC3339TimeFormat) result = append(result, s3response.Object{ ETag: obj.ETag, Key: obj.Key, - LastModified: &lastModified, + LastModified: obj.LastModified, Owner: obj.Owner, Size: obj.Size, RestoreStatus: obj.RestoreStatus, diff --git a/backend/scoutfs/scoutfs.go b/backend/scoutfs/scoutfs.go index 30961bf4..b9c608d4 100644 --- a/backend/scoutfs/scoutfs.go +++ b/backend/scoutfs/scoutfs.go @@ -858,11 +858,12 @@ func (s *ScoutFS) fileToObj(bucket string) backend.GetObjFunc { } key := path + "/" + mtime := fi.ModTime() return s3response.Object{ ETag: &etag, Key: &key, - LastModified: backend.GetStringPtr(fi.ModTime().UTC().Format(backend.RFC3339TimeFormat)), + LastModified: &mtime, StorageClass: types.ObjectStorageClassStandard, }, nil } @@ -905,11 +906,12 @@ func (s *ScoutFS) fileToObj(bucket string) backend.GetObjFunc { } size := fi.Size() + mtime := fi.ModTime() return s3response.Object{ ETag: &etag, Key: &path, - LastModified: backend.GetStringPtr(fi.ModTime().UTC().Format(backend.RFC3339TimeFormat)), + LastModified: &mtime, Size: &size, StorageClass: sc, }, nil diff --git a/backend/walk_test.go b/backend/walk_test.go index 9adbc4c0..e07061c7 100644 --- a/backend/walk_test.go +++ b/backend/walk_test.go @@ -44,11 +44,12 @@ func getObj(path string, d fs.DirEntry) (s3response.Object, error) { if err != nil { return s3response.Object{}, fmt.Errorf("get fileinfo: %w", err) } + mtime := fi.ModTime() return s3response.Object{ ETag: &etag, Key: &path, - LastModified: backend.GetStringPtr(fi.ModTime().UTC().Format(backend.RFC3339TimeFormat)), + LastModified: &mtime, }, nil } @@ -60,11 +61,12 @@ func getObj(path string, d fs.DirEntry) (s3response.Object, error) { } size := fi.Size() + mtime := fi.ModTime() return s3response.Object{ ETag: &etag, Key: &path, - LastModified: backend.GetStringPtr(fi.ModTime().UTC().Format(backend.RFC3339TimeFormat)), + LastModified: &mtime, Size: &size, }, nil } diff --git a/s3response/s3response.go b/s3response/s3response.go index dfe8c77e..8c2c58a0 100644 --- a/s3response/s3response.go +++ b/s3response/s3response.go @@ -21,14 +21,30 @@ import ( "github.com/aws/aws-sdk-go-v2/service/s3/types" ) +const RFC3339TimeFormat = "2006-01-02T15:04:05.999Z" + // Part describes part metadata. type Part struct { PartNumber int - LastModified string + LastModified time.Time ETag string Size int64 } +func (p Part) MarshalXML(e *xml.Encoder, start xml.StartElement) error { + type Alias Part + aux := &struct { + LastModified string `xml:"LastModified"` + *Alias + }{ + Alias: (*Alias)(&p), + } + + aux.LastModified = p.LastModified.Format(RFC3339TimeFormat) + + return e.EncodeElement(aux, start) +} + // ListPartsResponse - s3 api list parts response. type ListPartsResult struct { XMLName xml.Name `xml:"http://s3.amazonaws.com/doc/2006-03-01/ ListPartsResult" json:"-"` @@ -124,13 +140,30 @@ type ListObjectsV2Result struct { type Object struct { ETag *string Key *string - LastModified *string + LastModified *time.Time Owner *types.Owner RestoreStatus *types.RestoreStatus Size *int64 StorageClass types.ObjectStorageClass } +func (o Object) MarshalXML(e *xml.Encoder, start xml.StartElement) error { + type Alias Object + aux := &struct { + LastModified *string `xml:"LastModified,omitempty"` + *Alias + }{ + Alias: (*Alias)(&o), + } + + if o.LastModified != nil { + formattedTime := o.LastModified.Format(RFC3339TimeFormat) + aux.LastModified = &formattedTime + } + + return e.EncodeElement(aux, start) +} + // Upload describes in progress multipart upload type Upload struct { Key string @@ -138,7 +171,21 @@ type Upload struct { Initiator Initiator Owner Owner StorageClass string - Initiated string + Initiated time.Time +} + +func (u Upload) MarshalXML(e *xml.Encoder, start xml.StartElement) error { + type Alias Upload + aux := &struct { + Initiated string `xml:"Initiated"` + *Alias + }{ + Alias: (*Alias)(&u), + } + + aux.Initiated = u.Initiated.Format(RFC3339TimeFormat) + + return e.EncodeElement(aux, start) } // CommonPrefix ListObjectsResponse common prefixes (directory abstraction)