Go Windows Azure Client Library

Merge lp:~wallyworld/gwacl/storage-error-not-panic into lp:gwacl

  1. storage-error-not-panic
  2. Merge into trunk
Proposed by Ian Booth
Status: Merged
Approved by: Ian Booth
Approved revision: 220
Merged at revision: 224
Proposed branch: lp:~wallyworld/gwacl/storage-error-not-panic
Merge into: lp:gwacl
Diff against target: 221 lines (+53/-27)
4 files modified
shared_signature.go (+13/-7)
shared_signature_test.go (+6/-3)
storage_base.go (+25/-13)
storage_base_test.go (+9/-4)
To merge this branch: bzr merge lp:~wallyworld/gwacl/storage-error-not-panic
Reviewer Review Type Date Requested Status
Julian Edwards (community) Approve
Review via email: mp+184704@code.launchpad.net

Commit message

GetAnonymousFileURL() was panicing if there was an issue. I changed it to return an error. This is because juju sometimes needs to try and find a URL of a file but it doesn't know if it exists or not. So by using error and not panic, juju can handle the situation accordingly.

Description of the change

GetAnonymousFileURL() was panicing if there was an issue. I changed it to return an error. This is because juju sometimes needs to try and find a URL of a file but it doesn't know if it exists or not. So by using error and not panic, juju can handle the situation accordingly.

To post a comment you must log in.
Revision history for this message
Julian Edwards (julian-edwards) wrote :

make format required.

Otherwise looks good.

review: Approve
Revision history for this message
Go Bot (go-bot) wrote :

The attempt to merge lp:~wallyworld/gwacl/storage-error-not-panic into lp:gwacl failed. Below is the output from the failed tests.

./utilities/format -s

retry_policy.go:7:5: cannot find package "launchpad.net/gwacl/fork/http" in any of:
 /usr/lib/go/src/pkg/launchpad.net/gwacl/fork/http (from $GOROOT)
 /home/tarmac/trees/gwacl-trees/src/launchpad.net/gwacl/fork/http (from $GOPATH)
x509session.go:9:5: cannot find package "launchpad.net/gwacl/fork/tls" in any of:
 /usr/lib/go/src/pkg/launchpad.net/gwacl/fork/tls (from $GOROOT)
 /home/tarmac/trees/gwacl-trees/src/launchpad.net/gwacl/fork/tls (from $GOPATH)
storage.go:14:5: cannot find package "launchpad.net/gwacl/logging" in any of:
 /usr/lib/go/src/pkg/launchpad.net/gwacl/logging (from $GOROOT)
 /home/tarmac/trees/gwacl-trees/src/launchpad.net/gwacl/logging (from $GOPATH)
example/management/run.go:15:5: cannot find package "launchpad.net/gwacl" in any of:
 /usr/lib/go/src/pkg/launchpad.net/gwacl (from $GOROOT)
 /home/tarmac/trees/gwacl-trees/src/launchpad.net/gwacl (from $GOPATH)

Revision history for this message
Go Bot (go-bot) wrote :

The attempt to merge lp:~wallyworld/gwacl/storage-error-not-panic into lp:gwacl failed. Below is the output from the failed tests.

./utilities/format -s
FAIL launchpad.net/gwacl [build failed]
FAIL launchpad.net/gwacl/dedent [build failed]
? launchpad.net/gwacl/example/management [no test files]
? launchpad.net/gwacl/example/storage [no test files]
? launchpad.net/gwacl/fork/http [no test files]
? launchpad.net/gwacl/fork/tls [no test files]
FAIL launchpad.net/gwacl/logging [build failed]

# launchpad.net/gwacl/dedent
../../../../trees/gwacl-trees/src/launchpad.net/gwacl/dedent/dedent_test.go:7: import /home/tarmac/trees/gwacl-trees/pkg/linux_amd64/launchpad.net/gocheck.a: object is [linux amd64 go1.1.1 X:none] expected [linux amd64 go1.1.2 X:none]
# launchpad.net/gwacl/logging
../../../../trees/gwacl-trees/src/launchpad.net/gwacl/logging/logging_test.go:7: import /home/tarmac/trees/gwacl-trees/pkg/linux_amd64/launchpad.net/gocheck.a: object is [linux amd64 go1.1.1 X:none] expected [linux amd64 go1.1.2 X:none]
# launchpad.net/gwacl
../../../../trees/gwacl-trees/src/launchpad.net/gwacl/deletedisk_test.go:8: import /home/tarmac/trees/gwacl-trees/pkg/linux_amd64/launchpad.net/gocheck.a: object is [linux amd64 go1.1.1 X:none] expected [linux amd64 go1.1.2 X:none]

Revision history for this message
Go Bot (go-bot) wrote :

The attempt to merge lp:~wallyworld/gwacl/storage-error-not-panic into lp:gwacl failed. Below is the output from the failed tests.

./utilities/format -s
FAIL launchpad.net/gwacl [build failed]
FAIL launchpad.net/gwacl/dedent [build failed]
? launchpad.net/gwacl/example/management [no test files]
? launchpad.net/gwacl/example/storage [no test files]
? launchpad.net/gwacl/fork/http [no test files]
? launchpad.net/gwacl/fork/tls [no test files]
FAIL launchpad.net/gwacl/logging [build failed]

# launchpad.net/gwacl/dedent
../../../../trees/gwacl-trees/src/launchpad.net/gwacl/dedent/dedent_test.go:7: import /home/tarmac/trees/gwacl-trees/pkg/linux_amd64/launchpad.net/gocheck.a: object is [linux amd64 go1.1.1 X:none] expected [linux amd64 go1.1.2 X:none]
# launchpad.net/gwacl/logging
../../../../trees/gwacl-trees/src/launchpad.net/gwacl/logging/logging_test.go:7: import /home/tarmac/trees/gwacl-trees/pkg/linux_amd64/launchpad.net/gocheck.a: object is [linux amd64 go1.1.1 X:none] expected [linux amd64 go1.1.2 X:none]
# launchpad.net/gwacl
../../../../trees/gwacl-trees/src/launchpad.net/gwacl/deletedisk_test.go:8: import /home/tarmac/trees/gwacl-trees/pkg/linux_amd64/launchpad.net/gocheck.a: object is [linux amd64 go1.1.1 X:none] expected [linux amd64 go1.1.2 X:none]

Revision history for this message
Go Bot (go-bot) wrote :

The attempt to merge lp:~wallyworld/gwacl/storage-error-not-panic into lp:gwacl failed. Below is the output from the failed tests.

./utilities/format -s
FAIL launchpad.net/gwacl [build failed]
FAIL launchpad.net/gwacl/dedent [build failed]
? launchpad.net/gwacl/example/management [no test files]
? launchpad.net/gwacl/example/storage [no test files]
? launchpad.net/gwacl/fork/http [no test files]
? launchpad.net/gwacl/fork/tls [no test files]
FAIL launchpad.net/gwacl/logging [build failed]

# launchpad.net/gwacl/dedent
../../../../trees/gwacl-trees/src/launchpad.net/gwacl/dedent/dedent_test.go:7: import /home/tarmac/trees/gwacl-trees/pkg/linux_amd64/launchpad.net/gocheck.a: object is [linux amd64 go1.1.1 X:none] expected [linux amd64 go1.1.2 X:none]
# launchpad.net/gwacl/logging
../../../../trees/gwacl-trees/src/launchpad.net/gwacl/logging/logging_test.go:7: import /home/tarmac/trees/gwacl-trees/pkg/linux_amd64/launchpad.net/gocheck.a: object is [linux amd64 go1.1.1 X:none] expected [linux amd64 go1.1.2 X:none]
# launchpad.net/gwacl
../../../../trees/gwacl-trees/src/launchpad.net/gwacl/deletedisk_test.go:8: import /home/tarmac/trees/gwacl-trees/pkg/linux_amd64/launchpad.net/gocheck.a: object is [linux amd64 go1.1.1 X:none] expected [linux amd64 go1.1.2 X:none]

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'shared_signature.go'
2--- shared_signature.go 2013-07-26 08:49:56 +0000
3+++ shared_signature.go 2013-09-10 01:12:02 +0000
4@@ -27,21 +27,27 @@
5 // composeSharedSignature composes the "Shared Access Signature" as described
6 // in the paragraph "Constructing the Signature String" in
7 // http://msdn.microsoft.com/en-us/library/windowsazure/dn140255.aspx
8-func (params *sharedSignatureParams) composeSharedSignature() string {
9+func (params *sharedSignatureParams) composeSharedSignature() (string, error) {
10 // Compose the string to sign.
11 canonicalizedResource := fmt.Sprintf("/%s%s", params.accountName, params.path)
12 stringToSign := fmt.Sprintf("%s\n%s\n%s\n%s\n%s\n%s",
13 params.permission, params.signedStart, params.signedExpiry, canonicalizedResource, params.signedIdentifier, params.signedVersion)
14 // Create the signature.
15- signature := sign(params.accountKey, stringToSign)
16- return signature
17+ signature, err := sign(params.accountKey, stringToSign)
18+ if err != nil {
19+ return "", err
20+ }
21+ return signature, nil
22 }
23
24 // composeAccessQueryValues returns the values that correspond to the query
25 // string used to build a Shared Access Signature URI as described in
26 // http://msdn.microsoft.com/en-us/library/windowsazure/dn140255.aspx
27-func (params *sharedSignatureParams) composeAccessQueryValues() url.Values {
28- signature := params.composeSharedSignature()
29+func (params *sharedSignatureParams) composeAccessQueryValues() (url.Values, error) {
30+ signature, err := params.composeSharedSignature()
31+ if err != nil {
32+ return nil, err
33+ }
34 // Compose the "Shared Access Signature" query string.
35 values := url.Values{}
36 values.Set("sv", params.signedVersion)
37@@ -49,13 +55,13 @@
38 values.Set("sr", params.signedRessource)
39 values.Set("sp", params.permission)
40 values.Set("sig", signature)
41- return values
42+ return values, nil
43 }
44
45 // getReadBlobAccessValues returns the values that correspond to the query
46 // string used to build a Shared Access Signature URI. The signature grants
47 // read access to the given blob.
48-func getReadBlobAccessValues(container, filename, accountName, accountKey string, expires time.Time) url.Values {
49+func getReadBlobAccessValues(container, filename, accountName, accountKey string, expires time.Time) (url.Values, error) {
50 expiryDateString := expires.UTC().Format(time.RFC3339)
51
52 path := fmt.Sprintf("/%s/%s", container, filename)
53
54=== modified file 'shared_signature_test.go'
55--- shared_signature_test.go 2013-07-26 08:49:56 +0000
56+++ shared_signature_test.go 2013-09-10 01:12:02 +0000
57@@ -25,7 +25,8 @@
58 accountKey: base64.StdEncoding.EncodeToString([]byte("key")),
59 }
60
61- signature := params.composeSharedSignature()
62+ signature, err := params.composeSharedSignature()
63+ c.Assert(err, IsNil)
64 c.Check(signature, Equals, "C/COJt8UagHJR2LBT1129bhDChtgfLGFqfZ0YQpBdF0=")
65 }
66
67@@ -41,7 +42,8 @@
68 accountKey: base64.StdEncoding.EncodeToString([]byte("key")),
69 }
70
71- values := params.composeAccessQueryValues()
72+ values, err := params.composeAccessQueryValues()
73+ c.Assert(err, IsNil)
74
75 c.Check(values.Get("sv"), Equals, params.signedVersion)
76 c.Check(values.Get("se"), Equals, params.signedExpiry)
77@@ -58,7 +60,8 @@
78 expires, err := time.Parse("Monday, 02-Jan-06 15:04:05 MST", time.RFC850)
79 c.Assert(err, IsNil)
80
81- values := getReadBlobAccessValues(container, filename, accountName, accountKey, expires)
82+ values, err := getReadBlobAccessValues(container, filename, accountName, accountKey, expires)
83+ c.Assert(err, IsNil)
84
85 c.Check(values.Get("sv"), Equals, "2012-02-12")
86 expiryDateString := values.Get("se")
87
88=== modified file 'storage_base.go'
89--- storage_base.go 2013-08-09 14:59:50 +0000
90+++ storage_base.go 2013-09-10 01:12:02 +0000
91@@ -43,29 +43,32 @@
92
93 // sign returns the base64-encoded HMAC-SHA256 signature of the given string
94 // using the given base64-encoded key.
95-func sign(accountKey, signable string) string {
96+func sign(accountKey, signable string) (string, error) {
97 // Allegedly, this is already UTF8 encoded.
98 decodedKey, err := base64.StdEncoding.DecodeString(accountKey)
99 if err != nil {
100- panic(fmt.Errorf("invalid account key: %s", err))
101+ return "", fmt.Errorf("invalid account key: %s", err)
102 }
103 hash := hmac.New(sha256.New, decodedKey)
104 _, err = hash.Write([]byte(signable))
105 if err != nil {
106- panic(fmt.Errorf("failed to write hash: %s", err))
107+ return "", fmt.Errorf("failed to write hash: %s", err)
108 }
109 var hashed []byte
110 hashed = hash.Sum(hashed)
111 b64Hashed := base64.StdEncoding.EncodeToString(hashed)
112- return b64Hashed
113+ return b64Hashed, nil
114 }
115
116 // Calculate the value required for an Authorization header.
117-func composeAuthHeader(req *http.Request, accountName, accountKey string) string {
118+func composeAuthHeader(req *http.Request, accountName, accountKey string) (string, error) {
119 signable := composeStringToSign(req, accountName)
120
121- b64Hashed := sign(accountKey, signable)
122- return fmt.Sprintf("SharedKey %s:%s", accountName, b64Hashed)
123+ b64Hashed, err := sign(accountKey, signable)
124+ if err != nil {
125+ return "", err
126+ }
127+ return fmt.Sprintf("SharedKey %s:%s", accountName, b64Hashed), nil
128 }
129
130 // Calculate the string that needs to be HMAC signed. It is comprised of
131@@ -190,12 +193,16 @@
132 // signRequest adds the Authorization: header to a Request.
133 // Don't make any further changes to the request before sending it, or the
134 // signature will not be valid.
135-func (context *StorageContext) signRequest(req *http.Request) {
136+func (context *StorageContext) signRequest(req *http.Request) error {
137 // Only sign the request if the key is not empty.
138 if context.Key != "" {
139- header := composeAuthHeader(req, context.Account, context.Key)
140+ header, err := composeAuthHeader(req, context.Account, context.Key)
141+ if err != nil {
142+ return err
143+ }
144 req.Header.Set("Authorization", header)
145 }
146+ return nil
147 }
148
149 // StorageContext keeps track of the mandatory parameters required to send a
150@@ -285,7 +292,9 @@
151 addVersionHeader(req, params.APIVersion)
152 addDateHeader(req)
153 addContentHeaders(req)
154- context.signRequest(req)
155+ if err := context.signRequest(req); err != nil {
156+ return nil, nil, err
157+ }
158 return context.send(req, params.Result, params.ExpectedStatus)
159 }
160
161@@ -353,10 +362,13 @@
162
163 // GetAnonymousFileURL returns an anonymously-accessible URL for a given file
164 // in the given container.
165-func (context *StorageContext) GetAnonymousFileURL(container, filename string, expires time.Time) string {
166+func (context *StorageContext) GetAnonymousFileURL(container, filename string, expires time.Time) (string, error) {
167 url := context.GetFileURL(container, filename)
168- values := getReadBlobAccessValues(container, filename, context.Account, context.Key, expires)
169- return fmt.Sprintf("%s?%s", url, values.Encode())
170+ values, err := getReadBlobAccessValues(container, filename, context.Account, context.Key, expires)
171+ if err != nil {
172+ return "", err
173+ }
174+ return fmt.Sprintf("%s?%s", url, values.Encode()), nil
175 }
176
177 type ListContainersRequest struct {
178
179=== modified file 'storage_base_test.go'
180--- storage_base_test.go 2013-08-09 14:59:50 +0000
181+++ storage_base_test.go 2013-09-10 01:12:02 +0000
182@@ -192,7 +192,8 @@
183 key := base64.StdEncoding.EncodeToString([]byte("dummykey"))
184 signable := "a-string-to-sign"
185
186- observed := sign(key, signable)
187+ observed, err := sign(key, signable)
188+ c.Assert(err, IsNil)
189 expected := "5j1DSsm07IEh3u9JQQd0KPwtM6pEGChzrAF7Zf/LxLc="
190 c.Assert(observed, Equals, expected)
191 }
192@@ -208,7 +209,8 @@
193
194 key := base64.StdEncoding.EncodeToString([]byte("dummykey"))
195
196- observed := composeAuthHeader(req, "myname", key)
197+ observed, err := composeAuthHeader(req, "myname", key)
198+ c.Assert(err, IsNil)
199 expected := "SharedKey myname:Xf9hWQ99mM0IyEOL6rNeAUdTQlixVqiYnt2TpLCCpY0="
200 c.Assert(observed, Equals, expected)
201 }
202@@ -390,7 +392,8 @@
203 }
204 expires := time.Now()
205
206- signedURL := context.GetAnonymousFileURL(container, file, expires)
207+ signedURL, err := context.GetAnonymousFileURL(container, file, expires)
208+ c.Assert(err, IsNil)
209 // The only difference with the non-anon URL is the query string.
210 parsed, err := url.Parse(signedURL)
211 c.Assert(err, IsNil)
212@@ -403,7 +406,9 @@
213 values, err := url.ParseQuery(parsed.RawQuery)
214 c.Assert(err, IsNil)
215 signature := values.Get("sig")
216- expectedSignature := getReadBlobAccessValues(container, file, account, key, expires).Get("sig")
217+ readValues, err := getReadBlobAccessValues(container, file, account, key, expires)
218+ c.Assert(err, IsNil)
219+ expectedSignature := readValues.Get("sig")
220 c.Check(signature, Equals, expectedSignature)
221 }
222

Subscribers

People subscribed via source and target branches

to all changes: