Skip to content

Commit

Permalink
Fix folder overwrite prompt (#989)
Browse files Browse the repository at this point in the history
  • Loading branch information
zezha-msft authored and JohnRusk committed Apr 30, 2020
1 parent e91122d commit 43e7102
Show file tree
Hide file tree
Showing 4 changed files with 42 additions and 13 deletions.
6 changes: 6 additions & 0 deletions ChangeLog.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,12 @@

# Change Log

## Version 10.4.2

### Bug fixes

1. Fixed bug in overwrite prompt for folders.

## Version 10.4.1

### New features
Expand Down
19 changes: 15 additions & 4 deletions common/folderCreationTracker.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ package common

import (
"net/url"
"strings"
"sync"
)

Expand Down Expand Up @@ -83,10 +84,20 @@ func (f *simpleFolderTracker) ShouldSetProperties(folder string, overwrite Overw

// prompt only if we didn't create this folder
if overwrite == EOverwriteOption.Prompt() && !exists {
// get rid of SAS before prompting
parsedURL, _ := url.Parse(folder)
parsedURL.RawQuery = ""
return prompter.ShouldOverwrite(parsedURL.String(), EEntityType.Folder())
cleanedFolderPath := folder

// if it's a local Windows path, skip since it doesn't have SAS and won't parse correctly as an URL
if !strings.HasPrefix(folder, EXTENDED_PATH_PREFIX) {
// get rid of SAS before prompting
parsedURL, _ := url.Parse(folder)

// really make sure that it's not a local path
if parsedURL.Scheme != "" && parsedURL.Host != "" {
parsedURL.RawQuery = ""
cleanedFolderPath = parsedURL.String()
}
}
return prompter.ShouldOverwrite(cleanedFolderPath, EEntityType.Folder())
}

return exists
Expand Down
2 changes: 1 addition & 1 deletion common/version.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
package common

const AzcopyVersion = "10.4.1"
const AzcopyVersion = "10.4.2"
const UserAgent = "AzCopy/" + AzcopyVersion
const S3ImportUserAgent = "S3Import " + UserAgent
const BenchmarkUserAgent = "Benchmark " + UserAgent
28 changes: 20 additions & 8 deletions common/writeThoughFile.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,27 +51,39 @@ func CreateDirectoryIfNotExist(directory string, tracker FolderCreationTracker)
return nil
}

// try to create the root directory if the source does
// try to create the directory if it does not already exist
if _, err := OSStat(directory); err != nil {
// if the error is present, try to create the directory
// stat errors can be present in write-only scenarios, when the directory isn't present, etc.
// as a result, we care more about the mkdir error than the stat error, because that's the tell.
// It'd seem that mkdirall would be necessary to port like osstat and osopenfile for new folders in a already no-access dest,
// But in testing, this isn't the case.
err := os.MkdirAll(directory, os.ModePerm)
// if MkdirAll succeeds, no error is dropped-- it is nil.
// therefore, returning here is perfectly acceptable as it either succeeds (or it doesn't)
// first make sure the parent directory exists but we ignore any error that comes back
CreateParentDirectoryIfNotExist(directory, tracker)

// then create the directory
mkDirErr := os.Mkdir(directory, os.ModePerm)

if err == nil {
// if Mkdir succeeds, no error is dropped-- it is nil.
// therefore, returning here is perfectly acceptable as it either succeeds (or it doesn't)
if mkDirErr == nil {
// To run our folder overwrite logic, we have to know if this current job created the folder.
// As per the comments above, we are technically wrong here in a write-only scenario (maybe it already
// existed and our Stat failed). But using overwrite=false on a write-only destination doesn't make
// a lot of sense anyway. Yes, we'll make the wrong decision here in a write-only scenario, but we'll
// make the _same_ wrong overwrite decision for all the files too (not just folders). So this is, at least,
// consistent.
tracker.RecordCreation(directory)
return nil
}
return err

// another routine might have created the directory at the same time
// check whether the directory now exists
if _, err := OSStat(directory); err != nil {
// no other routine succeeded
// return the original error we got from Mkdir
return mkDirErr
}

return nil
} else { // if err is nil, we return err. if err has an error, we return it.
return nil
}
Expand Down

0 comments on commit 43e7102

Please sign in to comment.