Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NewSharedKeyCredential Panics with invalid key. Should return error #84

Closed
lawrencegripper opened this issue Oct 23, 2018 · 4 comments
Closed

Comments

@lawrencegripper
Copy link

Which version of the SDK was used?

  name = "github.com/Azure/azure-storage-blob-go"
  packages = ["2016-05-31/azblob"]
  pruneopts = "UT"
  revision = "bb46532f68b79e9e1baca8fb19a382ef5d40ed33"
  version = "0.2.0"

Which platform are you using? (ex: Windows, Linux, Debian)

Linux (Ubuntu 18) Go 1.10.1

What problem was encountered?

The following call panics if an invalid key is presented to it. This is not a nice way to handle things. The method should return creds, err with the error containing some detail on why it failed, eg: "invalid key, must be valid base64 data"

	creds := azblob.NewSharedKeyCredential(accountName, storageAccountKey)

=== RUN   TestLockingEnd2End_WrongStorageKey
--- FAIL: TestLockingEnd2End_WrongStorageKey (0.00s)
panic: illegal base64 data at input byte 4 [recovered]
	panic: illegal base64 data at input byte 4

goroutine 20 [running]:
testing.tRunner.func1(0xc4201441e0)
	/usr/lib/go-1.10/src/testing/testing.go:742 +0x29d
panic(0x6b2a40, 0xc4200acb08)
	/usr/lib/go-1.10/src/runtime/panic.go:502 +0x229
github.com/lawrencegripper/goazurelocking/vendor/github.com/Azure/azure-storage-blob-go/2016-05-31/azblob.NewSharedKeyCredential(0xc42015e11b, 0xf, 0x7175b4, 0x7, 0x0)
	/home/lawrence/go/src/github.com/lawrencegripper/goazurelocking/vendor/github.com/Azure/azure-storage-blob-go/2016-05-31/azblob/credential_shared_key.go:23 +0x11c
github.com/lawrencegripper/goazurelocking.NewLockInstance(0x753760, 0xc4200b2bc0, 0xc42015e113, 0x2d, 0x7175b4, 0x7, 0xc4200ac870, 0xa, 0x37e11d600, 0xc42009ee10, ...)
	/home/lawrence/go/src/github.com/lawrencegripper/goazurelocking/locking.go:194 +0x215
github.com/lawrencegripper/goazurelocking.TestLockingEnd2End_WrongStorageKey(0xc4201441e0)
	/home/lawrence/go/src/github.com/lawrencegripper/goazurelocking/locking_integration_test.go:385 +0x1d1
testing.tRunner(0xc4201441e0, 0x72ca38)
	/usr/lib/go-1.10/src/testing/testing.go:777 +0xd0
created by testing.(*T).Run
	/usr/lib/go-1.10/src/testing/testing.go:824 +0x2e0
FAIL	github.com/lawrencegripper/goazurelocking	0.007s
Error: Tests failed.

How can we reproduce the problem in the simplest way?

Pass a key of an account key of "somekey" to the NewSharedKeyCredential

Have you found a mitigation/solution?

Not yet but I plan to validate the key is valid Base64 before making the call.

@zezha-msft
Copy link
Contributor

zezha-msft commented Oct 23, 2018

Hi @lawrencegripper, this is already fixed in the 2018-03-28 version.

I would strongly suggest to upgrade to the latest version (0.3.0) instead, where panics are mostly removed from this SDK.

@lawrencegripper
Copy link
Author

@zezha-msft Thanks for the quick reply, is there a plan to re-gen the other Service API's using the new code? I'd like to target the 2016 API version of Azure Storage ideally.

@zezha-msft
Copy link
Contributor

Hi @lawrencegripper, is there a particular reason why you'd like to target the 2016 version?

Currently there is no plan to keep maintaining the 2016 version, as it is a lot of effort to maintain the hand-written code simultaneously.

@lawrencegripper
Copy link
Author

The locking abstraction I'm writing is aimed at being usable across many project so I was aiming for the lowest common denominator to allow people to target Azure Stack's available API versions.

That being said, as the amount of work is significant, I've put in a simple check in my code around the key value here which lets me stick to 2016 API

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants