Skip to content

Commit 0fedea3

Browse files
authored
Performance improvement for sigv4 signing. (#4891)
This is a reintroduction of the reverted commit #4867. This includes a fix to the issue that caused the revert: improper handling of empty header values.
1 parent 9aeba32 commit 0fedea3

File tree

2 files changed

+78
-25
lines changed

2 files changed

+78
-25
lines changed

core/http-auth-aws/src/main/java/software/amazon/awssdk/http/auth/aws/internal/signer/V4CanonicalRequest.java

Lines changed: 48 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -181,7 +181,10 @@ public static List<Pair<String, List<String>>> getCanonicalHeaders(Map<String, L
181181
* Each header-value pair is separated by a newline.
182182
*/
183183
public static String getCanonicalHeadersString(List<Pair<String, List<String>>> canonicalHeaders) {
184-
StringBuilder result = new StringBuilder(512);
184+
// 2048 chosen experimentally to avoid always needing to resize the string builder's internal byte array.
185+
// The minimal DynamoDB get-item request at the time of testing used ~1100 bytes. 2048 was chosen as the
186+
// next-highest power-of-two.
187+
StringBuilder result = new StringBuilder(2048);
185188
canonicalHeaders.forEach(header -> {
186189
result.append(header.left());
187190
result.append(":");
@@ -246,35 +249,45 @@ private static String getCanonicalRequestString(String httpMethod, String canoni
246249
* Matcher object as well.
247250
*/
248251
private static void addAndTrim(StringBuilder result, String value) {
249-
int lengthBefore = result.length();
250-
boolean isStart = true;
251-
boolean previousIsWhiteSpace = false;
252-
253-
for (int i = 0; i < value.length(); i++) {
254-
char ch = value.charAt(i);
255-
if (isWhiteSpace(ch)) {
256-
if (previousIsWhiteSpace || isStart) {
257-
continue;
258-
}
259-
result.append(' ');
260-
previousIsWhiteSpace = true;
261-
} else {
262-
result.append(ch);
263-
isStart = false;
264-
previousIsWhiteSpace = false;
265-
}
252+
int valueLength = value.length();
253+
if (valueLength == 0) {
254+
return;
266255
}
267256

268-
if (lengthBefore == result.length()) {
269-
return;
257+
int start = 0;
258+
// Find first non-whitespace
259+
while (isWhiteSpace(value.charAt(start))) {
260+
++start;
261+
if (start >= valueLength) {
262+
return;
263+
}
270264
}
271265

272-
int lastNonWhitespaceChar = result.length() - 1;
273-
while (isWhiteSpace(result.charAt(lastNonWhitespaceChar))) {
274-
--lastNonWhitespaceChar;
266+
// Add things word-by-word
267+
int lastWordStart = start;
268+
boolean lastWasWhitespace = false;
269+
for (int i = start; i < valueLength; i++) {
270+
char c = value.charAt(i);
271+
272+
if (isWhiteSpace(c)) {
273+
if (!lastWasWhitespace) {
274+
// End of word, add word
275+
result.append(value, lastWordStart, i);
276+
lastWasWhitespace = true;
277+
}
278+
} else {
279+
if (lastWasWhitespace) {
280+
// Start of new word, add space
281+
result.append(' ');
282+
lastWordStart = i;
283+
lastWasWhitespace = false;
284+
}
285+
}
275286
}
276287

277-
result.setLength(lastNonWhitespaceChar + 1);
288+
if (!lastWasWhitespace) {
289+
result.append(value, lastWordStart, valueLength);
290+
}
278291
}
279292

280293
/**
@@ -365,7 +378,17 @@ private static String getCanonicalQueryString(SortedMap<String, List<String>> ca
365378
}
366379

367380
private static boolean isWhiteSpace(char ch) {
368-
return ch == ' ' || ch == '\t' || ch == '\n' || ch == '\u000b' || ch == '\r' || ch == '\f';
381+
switch (ch) {
382+
case ' ':
383+
case '\t':
384+
case '\n':
385+
case '\u000b':
386+
case '\r':
387+
case '\f':
388+
return true;
389+
default:
390+
return false;
391+
}
369392
}
370393

371394
/**

core/http-auth-aws/src/test/java/software/amazon/awssdk/http/auth/aws/internal/signer/V4CanonicalRequestTest.java

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,36 @@ public void canonicalRequest_withSpacedHeaders_shouldStripWhitespace() {
123123
assertEquals("PUT\n/\n\nfoo:bar baz\n\nfoo\nsha-256", cr.getCanonicalRequestString());
124124
}
125125

126+
@Test
127+
public void canonicalRequest_withEmptyHeaders_shouldSucceed() {
128+
SdkHttpRequest request = SdkHttpRequest.builder()
129+
.protocol("https")
130+
.host("localhost")
131+
.method(SdkHttpMethod.PUT)
132+
.putHeader("foo", "")
133+
.build();
134+
V4CanonicalRequest cr = new V4CanonicalRequest(request, "sha-256",
135+
new V4CanonicalRequest.Options(true,
136+
true));
137+
138+
assertEquals("PUT\n/\n\nfoo:\n\nfoo\nsha-256", cr.getCanonicalRequestString());
139+
}
140+
141+
@Test
142+
public void canonicalRequest_withWhitespaceHeaders_shouldSucceed() {
143+
SdkHttpRequest request = SdkHttpRequest.builder()
144+
.protocol("https")
145+
.host("localhost")
146+
.method(SdkHttpMethod.PUT)
147+
.putHeader("foo", " ")
148+
.build();
149+
V4CanonicalRequest cr = new V4CanonicalRequest(request, "sha-256",
150+
new V4CanonicalRequest.Options(true,
151+
true));
152+
153+
assertEquals("PUT\n/\n\nfoo:\n\nfoo\nsha-256", cr.getCanonicalRequestString());
154+
}
155+
126156
@Test
127157
public void canonicalRequest_WithNullParamValue_shouldIncludeEquals() {
128158
SdkHttpRequest request = SdkHttpRequest.builder()

0 commit comments

Comments
 (0)