Merge pull request #38 from Azure/dev

Release v0.8.0

ChangeLog.md

@@ -2,6 +2,17 @@
 
 > See [BreakingChanges](BreakingChanges.md) for a detailed list of API breaks.
 
+## Version 0.8.0:
+- Allow more time formats for SAS
+- Enable recovering from an unexpectedEOF error
+
+## Version 0.7.0:
+- Add support for permissions
+- Add support for SMB properties
+
+## Version 0.6.0:
+- Added support for UploadRangeFromURL
+
 ## Version 0.5.0:
 - Align jitter calculations exactly to blob SDK
 - General secondary host improvements

azfile/version.go

@@ -1,3 +1,3 @@
 package azfile
 
-const serviceLibVersion = "0.7.0"
+const serviceLibVersion = "0.8.0"

azfile/zc_policy_retry.go

@@ -230,6 +230,11 @@ func NewRetryPolicyFactory(o RetryOptions) pipeline.Factory {
 						} else {
 							action = "NoRetry: net.Error and in the non-retriable list"
 						}
+					} else if err == io.ErrUnexpectedEOF {
+						// Some of our methods under the zz_ files do use io.Copy and other related methods that can throw an unexpectedEOF.
+						// However, we don't actually return those errors when we get them.
+						// Consider this more of a safety net.
+						action = "Retry: io.UnexpectedEOF"
 					} else {
 						action = "NoRetry: unrecognized error"
 					}

azfile/zc_retry_reader.go

@@ -39,6 +39,7 @@ type RetryReaderOptions struct {
 	MaxRetryRequests   int
 	doInjectError      bool
 	doInjectErrorRound int
+	injectedError error
 
 	// NotifyFailedRead is called, if non-nil, after any failure to read. Expected usage is diagnostic logging.
 	NotifyFailedRead FailedReadNotifier
@@ -124,7 +125,11 @@ func (s *retryReader) Read(p []byte) (n int, err error) {
 
 		// Injection mechanism for testing.
 		if s.o.doInjectError && try == s.o.doInjectErrorRound {
-			err = &net.DNSError{IsTemporary: true}
+			if s.o.injectedError != nil {
+				err = s.o.injectedError
+			} else {
+				err = &net.DNSError{IsTemporary: true}
+			}
 		}
 
 		// We successfully read data or end EOF.
@@ -141,7 +146,8 @@ func (s *retryReader) Read(p []byte) (n int, err error) {
 		// Check the retry count and error code, and decide whether to retry.
 		retriesExhausted := try >= s.o.MaxRetryRequests
 		_, isNetError := err.(net.Error)
-		willRetry := (isNetError || s.wasRetryableEarlyClose(err)) && !retriesExhausted
+		isUnexpectedEOF := err == io.ErrUnexpectedEOF
+		willRetry := (isNetError || isUnexpectedEOF || s.wasRetryableEarlyClose(err)) && !retriesExhausted
 
 		// Notify, for logging purposes, of any failures
 		if s.o.NotifyFailedRead != nil {

azfile/zc_sas_query_params.go

@@ -35,20 +35,21 @@ func FormatTimesForSASSigning(startTime, expiryTime time.Time) (string, string)
 	return ss, se
 }
 
+const SASTimeFormat = "2006-01-02T15:04:05Z"
 // SASTimeFormats represents the format of a SAS start or expiry time. Use it when formatting/parsing a time.Time.
-var SASTimeFormats = []string{"2006-01-02T15:04:05Z", "2006-01-02T15:04Z", "2006-01-02"} // ISO 8601 formats, please refer to https://docs.microsoft.com/en-us/rest/api/storageservices/constructing-a-service-sas for more details.
+var SASTimeFormats = []string{"2006-01-02T15:04:05.0000000Z", SASTimeFormat, "2006-01-02T15:04Z", "2006-01-02"} // ISO 8601 formats, please refer to https://docs.microsoft.com/en-us/rest/api/storageservices/constructing-a-service-sas for more details.
 
 // formatSASTimeWithDefaultFormat format time with ISO 8601 in "yyyy-MM-ddTHH:mm:ssZ".
 func formatSASTimeWithDefaultFormat(t *time.Time) string {
-	return formatSASTime(t, SASTimeFormats[0]) // By default, "yyyy-MM-ddTHH:mm:ssZ" is used
+	return formatSASTime(t, SASTimeFormat) // By default, "yyyy-MM-ddTHH:mm:ssZ" is used
 }
 
 // formatSASTime format time with given format, use ISO 8601 in "yyyy-MM-ddTHH:mm:ssZ" by default.
 func formatSASTime(t *time.Time, format string) string {
 	if format != "" {
 		return t.Format(format)
 	}
-	return t.Format(SASTimeFormats[0]) // By default, "yyyy-MM-ddTHH:mm:ssZ" is used
+	return t.Format(SASTimeFormat) // By default, "yyyy-MM-ddTHH:mm:ssZ" is used
 }
 
 // parseSASTimeString try to parse sas time string.

azfile/zt_hiddenfuncs_test.go

@@ -0,0 +1,15 @@
+package azfile
+
+// This file isn't normally compiled when not testing.
+// Therefore, since we're in the azfile package, we can export a method to help us construct a failing retry reader options.
+
+func InjectErrorInRetryReaderOptions(err error) RetryReaderOptions {
+	return RetryReaderOptions{
+		MaxRetryRequests:       1,
+		doInjectError:          true,
+		doInjectErrorRound:     0,
+		injectedError:          err,
+		NotifyFailedRead:       nil,
+		TreatEarlyCloseAsError: false,
+	}
+}

azfile/zt_parsing_url_test.go

@@ -292,3 +292,56 @@ func (s *ParsingURLSuite) TestFileURLPartsStSe(c *chk.C) {
 	uResult = parts.URL()
 	c.Assert(uResult.String(), chk.Equals, "https://myaccount.file.core.windows.net/myshare/mydirectory/ReadMe.txt?sharesnapshot=2018-03-08T02:29:11.0000000Z&se=2222-03-09&si=myIdentifier&sig=92836758923659283652983562%3D%3D&sip=168.1.5.60-168.1.5.70&sp=rw&spr=https%2Chttp&sr=b&srt=s&ss=bf&st=2111-01-09T01%3A42Z&sv=2015-02-21")
 }
+
+func (s *ParsingURLSuite) TestFileURLPartsSASQueryTimes(c *chk.C) {
+	StartTimesInputs := []string{
+		"2020-04-20",
+		"2020-04-20T07:00Z",
+		"2020-04-20T07:15:00Z",
+		"2020-04-20T07:30:00.1234567Z",
+	}
+	StartTimesExpected := []time.Time{
+		time.Date(2020, time.April, 20, 0, 0, 0, 0, time.UTC),
+		time.Date(2020, time.April, 20, 7, 0, 0, 0, time.UTC),
+		time.Date(2020, time.April, 20, 7, 15, 0, 0, time.UTC),
+		time.Date(2020, time.April, 20, 7, 30, 0, 123456700, time.UTC),
+	}
+	ExpiryTimesInputs := []string{
+		"2020-04-21",
+		"2020-04-20T08:00Z",
+		"2020-04-20T08:15:00Z",
+		"2020-04-20T08:30:00.2345678Z",
+	}
+	ExpiryTimesExpected := []time.Time{
+		time.Date(2020, time.April, 21, 0, 0, 0, 0, time.UTC),
+		time.Date(2020, time.April, 20, 8, 0, 0, 0, time.UTC),
+		time.Date(2020, time.April, 20, 8, 15, 0, 0, time.UTC),
+		time.Date(2020, time.April, 20, 8, 30, 0, 234567800, time.UTC),
+	}
+
+	for i := 0; i < len(StartTimesInputs); i++ {
+		urlString :=
+			"https://myaccount.dfs.core.windows.net/myshare/mydirectory/myfile.txt?" +
+				"se=" + url.QueryEscape(ExpiryTimesInputs[i]) + "&" +
+				"sig=NotASignature&" +
+				"sp=r&" +
+				"spr=https&" +
+				"sr=b&" +
+				"st=" + url.QueryEscape(StartTimesInputs[i]) + "&" +
+				"sv=2019-10-10"
+		url, _ := url.Parse(urlString)
+
+		parts := azfile.NewFileURLParts(*url)
+		c.Assert(parts.Scheme, chk.Equals, "https")
+		c.Assert(parts.Host, chk.Equals, "myaccount.dfs.core.windows.net")
+		c.Assert(parts.ShareName, chk.Equals, "myshare")
+		c.Assert(parts.DirectoryOrFilePath, chk.Equals, "mydirectory/myfile.txt")
+
+		sas := parts.SAS
+		c.Assert(sas.StartTime(), chk.Equals, StartTimesExpected[i])
+		c.Assert(sas.ExpiryTime(), chk.Equals, ExpiryTimesExpected[i])
+
+		uResult := parts.URL()
+		c.Assert(uResult.String(), chk.Equals, urlString)
+	}
+}

azfile/zt_retryreader_test.go

@@ -157,6 +157,76 @@ func (r *retryReaderSuite) TestRetryReaderReadWithRetry(c *chk.C) {
 	}
 }
 
+// Test normal retry succeed, note initial response not provided.
+// Tests both with and without notification of failures
+func (r *retryReaderSuite) TestRetryReaderReadWithRetryIoUnexpectedEOF(c *chk.C) {
+	// Test twice, the second time using the optional "logging"/notification callback for failed tries
+	// We must test both with and without the callback, since be testing without
+	// we are testing that it is, indeed, optional to provide the callback
+	for _, logThisRun := range []bool{false, true} {
+
+		// Extra setup for testing notification of failures (i.e. of unsuccessful tries)
+		failureMethodNumCalls := 0
+		failureWillRetryCount := 0
+		failureLastReportedFailureCount := -1
+		var failureLastReportedError error = nil
+		failureMethod := func(failureCount int, lastError error, offset int64, count int64, willRetry bool) {
+			failureMethodNumCalls++
+			if willRetry {
+				failureWillRetryCount++
+			}
+			failureLastReportedFailureCount = failureCount
+			failureLastReportedError = lastError
+		}
+
+		// Main test setup
+		byteCount := 1
+		body := newPerByteReader(byteCount)
+		body.doInjectError = true
+		body.doInjectErrorByteIndex = 0
+		body.doInjectTimes = 1
+		body.injectedError = io.ErrUnexpectedEOF
+
+		getter := func(ctx context.Context, info HTTPGetterInfo) (*http.Response, error) {
+			r := http.Response{}
+			body.currentByteIndex = int(info.Offset)
+			r.Body = body
+
+			return &r, nil
+		}
+
+		httpGetterInfo := HTTPGetterInfo{Offset: 0, Count: int64(byteCount)}
+		initResponse, err := getter(ctx, httpGetterInfo)
+		c.Assert(err, chk.IsNil)
+
+		rrOptions := RetryReaderOptions{MaxRetryRequests: 1}
+		if logThisRun {
+			rrOptions.NotifyFailedRead = failureMethod
+		}
+		retryReader := NewRetryReader(ctx, initResponse, httpGetterInfo, rrOptions, getter)
+
+		// should fail and succeed through retry
+		can := make([]byte, 1)
+		n, err := retryReader.Read(can)
+		c.Assert(n, chk.Equals, 1)
+		c.Assert(err, chk.IsNil)
+
+		// check "logging", if it was enabled
+		if logThisRun {
+			// We only expect one failed try in this test
+			// And the notification method is not called for successes
+			c.Assert(failureMethodNumCalls, chk.Equals, 1)           // this is the number of calls we counted
+			c.Assert(failureWillRetryCount, chk.Equals, 1)           // the sole failure was retried
+			c.Assert(failureLastReportedFailureCount, chk.Equals, 1) // this is the number of failures reported by the notification method
+			c.Assert(failureLastReportedError, chk.NotNil)
+		}
+		// should return EOF
+		n, err = retryReader.Read(can)
+		c.Assert(n, chk.Equals, 0)
+		c.Assert(err, chk.Equals, io.EOF)
+	}
+}
+
 // Test normal retry fail as retry Count not enough.
 func (r *retryReaderSuite) TestRetryReaderReadNegativeNormalFail(c *chk.C) {
 	// Extra setup for testing notification of failures (i.e. of unsuccessful tries)

azfile/zt_url_file_test.go

@@ -4,7 +4,9 @@ import (
 	"bytes"
 	"context"
 	"crypto/md5"
+	"errors"
 	"fmt"
+	"io"
 	"io/ioutil"
 	"net/http"
 	"net/url"
@@ -1505,6 +1507,38 @@ func (s *FileURLSuite) TestFileGetRangeListSnapshot(c *chk.C) {
 	validateBasicGetRangeList(c, resp2, err)
 }
 
+func (s *FileURLSuite) TestUnexpectedEOFRecovery(c *chk.C) {
+	fsu := getFSU()
+	share, _ := createNewShare(c, fsu)
+	defer delShare(c, share, azfile.DeleteSnapshotsOptionInclude)
+
+	fileURL, _ := createNewFileFromShare(c, share, 2048)
+
+	contentR, contentD := getRandomDataAndReader(2048)
+
+	resp, err := fileURL.UploadRange(ctx, 0, contentR, nil)
+	c.Assert(err, chk.IsNil)
+	c.Assert(resp.StatusCode(), chk.Equals, http.StatusCreated)
+	c.Assert(resp.RequestID(), chk.Not(chk.Equals), "")
+
+	dlResp, err := fileURL.Download(ctx, 0, 2048, false)
+	c.Assert(err, chk.IsNil)
+
+	// Verify that we can inject errors first.
+	reader := dlResp.Body(azfile.InjectErrorInRetryReaderOptions(errors.New("unrecoverable error")))
+
+	_, err = ioutil.ReadAll(reader)
+	c.Assert(err, chk.NotNil)
+	c.Assert(err.Error(), chk.Equals, "unrecoverable error")
+
+	// Then inject the retryable error.
+	reader = dlResp.Body(azfile.InjectErrorInRetryReaderOptions(io.ErrUnexpectedEOF))
+
+	buf, err := ioutil.ReadAll(reader)
+	c.Assert(err, chk.IsNil)
+	c.Assert(buf, chk.DeepEquals, contentD)
+}
+
 // Don't check offset by design.
 // func (s *FileURLSuite) TestFileGetRangeListNegativeInvalidOffset(c *chk.C) {
 // 	fsu := getFSU()