Added support for trailing dot transfers to unsafe locations (#2827)

* Add AllowToUnsafeDestination

* implementation to stop msg sends on a closed channel

* RWMutex added on channel closed flag

* mutex synchronism mechanism added to channel sends and closures

* replaced mutex panic handling implementation with recover

* replace console msgs to log msg

* modified panic handling

* removed unnecessary print stmt

* added unsafe destination trailing dot option and tests

* testing for trailing dot

* added unsafe destination trailing dot option and tests

* testing for trailing dot

* set AllowToUnsafeDestination on client + tests

* test for third trailing dot option

* widened the client options type

* address Pr comments

* Update e2etest/zt_newe2e_basic_functionality_test.go

Co-authored-by: Gauri Lamunion <[email protected]>

* small fixes

* fix tests

* cleaned tests

* fixed tests

* removed list test

* trailing dot should be stripped in tests

---------

Co-authored-by: Adele Reed <[email protected]>
Co-authored-by: Gauri Lamunion <[email protected]>

Author: 3 people

cmd/copy.go

@@ -1568,7 +1568,7 @@ func (cca *CookedCopyCmdArgs) processCopyJobPartOrders() (err error) {
 	var azureFileSpecificOptions any
 	if cca.FromTo.From() == common.ELocation.File() {
 		azureFileSpecificOptions = &common.FileClientOptions{
-			AllowTrailingDot: cca.trailingDot == common.ETrailingDotOption.Enable(),
+			AllowTrailingDot: cca.trailingDot.IsEnabled(),
 		}
 	}
 
@@ -1590,8 +1590,8 @@ func (cca *CookedCopyCmdArgs) processCopyJobPartOrders() (err error) {
 
 	if cca.FromTo.To() == common.ELocation.File() {
 		azureFileSpecificOptions = &common.FileClientOptions{
-			AllowTrailingDot:       cca.trailingDot == common.ETrailingDotOption.Enable(),
-			AllowSourceTrailingDot: cca.trailingDot == common.ETrailingDotOption.Enable() && cca.FromTo.From() == common.ELocation.File(),
+			AllowTrailingDot:       cca.trailingDot.IsEnabled(),
+			AllowSourceTrailingDot: cca.trailingDot.IsEnabled() && cca.FromTo.From() == common.ELocation.File(),
 		}
 	}
 
@@ -2134,9 +2134,11 @@ func init() {
 	// so properties can be get in parallel, at same time no additional go routines are created for this specific job.
 	// The usage of this hidden flag is to provide fallback to traditional behavior, when service supports returning full properties during list.
 	cpCmd.PersistentFlags().BoolVar(&raw.s2sGetPropertiesInBackend, "s2s-get-properties-in-backend", true, "True by default. Gets S3 objects' or Azure files' properties in backend, if properties need to be accessed. Properties need to be accessed if s2s-preserve-properties is true, and in certain other cases where we need the properties for modification time checks or MD5 checks.")
-	cpCmd.PersistentFlags().StringVar(&raw.trailingDot, "trailing-dot", "", "'Enable' by default to treat file share related operations in a safe manner. Available options: "+strings.Join(common.ValidTrailingDotOptions(), ", ")+". "+
-		"Choose 'Disable' to go back to legacy (potentially unsafe) treatment of trailing dot files where the file service will trim any trailing dots in paths. This can result in potential data corruption if the transfer contains two paths that differ only by a trailing dot (ex: mypath and mypath.). If this flag is set to 'Disable' and AzCopy encounters a trailing dot file, it will warn customers in the scanning log but will not attempt to abort the operation."+
-		"If the destination does not support trailing dot files (Windows or Blob Storage), AzCopy will fail if the trailing dot file is the root of the transfer and skip any trailing dot paths encountered during enumeration.")
+	cpCmd.PersistentFlags().StringVar(&raw.trailingDot, "trailing-dot", "", "Available options: "+strings.Join(common.ValidTrailingDotOptions(), ", ")+". "+
+		"'Enable'(Default) treats trailing dot file operations in a safe manner between systems that support these files. On Windows, the transfers will not occur to stop risk of data corruption. See 'AllowToUnsafeDestination' to bypass this."+
+		"'Disable' reverts to the legacy functionality, where trailing dot files are ignored. This can result in potential data corruption if the transfer contains two paths that differ only by a trailing dot (E.g 'path/foo' and 'path/foo.'). If this flag is set to 'Disable' and AzCopy encounters a trailing dot file, it will warn customers in the scanning log but will not attempt to abort the operation."+
+		"If the destination does not support trailing dot files (Windows or Blob Storage), AzCopy will fail if the trailing dot file is the root of the transfer and skip any trailing dot paths encountered during enumeration."+
+		"'AllowToUnsafeDestination' supports transferring trailing dot files to systems that do not support them e.g Windows. Use with caution acknowledging risk of data corruption, when two files with different contents 'path/bar' and 'path/bar.' (differ only by a trailing dot) are seen as identical.")
 
 	// Public Documentation: https://docs.microsoft.com/en-us/azure/storage/blobs/encryption-customer-provided-keys
 	// Clients making requests against Azure Blob storage have the option to provide an encryption key on a per-request basis.

cmd/jobsResume.go

@@ -295,8 +295,28 @@ func (rca resumeCmdArgs) getSourceAndDestinationServiceClients(
 	}
 
 	options := createClientOptions(common.AzcopyCurrentJobLogger, nil)
+	jobID, err := common.ParseJobID(rca.jobID)
+	if err != nil {
+		// Error for invalid JobId format
+		return nil, nil, fmt.Errorf("error parsing the jobId %s. Failed with error %s", rca.jobID, err.Error())
+	}
+
+	var getJobDetailsResponse common.GetJobDetailsResponse
+	// Get job details from the STE
+	Rpc(common.ERpcCmd.GetJobDetails(),
+		&common.GetJobDetailsRequest{JobID: jobID},
+		&getJobDetailsResponse)
+	if getJobDetailsResponse.ErrorMsg != "" {
+		glcm.Error(getJobDetailsResponse.ErrorMsg)
+	}
 
-	srcServiceClient, err := common.GetServiceClientForLocation(fromTo.From(), source, srcCredType, tc, &options, nil)
+	var fileSrcClientOptions any
+	if fromTo.From() == common.ELocation.File() {
+		fileSrcClientOptions = &common.FileClientOptions{
+			AllowTrailingDot: getJobDetailsResponse.TrailingDot.IsEnabled(), //Access the trailingDot option of the job
+		}
+	}
+	srcServiceClient, err := common.GetServiceClientForLocation(fromTo.From(), source, srcCredType, tc, &options, fileSrcClientOptions)
 	if err != nil {
 		return nil, nil, err
 	}
@@ -306,11 +326,17 @@ func (rca resumeCmdArgs) getSourceAndDestinationServiceClients(
 		srcCred = common.NewScopedCredential(tc, srcCredType)
 	}
 	options = createClientOptions(common.AzcopyCurrentJobLogger, srcCred)
-	dstServiceClient, err := common.GetServiceClientForLocation(fromTo.To(), destination, dstCredType, tc, &options, nil)
+	var fileClientOptions any
+	if fromTo.To() == common.ELocation.File() {
+		fileClientOptions = &common.FileClientOptions{
+			AllowSourceTrailingDot: getJobDetailsResponse.TrailingDot.IsEnabled() && fromTo.From() == common.ELocation.File(),
+			AllowTrailingDot:       getJobDetailsResponse.TrailingDot.IsEnabled(),
+		}
+	}
+	dstServiceClient, err := common.GetServiceClientForLocation(fromTo.To(), destination, dstCredType, tc, &options, fileClientOptions)
 	if err != nil {
 		return nil, nil, err
 	}
-
 	return srcServiceClient, dstServiceClient, nil
 }
 
@@ -357,9 +383,9 @@ func (rca resumeCmdArgs) process() error {
 	}
 
 	// Get fromTo info, so we can decide what's the proper credential type to use.
-	var getJobFromToResponse common.GetJobFromToResponse
-	Rpc(common.ERpcCmd.GetJobFromTo(),
-		&common.GetJobFromToRequest{JobID: jobID},
+	var getJobFromToResponse common.GetJobDetailsResponse
+	Rpc(common.ERpcCmd.GetJobDetails(),
+		&common.GetJobDetailsRequest{JobID: jobID},
 		&getJobFromToResponse)
 	if getJobFromToResponse.ErrorMsg != "" {
 		glcm.Error(getJobFromToResponse.ErrorMsg)

cmd/removeEnumerator.go

@@ -93,7 +93,7 @@ func newRemoveEnumerator(cca *CookedCopyCmdArgs) (enumerator *CopyEnumerator, er
 	options := createClientOptions(common.AzcopyCurrentJobLogger, nil)
 	var fileClientOptions any
 	if cca.FromTo.From() == common.ELocation.File() {
-		fileClientOptions = &common.FileClientOptions{AllowTrailingDot: cca.trailingDot == common.ETrailingDotOption.Enable()}
+		fileClientOptions = &common.FileClientOptions{AllowTrailingDot: cca.trailingDot.IsEnabled()}
 	}
 	targetServiceClient, err := common.GetServiceClientForLocation(
 		cca.FromTo.From(),

cmd/rpc.go

@@ -59,8 +59,8 @@ func inprocSend(rpcCmd common.RpcCmd, requestData interface{}, responseData inte
 	case common.ERpcCmd.ResumeJob():
 		*(responseData.(*common.CancelPauseResumeResponse)) = jobsAdmin.ResumeJobOrder(*requestData.(*common.ResumeJobRequest))
 
-	case common.ERpcCmd.GetJobFromTo():
-		*(responseData.(*common.GetJobFromToResponse)) = jobsAdmin.GetJobFromTo(*requestData.(*common.GetJobFromToRequest))
+	case common.ERpcCmd.GetJobDetails():
+		*(responseData.(*common.GetJobDetailsResponse)) = jobsAdmin.GetJobDetails(*requestData.(*common.GetJobDetailsRequest))
 
 	default:
 		panic(fmt.Errorf("Unrecognized RpcCmd: %q", rpcCmd.String()))

cmd/setPropertiesEnumerator.go

@@ -75,7 +75,7 @@ func setPropertiesEnumerator(cca *CookedCopyCmdArgs) (enumerator *CopyEnumerator
 	options := createClientOptions(common.AzcopyCurrentJobLogger, nil)
 	var fileClientOptions any
 	if cca.FromTo.From() == common.ELocation.File() {
-		fileClientOptions = &common.FileClientOptions{AllowTrailingDot: cca.trailingDot == common.ETrailingDotOption.Enable()}
+		fileClientOptions = &common.FileClientOptions{AllowTrailingDot: cca.trailingDot.IsEnabled()}
 	}
 
 	targetServiceClient, err := common.GetServiceClientForLocation(

cmd/zc_enumerator.go

@@ -495,7 +495,7 @@ func InitResourceTraverser(resource common.ResourceString, location common.Locat
 		fileURLParts.ShareSnapshot = ""
 		fileURLParts.DirectoryOrFilePath = ""
 		fileOptions := &common.FileClientOptions{
-			AllowTrailingDot: trailingDot == common.ETrailingDotOption.Enable(),
+			AllowTrailingDot: trailingDot.IsEnabled(),
 		}
 
 		res, err := SplitResourceString(fileURLParts.String(), common.ELocation.File())

cmd/zc_traverser_file.go

@@ -48,8 +48,8 @@ type fileTraverser struct {
 
 	// a generic function to notify that a new stored object has been enumerated
 	incrementEnumerationCounter enumerationCounterFunc
-	trailingDot common.TrailingDotOption
-	destination *common.Location
+	trailingDot                 common.TrailingDotOption
+	destination                 *common.Location
 }
 
 func createShareClientFromServiceClient(fileURLParts file.URLParts, client *service.Client) (*share.Client, error) {
@@ -123,6 +123,10 @@ func (t *fileTraverser) getPropertiesIfSingleFile() (*file.GetPropertiesResponse
 func (t *fileTraverser) Traverse(preprocessor objectMorpher, processor objectProcessor, filters []ObjectFilter) (err error) {
 	invalidBlobOrWindowsName := func(path string) bool {
 		if t.destination != nil {
+			if t.trailingDot == common.ETrailingDotOption.AllowToUnsafeDestination() && (*t.destination != common.ELocation.Blob() || *t.destination != common.ELocation.BlobFS()) { // Allow only Local, Trailing dot files not supported in Blob
+				return false // Please let me shoot myself in the foot!
+			}
+
 			if (t.destination.IsLocal() && runtime.GOOS == "windows") || *t.destination == common.ELocation.Blob() || *t.destination == common.ELocation.BlobFS() {
 				/* Blob or Windows object name is invalid if it ends with period or
 				   one of (virtual) directories in path ends with period.
@@ -145,7 +149,7 @@ func (t *fileTraverser) Traverse(preprocessor objectMorpher, processor objectPro
 			WarnStdoutAndScanningLog(fmt.Sprintf(invalidNameErrorMsg, targetURLParts.DirectoryOrFilePath))
 			return common.EAzError.InvalidBlobOrWindowsName()
 		}
-		if t.trailingDot != common.ETrailingDotOption.Enable() && strings.HasSuffix(targetURLParts.DirectoryOrFilePath, ".") {
+		if !t.trailingDot.IsEnabled() && strings.HasSuffix(targetURLParts.DirectoryOrFilePath, ".") {
 			azcopyScanningLogger.Log(common.LogWarning, fmt.Sprintf(trailingDotErrMsg, getObjectNameOnly(targetURLParts.DirectoryOrFilePath)))
 		}
 		// check if the url points to a single file
@@ -288,7 +292,7 @@ func (t *fileTraverser) Traverse(preprocessor objectMorpher, processor objectPro
 					WarnStdoutAndScanningLog(fmt.Sprintf(invalidNameErrorMsg, *fileInfo.Name))
 					continue
 				} else {
-					if t.trailingDot != common.ETrailingDotOption.Enable() && strings.HasSuffix(*fileInfo.Name, ".") {
+					if !t.trailingDot.IsEnabled() && strings.HasSuffix(*fileInfo.Name, ".") {
 						azcopyScanningLogger.Log(common.LogWarning, fmt.Sprintf(trailingDotErrMsg, *fileInfo.Name))
 					}
 				}
@@ -301,13 +305,13 @@ func (t *fileTraverser) Traverse(preprocessor objectMorpher, processor objectPro
 					WarnStdoutAndScanningLog(fmt.Sprintf(invalidNameErrorMsg, *dirInfo.Name))
 					continue
 				} else {
-					if t.trailingDot != common.ETrailingDotOption.Enable() && strings.HasSuffix(*dirInfo.Name, ".") {
+					if !t.trailingDot.IsEnabled() && strings.HasSuffix(*dirInfo.Name, ".") {
 						azcopyScanningLogger.Log(common.LogWarning, fmt.Sprintf(trailingDotErrMsg, *dirInfo.Name))
 					}
 				}
 				enqueueOutput(newAzFileSubdirectoryEntity(currentDirectoryClient, *dirInfo.Name), nil)
 				if t.recursive {
-						// If recursive is turned on, add sub directories to be processed
+					// If recursive is turned on, add sub directories to be processed
 					enqueueDir(currentDirectoryClient.NewSubdirectoryClient(*dirInfo.Name))
 				}
 
@@ -381,14 +385,14 @@ func (t *fileTraverser) Traverse(preprocessor objectMorpher, processor objectPro
 
 func newFileTraverser(rawURL string, serviceClient *service.Client, ctx context.Context, recursive, getProperties bool, incrementEnumerationCounter enumerationCounterFunc, trailingDot common.TrailingDotOption, destination *common.Location) (t *fileTraverser) {
 	t = &fileTraverser{
-		rawURL: rawURL,
-		serviceClient: serviceClient,
-		ctx: ctx,
-		recursive: recursive,
-		getProperties: getProperties,
+		rawURL:                      rawURL,
+		serviceClient:               serviceClient,
+		ctx:                         ctx,
+		recursive:                   recursive,
+		getProperties:               getProperties,
 		incrementEnumerationCounter: incrementEnumerationCounter,
-		trailingDot: trailingDot,
-		destination: destination,
+		trailingDot:                 trailingDot,
+		destination:                 destination,
 	}
 	return
 }

cmd/zt_interceptors_for_test.go

@@ -53,7 +53,7 @@ func (i *interceptor) intercept(cmd common.RpcCmd, request interface{}, response
 	case common.ERpcCmd.PauseJob():
 	case common.ERpcCmd.CancelJob():
 	case common.ERpcCmd.ResumeJob():
-	case common.ERpcCmd.GetJobFromTo():
+	case common.ERpcCmd.GetJobDetails():
 		fallthrough
 	default:
 		panic("RPC mock not implemented")

common/fe-ste-models.go

@@ -153,8 +153,15 @@ var ETrailingDotOption = TrailingDotOption(0)
 
 type TrailingDotOption uint8
 
-func (TrailingDotOption) Enable() TrailingDotOption  { return TrailingDotOption(0) }
-func (TrailingDotOption) Disable() TrailingDotOption { return TrailingDotOption(1) }
+func (TrailingDotOption) Enable() TrailingDotOption                   { return TrailingDotOption(0) }
+func (TrailingDotOption) Disable() TrailingDotOption                  { return TrailingDotOption(1) }
+func (TrailingDotOption) AllowToUnsafeDestination() TrailingDotOption { return TrailingDotOption(2) }
+
+// Trailing dots are supported in the Enable and AllowToUnsafeDestination options
+func (d TrailingDotOption) IsEnabled() bool {
+	return d == d.Enable() ||
+		d == d.AllowToUnsafeDestination()
+}
 
 func (d TrailingDotOption) String() string {
 	return enum.StringInt(d, reflect.TypeOf(d))
@@ -178,6 +185,7 @@ func ValidTrailingDotOptions() []string {
 	return []string{
 		ETrailingDotOption.Enable().String(),
 		ETrailingDotOption.Disable().String(),
+		ETrailingDotOption.AllowToUnsafeDestination().String(),
 	}
 }
 

common/rpc-models.go

@@ -27,7 +27,7 @@ func (RpcCmd) ListJobTransfers() RpcCmd   { return RpcCmd("ListJobTransfers") }
 func (RpcCmd) CancelJob() RpcCmd          { return RpcCmd("Cancel") }
 func (RpcCmd) PauseJob() RpcCmd           { return RpcCmd("PauseJob") }
 func (RpcCmd) ResumeJob() RpcCmd          { return RpcCmd("ResumeJob") }
-func (RpcCmd) GetJobFromTo() RpcCmd       { return RpcCmd("GetJobFromTo") }
+func (RpcCmd) GetJobDetails() RpcCmd      { return RpcCmd("GetJobDetails") }
 
 func (c RpcCmd) String() string {
 	return enum.String(c, reflect.TypeOf(c))
@@ -375,15 +375,16 @@ type ListJobTransfersResponse struct {
 	Details  []TransferDetail
 }
 
-// GetJobFromToRequest indicates request to get job's FromTo info from job part plan header
-type GetJobFromToRequest struct {
+// GetJobDetailsRequest indicates request to get job's FromTo and TrailingDot info from job part plan header
+type GetJobDetailsRequest struct {
 	JobID JobID
 }
 
-// GetJobFromToResponse indicates response to get job's FromTo info.
-type GetJobFromToResponse struct {
+// GetJobDetailsResponse indicates response to get job's FromTo and TrailingDot info.
+type GetJobDetailsResponse struct {
 	ErrorMsg    string
 	FromTo      FromTo
 	Source      string
 	Destination string
+	TrailingDot TrailingDotOption
 }