From 58e44ac6648fb0623703ae59adca2e949dda80ae Mon Sep 17 00:00:00 2001 From: Adele Reed Date: Tue, 8 Nov 2022 15:29:04 -0800 Subject: [PATCH 1/4] Fix metadata parsing --- common/fe-ste-models.go | 30 ++++++++++++++++-------------- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/common/fe-ste-models.go b/common/fe-ste-models.go index a0a32f4e0..e2faa4a5c 100644 --- a/common/fe-ste-models.go +++ b/common/fe-ste-models.go @@ -23,6 +23,7 @@ package common import ( "bytes" "encoding/json" + "errors" "fmt" "math" "os" @@ -110,7 +111,7 @@ type PartNumber uint32 type Version uint32 type Status uint32 -//////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// +// ////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// var EDeleteSnapshotsOption = DeleteSnapshotsOption(0) type DeleteSnapshotsOption uint8 @@ -145,7 +146,7 @@ func (d DeleteSnapshotsOption) ToDeleteSnapshotsOptionType() azblob.DeleteSnapsh return azblob.DeleteSnapshotsOptionType(strings.ToLower(d.String())) } -//////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// +// ////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// var EPermanentDeleteOption = PermanentDeleteOption(3) // Default to "None" type PermanentDeleteOption uint8 @@ -610,7 +611,7 @@ func (ft *FromTo) IsPropertyOnlyTransfer() bool { var BenchmarkLmt = time.Date(1900, 1, 1, 0, 0, 0, 0, time.UTC) -//////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// +// ////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// // Enumerates the values for blob type. type BlobType uint8 @@ -674,9 +675,11 @@ type TransferStatus int32 // Must be 32-bit for atomic operations; negative #s r func (TransferStatus) NotStarted() TransferStatus { return TransferStatus(0) } // TODO confirm whether this is actually needed -// Outdated: -// Transfer started & at least 1 chunk has successfully been transferred. -// Used to resume a transfer that started to avoid transferring all chunks thereby improving performance +// +// Outdated: +// Transfer started & at least 1 chunk has successfully been transferred. +// Used to resume a transfer that started to avoid transferring all chunks thereby improving performance +// // Update(Jul 2020): This represents the state of transfer as soon as the file is scheduled. func (TransferStatus) Started() TransferStatus { return TransferStatus(1) } @@ -972,7 +975,7 @@ func (i *InvalidMetadataHandleOption) UnmarshalJSON(b []byte) error { return i.Parse(s) } -//////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// +// ////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// const ( DefaultBlockBlobBlockSize = 8 * 1024 * 1024 MaxBlockBlobBlockSize = 4000 * 1024 * 1024 @@ -1076,12 +1079,11 @@ func StringToMetadata(metadataString string) (Metadata, error) { metadataMap := Metadata{} if len(metadataString) > 0 { for _, keyAndValue := range strings.Split(metadataString, ";") { // key/value pairs are separated by ';' - kv := strings.Split(keyAndValue, "=") // key/value are separated by '=' - // what if '=' not present? - if len(kv) != 2 { - return metadataMap, fmt.Errorf("invalid metadata string passed") + kEnd := strings.IndexByte(keyAndValue, '=') + if kEnd == -1 { + return nil, errors.New("invalid metadata; no value present with key") } - metadataMap[kv[0]] = kv[1] + metadataMap[keyAndValue[:kEnd]] = keyAndValue[kEnd+1:] } } return metadataMap, nil @@ -1561,7 +1563,7 @@ func GetClientProvidedKey(options CpkOptions) azblob.ClientProvidedKeyOptions { return ToClientProvidedKeyOptions(_cpkInfo, _cpkScopeInfo) } -//////////////////////////////////////////////////////////////////////////////// +// ////////////////////////////////////////////////////////////////////////////// type SetPropertiesFlags uint32 // [0000000000...32 times] var ESetPropertiesFlags = SetPropertiesFlags(0) @@ -1584,7 +1586,7 @@ func (op *SetPropertiesFlags) ShouldTransferBlobTags() bool { return (*op)&ESetPropertiesFlags.SetBlobTags() == ESetPropertiesFlags.SetBlobTags() } -//////////////////////////////////////////////////////////////////////////////// +// ////////////////////////////////////////////////////////////////////////////// type RehydratePriorityType uint8 var ERehydratePriorityType = RehydratePriorityType(0) // setting default as none From 703dda347b32ee7e4870955346330f8c153f6948 Mon Sep 17 00:00:00 2001 From: Adele Reed Date: Tue, 17 Jan 2023 13:29:11 -0800 Subject: [PATCH 2/4] rework metadata parsing to be more robust; add test --- common/fe-ste-models.go | 51 +++++++++++++++++++++++--- e2etest/zt_preserve_properties_test.go | 4 +- ste/mgr-JobPartMgr.go | 7 ++-- 3 files changed, 52 insertions(+), 10 deletions(-) diff --git a/common/fe-ste-models.go b/common/fe-ste-models.go index e2faa4a5c..a6b388df9 100644 --- a/common/fe-ste-models.go +++ b/common/fe-ste-models.go @@ -1078,12 +1078,53 @@ func UnMarshalToCommonMetadata(metadataString string) (Metadata, error) { func StringToMetadata(metadataString string) (Metadata, error) { metadataMap := Metadata{} if len(metadataString) > 0 { - for _, keyAndValue := range strings.Split(metadataString, ";") { // key/value pairs are separated by ';' - kEnd := strings.IndexByte(keyAndValue, '=') - if kEnd == -1 { - return nil, errors.New("invalid metadata; no value present with key") + cKey := "" + cVal := "" + keySet := false + ignoreRules := false + + addchar := func(c rune) { + if !keySet { + cKey += string(c) + } else { + cVal += string(c) + } + } + for _, c := range metadataString { + if ignoreRules { + addchar(c) + ignoreRules = false + } else { + switch c { + case '=': + if keySet { + addchar(c) + } else { + keySet = true + } + + case ';': + if !keySet { + return Metadata{}, errors.New("metadata names must conform to C# naming rules (https://learn.microsoft.com/en-us/rest/api/storageservices/naming-and-referencing-containers--blobs--and-metadata#metadata-names)") + } + + metadataMap[cKey] = cVal + cKey = "" + cVal = "" + keySet = false + ignoreRules = false + + case '\\': + ignoreRules = true // ignore the rules on the next character + + default: + addchar(c) + } } - metadataMap[keyAndValue[:kEnd]] = keyAndValue[kEnd+1:] + } + + if cKey != "" { + metadataMap[cKey] = cVal } } return metadataMap, nil diff --git a/e2etest/zt_preserve_properties_test.go b/e2etest/zt_preserve_properties_test.go index a2d7bec09..97ebc110e 100644 --- a/e2etest/zt_preserve_properties_test.go +++ b/e2etest/zt_preserve_properties_test.go @@ -42,10 +42,10 @@ func TestProperties_NameValueMetadataIsPreservedS2S(t *testing.T) { } func TestProperties_NameValueMetadataCanBeUploaded(t *testing.T) { - expectedMap := map[string]string{"foo": "abc", "bar": "def"} + expectedMap := map[string]string{"foo": "abc", "bar": "def", "baz": "te=s;t"} RunScenarios(t, eOperation.Copy(), eTestFromTo.AllUploads(), eValidate.Auto(), anonymousAuthOnly, anonymousAuthOnly, params{ recursive: true, - metadata: "foo=abc;bar=def", + metadata: "foo=abc;bar=def;baz=te=s\\;t", }, nil, testFiles{ defaultSize: "1K", shouldTransfer: []interface{}{ diff --git a/ste/mgr-JobPartMgr.go b/ste/mgr-JobPartMgr.go index 6e1e1b606..b11e67759 100644 --- a/ste/mgr-JobPartMgr.go +++ b/ste/mgr-JobPartMgr.go @@ -377,9 +377,10 @@ func (jpm *jobPartMgr) ScheduleTransfers(jobCtx context.Context) { metadataString := string(dstData.Metadata[:dstData.MetadataLength]) jpm.metadata = common.Metadata{} if len(metadataString) > 0 { - for _, keyAndValue := range strings.Split(metadataString, ";") { // key/value pairs are separated by ';' - kv := strings.Split(keyAndValue, "=") // key/value are separated by '=' - jpm.metadata[kv[0]] = kv[1] + var err error + jpm.metadata, err = common.StringToMetadata(metadataString) + if err != nil { + panic("sanity check: metadata string should be valid at this point: " + metadataString) } } blobTagsStr := string(dstData.BlobTags[:dstData.BlobTagsLength]) From de124e44d990e73e3a5cdebb7cdf28016c6b5a8d Mon Sep 17 00:00:00 2001 From: Adele Reed Date: Tue, 17 Jan 2023 13:35:30 -0800 Subject: [PATCH 3/4] Fix comment lines --- common/fe-ste-models.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/common/fe-ste-models.go b/common/fe-ste-models.go index a6b388df9..f6160bb61 100644 --- a/common/fe-ste-models.go +++ b/common/fe-ste-models.go @@ -111,7 +111,7 @@ type PartNumber uint32 type Version uint32 type Status uint32 -// ////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// +//////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// var EDeleteSnapshotsOption = DeleteSnapshotsOption(0) type DeleteSnapshotsOption uint8 @@ -146,7 +146,7 @@ func (d DeleteSnapshotsOption) ToDeleteSnapshotsOptionType() azblob.DeleteSnapsh return azblob.DeleteSnapshotsOptionType(strings.ToLower(d.String())) } -// ////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// +//////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// var EPermanentDeleteOption = PermanentDeleteOption(3) // Default to "None" type PermanentDeleteOption uint8 @@ -611,7 +611,7 @@ func (ft *FromTo) IsPropertyOnlyTransfer() bool { var BenchmarkLmt = time.Date(1900, 1, 1, 0, 0, 0, 0, time.UTC) -// ////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// +//////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// // Enumerates the values for blob type. type BlobType uint8 @@ -975,7 +975,7 @@ func (i *InvalidMetadataHandleOption) UnmarshalJSON(b []byte) error { return i.Parse(s) } -// ////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// +//////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// const ( DefaultBlockBlobBlockSize = 8 * 1024 * 1024 MaxBlockBlobBlockSize = 4000 * 1024 * 1024 @@ -1604,7 +1604,7 @@ func GetClientProvidedKey(options CpkOptions) azblob.ClientProvidedKeyOptions { return ToClientProvidedKeyOptions(_cpkInfo, _cpkScopeInfo) } -// ////////////////////////////////////////////////////////////////////////////// +//////////////////////////////////////////////////////////////////////////////// type SetPropertiesFlags uint32 // [0000000000...32 times] var ESetPropertiesFlags = SetPropertiesFlags(0) @@ -1627,7 +1627,7 @@ func (op *SetPropertiesFlags) ShouldTransferBlobTags() bool { return (*op)&ESetPropertiesFlags.SetBlobTags() == ESetPropertiesFlags.SetBlobTags() } -// ////////////////////////////////////////////////////////////////////////////// +//////////////////////////////////////////////////////////////////////////////// type RehydratePriorityType uint8 var ERehydratePriorityType = RehydratePriorityType(0) // setting default as none From c6d9d0b5fd8920eee39cca43f6d11e20506280bb Mon Sep 17 00:00:00 2001 From: Adele Reed Date: Wed, 18 Jan 2023 14:44:00 -0800 Subject: [PATCH 4/4] Codespell :| --- e2etest/zt_preserve_properties_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/e2etest/zt_preserve_properties_test.go b/e2etest/zt_preserve_properties_test.go index 97ebc110e..d94a9514e 100644 --- a/e2etest/zt_preserve_properties_test.go +++ b/e2etest/zt_preserve_properties_test.go @@ -42,10 +42,10 @@ func TestProperties_NameValueMetadataIsPreservedS2S(t *testing.T) { } func TestProperties_NameValueMetadataCanBeUploaded(t *testing.T) { - expectedMap := map[string]string{"foo": "abc", "bar": "def", "baz": "te=s;t"} + expectedMap := map[string]string{"foo": "abc", "bar": "def", "baz": "state=a;b"} RunScenarios(t, eOperation.Copy(), eTestFromTo.AllUploads(), eValidate.Auto(), anonymousAuthOnly, anonymousAuthOnly, params{ recursive: true, - metadata: "foo=abc;bar=def;baz=te=s\\;t", + metadata: "foo=abc;bar=def;baz=state=a\\;b", }, nil, testFiles{ defaultSize: "1K", shouldTransfer: []interface{}{