Skip to content

Commit 1c21fb4

Browse files
alzimmermsftalzimmermsft
authored
Improve Error Message when Key Isn't Properly Base64 Encoded (Azure#32726)
Improve Error Message when Key Isn't Properly Base64 Encoded
1 parent 610d3d8 commit 1c21fb4

File tree

3 files changed

+68
-71
lines changed

3 files changed

+68
-71
lines changed

‎sdk/storage/azure-storage-common/src/main/java/com/azure/storage/common/StorageSharedKeyCredential.java‎

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@
1919
import java.net.URL;
2020
import java.text.Collator;
2121
import java.util.Arrays;
22-
import java.util.HashMap;
2322
import java.util.Locale;
2423
import java.util.Map;
2524
import java.util.Objects;
@@ -34,8 +33,8 @@ public final class StorageSharedKeyCredential {
3433
private static final Context LOG_STRING_TO_SIGN_CONTEXT = new Context(Constants.STORAGE_LOG_STRING_TO_SIGN, true);
3534

3635
// Pieces of the connection string that are needed.
37-
private static final String ACCOUNT_NAME = "accountname";
3836
private static final String ACCOUNT_KEY = "accountkey";
37+
private static final String ACCOUNT_NAME = "accountname";
3938

4039
private final AzureNamedKeyCredential azureNamedKeyCredential;
4140

@@ -73,14 +72,22 @@ private StorageSharedKeyCredential(AzureNamedKeyCredential azureNamedKeyCredenti
7372
* @throws IllegalArgumentException If {@code connectionString} doesn't have AccountName or AccountKey.
7473
*/
7574
public static StorageSharedKeyCredential fromConnectionString(String connectionString) {
76-
HashMap<String, String> connectionStringPieces = new HashMap<>();
75+
String accountName = null;
76+
String accountKey = null;
77+
7778
for (String connectionStringPiece : connectionString.split(";")) {
7879
String[] kvp = connectionStringPiece.split("=", 2);
79-
connectionStringPieces.put(kvp[0].toLowerCase(Locale.ROOT), kvp[1]);
80-
}
8180

82-
String accountName = connectionStringPieces.get(ACCOUNT_NAME);
83-
String accountKey = connectionStringPieces.get(ACCOUNT_KEY);
81+
if (kvp.length < 2) {
82+
continue;
83+
}
84+
85+
if (ACCOUNT_NAME.equalsIgnoreCase(kvp[0])) {
86+
accountName = kvp[1];
87+
} else if (ACCOUNT_KEY.equalsIgnoreCase(kvp[0])) {
88+
accountKey = kvp[1];
89+
}
90+
}
8491

8592
if (CoreUtils.isNullOrEmpty(accountName) || CoreUtils.isNullOrEmpty(accountKey)) {
8693
throw new IllegalArgumentException("Connection string must contain 'AccountName' and 'AccountKey'.");

‎sdk/storage/azure-storage-common/src/main/java/com/azure/storage/common/implementation/StorageImplUtils.java‎

Lines changed: 19 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -41,8 +41,6 @@ public class StorageImplUtils {
4141

4242
private static final String PARAMETER_NOT_IN_RANGE = "The value of the parameter '%s' should be between %s and %s.";
4343

44-
private static final String NO_PATH_SEGMENTS = "URL %s does not contain path segments.";
45-
4644
private static final String STRING_TO_SIGN_LOG_INFO_MESSAGE = "The string to sign computed by the SDK is: {}{}";
4745

4846
private static final String STRING_TO_SIGN_LOG_WARNING_MESSAGE = "Please remember to disable '{}' before going "
@@ -62,52 +60,27 @@ public class StorageImplUtils {
6260
Constants.STORAGE_LOG_STRING_TO_SIGN, Constants.STORAGE_LOG_STRING_TO_SIGN,
6361
Constants.STORAGE_LOG_STRING_TO_SIGN);
6462

65-
/**
66-
* @deprecated See value in {@link StorageImplUtils}
67-
*/
68-
@Deprecated
69-
public static final String INVALID_DATE_STRING = "Invalid Date String: %s.";
70-
71-
/**
72-
* Stores a reference to the date/time pattern with the greatest precision Java.util.Date is capable of expressing.
73-
* @deprecated See value in {@link StorageImplUtils}
74-
*/
75-
@Deprecated
76-
public static final String MAX_PRECISION_PATTERN = "yyyy-MM-dd'T'HH:mm:ss.SSS";
63+
private static final String INVALID_BASE64_KEY =
64+
"'base64Key' was not a valid Base64 scheme. Ensure the Storage account key or SAS key is properly formatted.";
7765

78-
/**
79-
* The length of a datestring that matches the MAX_PRECISION_PATTERN.
80-
* @deprecated See value in {@link StorageImplUtils}
81-
*/
82-
@Deprecated
83-
public static final int MAX_PRECISION_DATESTRING_LENGTH = MAX_PRECISION_PATTERN.replaceAll("'", "")
84-
.length();
66+
private static final String INVALID_DATE_STRING = "Invalid Date String: %s.";
8567

86-
/**
87-
* Stores a reference to the ISO8601 date/time pattern.
88-
* @deprecated See value in {@link StorageImplUtils}
89-
*/
90-
@Deprecated
91-
public static final String ISO8601_PATTERN = "yyyy-MM-dd'T'HH:mm:ss'Z'";
68+
private static final String MAX_PRECISION_PATTERN = "yyyy-MM-dd'T'HH:mm:ss.SSS";
9269

93-
/**
94-
* Stores a reference to the ISO8601 date/time pattern.
95-
* @deprecated See value in {@link StorageImplUtils}
96-
*/
97-
@Deprecated
98-
public static final String ISO8601_PATTERN_NO_SECONDS = "yyyy-MM-dd'T'HH:mm'Z'";
70+
private static final int MAX_PRECISION_DATESTRING_LENGTH = MAX_PRECISION_PATTERN.replaceAll("'", "").length();
9971

10072
// Use constant DateTimeFormatters as 'ofPattern' requires the passed pattern to be parsed each time, significantly
10173
// increasing the overhead of using DateTimeFormatter.
102-
private static final DateTimeFormatter MAX_PRECISION_FORMATTER = DateTimeFormatter.ofPattern(MAX_PRECISION_PATTERN)
103-
.withLocale(Locale.ROOT);
74+
private static final DateTimeFormatter MAX_PRECISION_FORMATTER =
75+
DateTimeFormatter.ofPattern("yyyy-MM-dd'T'HH:mm:ss.SSS")
76+
.withLocale(Locale.ROOT);
10477

105-
private static final DateTimeFormatter ISO8601_FORMATTER = DateTimeFormatter.ofPattern(ISO8601_PATTERN)
78+
private static final DateTimeFormatter ISO8601_FORMATTER = DateTimeFormatter.ofPattern("yyyy-MM-dd'T'HH:mm:ss'Z'")
10679
.withLocale(Locale.ROOT);
10780

108-
private static final DateTimeFormatter NO_SECONDS_FORMATTER = DateTimeFormatter
109-
.ofPattern(ISO8601_PATTERN_NO_SECONDS)
110-
.withLocale(Locale.ROOT);
81+
private static final DateTimeFormatter NO_SECONDS_FORMATTER =
82+
DateTimeFormatter.ofPattern("yyyy-MM-dd'T'HH:mm'Z'")
83+
.withLocale(Locale.ROOT);
11184

11285
/**
11386
* Parses the query string into a key-value pair map that maintains key, query parameter key, order. The value is
@@ -213,8 +186,14 @@ public static void assertInBounds(final String param, final long value, final lo
213186
* string, or the UTF-8 charset isn't supported.
214187
*/
215188
public static String computeHMac256(final String base64Key, final String stringToSign) {
189+
byte[] key;
190+
try {
191+
key = Base64.getDecoder().decode(base64Key);
192+
} catch (IllegalArgumentException ex) {
193+
throw new RuntimeException(INVALID_BASE64_KEY, ex);
194+
}
195+
216196
try {
217-
byte[] key = Base64.getDecoder().decode(base64Key);
218197
Mac hmacSHA256 = Mac.getInstance("HmacSHA256");
219198
hmacSHA256.init(new SecretKeySpec(key, "HmacSHA256"));
220199
byte[] utf8Bytes = stringToSign.getBytes(StandardCharsets.UTF_8);
@@ -250,30 +229,6 @@ public static URL appendToUrlPath(String baseURL, String name) {
250229
}
251230
}
252231

253-
254-
/**
255-
* Strips the last path segment from the passed URL.
256-
*
257-
* @param baseUrl URL having its last path segment stripped
258-
* @return a URL with the path segment stripped.
259-
* @throws IllegalArgumentException If stripping the last path segment causes the URL to become malformed or it
260-
* doesn't contain any path segments.
261-
*/
262-
public static URL stripLastPathSegment(URL baseUrl) {
263-
UrlBuilder builder = UrlBuilder.parse(baseUrl);
264-
265-
if (builder.getPath() == null || !builder.getPath().contains("/")) {
266-
throw new IllegalArgumentException(String.format(Locale.ROOT, NO_PATH_SEGMENTS, baseUrl));
267-
}
268-
269-
builder.setPath(builder.getPath().substring(0, builder.getPath().lastIndexOf("/")));
270-
try {
271-
return builder.toUrl();
272-
} catch (MalformedURLException ex) {
273-
throw new IllegalArgumentException(ex);
274-
}
275-
}
276-
277232
/**
278233
* Strips the account name from host part of the URL object.
279234
*

‎sdk/storage/azure-storage-common/src/test/java/com/azure/storage/common/StorageSharedKeyCredentialTest.groovy‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,4 +47,39 @@ class StorageSharedKeyCredentialTest extends Specification {
4747
signature1 != signature2
4848
signature2 == expectedSignature
4949
}
50+
51+
def "Can parse valid connection string"() {
52+
when:
53+
def storageSharedKeyCredential = StorageSharedKeyCredential.fromConnectionString(connectionString)
54+
55+
then:
56+
storageSharedKeyCredential.getAccountName() == "teststorage"
57+
58+
where:
59+
connectionString || _
60+
"DefaultEndpointsProtocol=https;AccountName=teststorage;AccountKey=atestaccountkey;EndpointSuffix=core.windows.net" || _
61+
"DefaultEndpointsProtocol=https;accountName=teststorage;AccountKey=atestaccountkey;EndpointSuffix=core.windows.net" || _
62+
"DefaultEndpointsProtocol=https;AccountName=teststorage;accountKey=atestaccountkey;EndpointSuffix=core.windows.net" || _
63+
"DefaultEndpointsProtocol=https;Accountname=teststorage;accountKey=atestaccountkey;EndpointSuffix=core.windows.net" || _
64+
"DefaultEndpointsProtocol=https;AccountName=teststorage;accountkey=atestaccountkey;EndpointSuffix=core.windows.net" || _
65+
"DefaultEndpointsProtocol=https;accountname=teststorage;accountkey=atestaccountkey;EndpointSuffix=core.windows.net" || _
66+
}
67+
68+
def "Cannot parse invalid connection string"() {
69+
when:
70+
StorageSharedKeyCredential.fromConnectionString(connectionString)
71+
72+
then:
73+
thrown(IllegalArgumentException)
74+
75+
where:
76+
connectionString || _
77+
"DefaultEndpointsProtocol=https;EndpointSuffix=core.windows.net" || _
78+
"DefaultEndpointsProtocol=https;AccountKey=atestaccountkey;EndpointSuffix=core.windows.net" || _
79+
"DefaultEndpointsProtocol=https;AccountName=teststorage;EndpointSuffix=core.windows.net" || _
80+
"DefaultEndpointsProtocol=https;AccountName =teststorage;AccountKey=atestaccountkey;EndpointSuffix=core.windows.net" || _
81+
"DefaultEndpointsProtocol=https;AccountName=teststorage;AccountKey =atestaccountkey;EndpointSuffix=core.windows.net" || _
82+
"DefaultEndpointsProtocol=https;Account Name=teststorage;AccountKey=atestaccountkey;EndpointSuffix=core.windows.net" || _
83+
"DefaultEndpointsProtocol=https;AccountName=teststorage;Account Key=atestaccountkey;EndpointSuffix=core.windows.net" || _
84+
}
5085
}

0 commit comments

Comments
 (0)