Skip to content

Commit 10288ad

Browse files
[Storage][Azdatalake][BugFix] Fix panic on auth failure (Azure#24811)
* fix panic on authentication failure * fix panic on authentication failure * Update sdk/storage/azdatalake/directory/client.go Co-authored-by: Copilot <[email protected]> --------- Co-authored-by: Copilot <[email protected]>
1 parent 148126d commit 10288ad

File tree

5 files changed

+67
-3
lines changed

5 files changed

+67
-3
lines changed

sdk/storage/azdatalake/CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
### Breaking Changes
88

99
### Bugs Fixed
10+
* Fix panic in File and Directory client DownloadStream and Get Properties on authentication failure.
1011

1112
### Other Changes
1213

sdk/storage/azdatalake/directory/client.go

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -279,9 +279,11 @@ func (d *Client) GetProperties(ctx context.Context, options *GetPropertiesOption
279279
var respFromCtx *http.Response
280280
ctxWithResp := shared.WithCaptureBlobResponse(ctx, &respFromCtx)
281281
resp, err := d.blobClient().GetProperties(ctxWithResp, opts)
282+
if err != nil {
283+
return GetPropertiesResponse{}, exported.ConvertToDFSError(err)
284+
}
282285
newResp := path.FormatGetPropertiesResponse(&resp, respFromCtx)
283-
err = exported.ConvertToDFSError(err)
284-
return newResp, err
286+
return newResp, nil
285287
}
286288

287289
// Rename renames a directory. The original directory will no longer exist and the client will be stale.

sdk/storage/azdatalake/directory/client_test.go

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,9 @@ package directory_test
88

99
import (
1010
"context"
11+
"github.com/Azure/azure-sdk-for-go/sdk/azidentity"
1112
"github.com/Azure/azure-sdk-for-go/sdk/storage/azdatalake"
13+
"github.com/Azure/azure-sdk-for-go/sdk/storage/azdatalake/service"
1214
"net/http"
1315
"strconv"
1416
"testing"
@@ -2979,3 +2981,30 @@ func (s *RecordedTestSuite) TestCreateDirectoryClientCustomAudience() {
29792981
_require.NoError(err)
29802982

29812983
}
2984+
2985+
func (s *UnrecordedTestSuite) TestDirectoryClientOnAuthenticationFailure() {
2986+
_require := require.New(s.T())
2987+
testName := s.T().Name()
2988+
2989+
tenantID := "xxx"
2990+
clientID := "xxx"
2991+
clientSecret := "xxx"
2992+
2993+
cred, err := azidentity.NewClientSecretCredential(tenantID, clientID, clientSecret, nil)
2994+
_require.NoError(err)
2995+
2996+
accountName, _ := testcommon.GetGenericAccountInfo(testcommon.TestAccountDatalake)
2997+
url := "https://" + accountName + ".dfs.core.windows.net/"
2998+
2999+
srvClient, err := service.NewClient(url, cred, nil)
3000+
_require.NoError(err)
3001+
3002+
fsName := testcommon.GenerateFileSystemName(testName)
3003+
fsClient := srvClient.NewFileSystemClient(fsName)
3004+
dirClient := fsClient.NewDirectoryClient("testdir")
3005+
3006+
// This should return an error, not panic
3007+
_, err = dirClient.GetProperties(context.Background(), nil)
3008+
_require.Error(err, "Expected authentication error")
3009+
_require.Contains(err.Error(), "ClientSecretCredential")
3010+
}

sdk/storage/azdatalake/file/client.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -558,6 +558,9 @@ func (f *Client) DownloadStream(ctx context.Context, o *DownloadStreamOptions) (
558558
var respFromCtx *http.Response
559559
ctxWithResp := shared.WithCaptureBlobResponse(ctx, &respFromCtx)
560560
resp, err := f.blobClient().DownloadStream(ctxWithResp, opts)
561+
if err != nil {
562+
return DownloadStreamResponse{}, exported.ConvertToDFSError(err)
563+
}
561564
newResp := FormatDownloadStreamResponse(&resp, respFromCtx)
562565
fullResp := DownloadStreamResponse{
563566
client: f,
@@ -567,7 +570,7 @@ func (f *Client) DownloadStream(ctx context.Context, o *DownloadStreamOptions) (
567570
cpkScope: o.CPKScopeInfo,
568571
}
569572

570-
return fullResp, exported.ConvertToDFSError(err)
573+
return fullResp, nil
571574
}
572575

573576
// DownloadBuffer downloads an Azure file to a buffer with parallel.

sdk/storage/azdatalake/file/client_test.go

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import (
1414
"encoding/binary"
1515
"fmt"
1616
"github.com/Azure/azure-sdk-for-go/sdk/azcore/log"
17+
"github.com/Azure/azure-sdk-for-go/sdk/azidentity"
1718
"github.com/Azure/azure-sdk-for-go/sdk/storage/azblob/lease"
1819
"github.com/Azure/azure-sdk-for-go/sdk/storage/azdatalake/internal/path"
1920
"hash/crc64"
@@ -5683,3 +5684,31 @@ func (s *UnrecordedTestSuite) TestGetPropertiesWithInvalidSAS() {
56835684
_, err = fClientWithInvalidSAS.GetProperties(context.Background(), nil)
56845685
_require.Error(err)
56855686
}
5687+
5688+
// TestFileClientAuthenticationFailure tests that GetProperties and DownloadStream handle authentication failures gracefully
5689+
func (s *UnrecordedTestSuite) TestFileClientAuthenticationFailure() {
5690+
_require := require.New(s.T())
5691+
tenantID := "invalid-tenant-id"
5692+
clientID := "invalid-client-id"
5693+
clientSecret := "invalid-secret"
5694+
5695+
cred, err := azidentity.NewClientSecretCredential(tenantID, clientID, clientSecret, nil)
5696+
_require.NoError(err)
5697+
accountName, _ := testcommon.GetGenericAccountInfo(testcommon.TestAccountDatalake)
5698+
url := "https://" + accountName + ".dfs.core.windows.net/"
5699+
5700+
srvClient, err := service.NewClient(url, cred, nil)
5701+
_require.NoError(err)
5702+
5703+
fsClient := srvClient.NewFileSystemClient("testfs")
5704+
fileClient := fsClient.NewFileClient("testfile")
5705+
5706+
_, err = fileClient.GetProperties(context.Background(), nil)
5707+
_require.Error(err, "Expected authentication error")
5708+
_require.Contains(err.Error(), "ClientSecretCredential")
5709+
5710+
// Test DownloadStream - should return an error, not panic
5711+
_, err = fileClient.DownloadStream(context.Background(), nil)
5712+
_require.Error(err, "Expected authentication error")
5713+
_require.Contains(err.Error(), "ClientSecretCredential")
5714+
}

0 commit comments

Comments
 (0)