Skip to content

Commit 229d4a9

Browse files
joviegasjoviegas
committed
Skip User-Agent Stage only if Additional headers configured on client option
1 parent 6d38397 commit 229d4a9

File tree

3 files changed

+66
-42
lines changed

3 files changed

+66
-42
lines changed

‎core/sdk-core/src/main/java/software/amazon/awssdk/core/internal/http/pipeline/stages/ApplyUserAgentStage.java‎

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
import java.util.Collection;
2626
import java.util.Collections;
2727
import java.util.List;
28+
import java.util.Map;
2829
import java.util.Optional;
2930
import software.amazon.awssdk.annotations.SdkInternalApi;
3031
import software.amazon.awssdk.core.ApiName;
@@ -40,7 +41,6 @@
4041
import software.amazon.awssdk.core.useragent.BusinessMetricCollection;
4142
import software.amazon.awssdk.http.SdkHttpFullRequest;
4243
import software.amazon.awssdk.identity.spi.Identity;
43-
import software.amazon.awssdk.utils.CollectionUtils;
4444
import software.amazon.awssdk.utils.CompletableFutureUtils;
4545
import software.amazon.awssdk.utils.Logger;
4646
import software.amazon.awssdk.utils.Pair;
@@ -68,20 +68,23 @@ public ApplyUserAgentStage(HttpClientDependencies dependencies) {
6868
public SdkHttpFullRequest.Builder execute(SdkHttpFullRequest.Builder request,
6969
RequestExecutionContext context) throws Exception {
7070

71-
if (request.firstMatchingHeader(HEADER_USER_AGENT).isPresent()) {
71+
if (hasUserAgentInAdditionalHeaders()) {
7272
return request;
7373
}
7474
String headerValue = finalizeUserAgent(context);
7575
return request.putHeader(HEADER_USER_AGENT, headerValue);
7676
}
7777

7878
/**
79-
* Checks if User-Agent header exists with a non-null value (including empty string).
80-
* This is done to maintain backward compatibility since MergeCustomHeadersStage merges non-null headers only.
79+
* Checks if User-Agent header is present in ADDITIONAL_HTTP_HEADERS configuration.
80+
* We skip adding user-agent in the ApplyUserAgentStage if user has set "User-Agent" header in additional header of client
8181
*/
82-
private boolean hasNonNullUserAgentHeader(SdkHttpFullRequest.Builder request) {
83-
List<String> userAgentValues = request.matchingHeaders(HEADER_USER_AGENT);
84-
return CollectionUtils.firstIfPresent(userAgentValues) != null;
82+
private boolean hasUserAgentInAdditionalHeaders() {
83+
Map<String, List<String>> additionalHeaders = clientConfig.option(SdkClientOption.ADDITIONAL_HTTP_HEADERS);
84+
if (additionalHeaders == null) {
85+
return false;
86+
}
87+
return additionalHeaders.containsKey(HEADER_USER_AGENT);
8588
}
8689

8790
/**

‎core/sdk-core/src/test/java/software/amazon/awssdk/core/internal/http/pipeline/stages/ApplyUserAgentStageTest.java‎

Lines changed: 30 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,9 @@
2525
import static software.amazon.awssdk.core.internal.useragent.UserAgentConstant.SPACE;
2626

2727
import java.util.Arrays;
28+
import java.util.LinkedHashMap;
2829
import java.util.List;
30+
import java.util.Map;
2931
import java.util.concurrent.CompletableFuture;
3032
import org.junit.Test;
3133
import org.junit.runner.RunWith;
@@ -153,7 +155,7 @@ public void when_identityContainsProvider_authSourceIsPresent() throws Exception
153155
}
154156

155157
@Test
156-
public void when_userAgentHeaderAlreadyPresent_doesNotOverwrite() throws Exception {
158+
public void when_userAgentHeaderAlreadyPresent_AndSdkOptionAdditionalHeaderNotPresent_doesNotOverwrite() throws Exception {
157159
ApplyUserAgentStage stage = new ApplyUserAgentStage(dependencies(clientUserAgent()));
158160

159161
String existingUserAgent = "CustomUserAgent/1.0";
@@ -165,13 +167,11 @@ public void when_userAgentHeaderAlreadyPresent_doesNotOverwrite() throws Excepti
165167

166168
List<String> userAgentHeaders = result.headers().get(HEADER_USER_AGENT);
167169
assertThat(userAgentHeaders).isNotNull().hasSize(1);
168-
assertThat(userAgentHeaders.get(0)).isEqualTo(existingUserAgent);
169-
// Verify it does NOT contain SDK user agent values
170-
assertThat(userAgentHeaders.get(0)).doesNotContain("aws-sdk-java");
170+
assertThat(userAgentHeaders.get(0)).startsWith("aws-sdk-java");
171171
}
172172

173173
@Test
174-
public void when_userAgentHeaderPresentButEmpty_EmptyHeaderIsPreserved() throws Exception {
174+
public void when_userAgentHeaderPresentButEmpty_sdkAddsUserAgent() throws Exception {
175175
ApplyUserAgentStage stage = new ApplyUserAgentStage(dependencies(clientUserAgent()));
176176

177177
SdkHttpFullRequest.Builder requestWithEmptyHeader = SdkHttpFullRequest.builder()
@@ -182,26 +182,26 @@ public void when_userAgentHeaderPresentButEmpty_EmptyHeaderIsPreserved() throws
182182

183183
List<String> userAgentHeaders = result.headers().get(HEADER_USER_AGENT);
184184
assertThat(userAgentHeaders).isNotNull().hasSize(1);
185-
assertThat(userAgentHeaders.get(0)).contains("aws-sdk-java");
185+
assertThat(userAgentHeaders.get(0)).startsWith("aws-sdk-java");
186186
}
187187

188188
@Test
189189
public void when_userAgentHeaderPresentButNull_sdkAddsHeader() throws Exception {
190190
ApplyUserAgentStage stage = new ApplyUserAgentStage(dependencies(clientUserAgent()));
191191
String headerValue = null;
192-
SdkHttpFullRequest.Builder requestWithEmptyHeader = SdkHttpFullRequest.builder()
193-
.putHeader(HEADER_USER_AGENT, headerValue);
192+
SdkHttpFullRequest.Builder requestWithNullHeader = SdkHttpFullRequest.builder()
193+
.putHeader(HEADER_USER_AGENT, headerValue);
194194

195195
RequestExecutionContext ctx = requestExecutionContext(executionAttributes(IDENTITY_WITHOUT_SOURCE), noOpRequest());
196-
SdkHttpFullRequest.Builder result = stage.execute(requestWithEmptyHeader, ctx);
196+
SdkHttpFullRequest.Builder result = stage.execute(requestWithNullHeader, ctx);
197197

198198
List<String> userAgentHeaders = result.headers().get(HEADER_USER_AGENT);
199199
assertThat(userAgentHeaders).isNotNull().hasSize(1);
200200
assertThat(userAgentHeaders.get(0)).startsWith("aws-sdk-java");
201201
}
202202

203203
@Test
204-
public void when_userAgentHeaderAbsent_addsHeader() throws Exception {
204+
public void when_userAgentHeaderAbsent_sdkAddsHeader() throws Exception {
205205
ApplyUserAgentStage stage = new ApplyUserAgentStage(dependencies(clientUserAgent()));
206206

207207
SdkHttpFullRequest.Builder requestWithoutHeader = SdkHttpFullRequest.builder();
@@ -215,22 +215,33 @@ public void when_userAgentHeaderAbsent_addsHeader() throws Exception {
215215
}
216216

217217
@Test
218-
public void when_multipleUserAgentHeadersPresent_doesNotOverwrite() throws Exception {
219-
ApplyUserAgentStage stage = new ApplyUserAgentStage(dependencies(clientUserAgent()));
218+
public void when_userAgentInAdditionalHeaders_doesNotOverwriteUserAgent() throws Exception {
219+
Map<String, List<String>> headerMap = new LinkedHashMap<>();
220+
headerMap.put(HEADER_USER_AGENT, Arrays.asList("CustomAgent/1.0", "AnotherAgent/2.0"));
220221

221-
SdkHttpFullRequest.Builder requestWithMultipleHeaders =
222-
SdkHttpFullRequest.builder()
223-
.putHeader(HEADER_USER_AGENT, "CustomAgent/1.0")
224-
.appendHeader(HEADER_USER_AGENT, "AnotherAgent/2.0");
222+
SdkClientConfiguration clientConfiguration =
223+
SdkClientConfiguration.builder()
224+
.option(SdkClientOption.CLIENT_USER_AGENT, clientUserAgent())
225+
.option(SdkClientOption.ADDITIONAL_HTTP_HEADERS, headerMap)
226+
.build();
227+
HttpClientDependencies httpClientDependencies = HttpClientDependencies.builder()
228+
.clientConfiguration(clientConfiguration)
229+
.build();
230+
231+
ApplyUserAgentStage stage = new ApplyUserAgentStage(httpClientDependencies);
232+
233+
SdkHttpFullRequest.Builder request = SdkHttpFullRequest.builder();
225234

226235
RequestExecutionContext ctx = requestExecutionContext(executionAttributes(IDENTITY_WITHOUT_SOURCE), noOpRequest());
227-
SdkHttpFullRequest.Builder result = stage.execute(requestWithMultipleHeaders, ctx);
236+
SdkHttpFullRequest.Builder result = stage.execute(request, ctx);
228237

238+
// ApplyUserAgentStage should skip adding User-Agent since it's in ADDITIONAL_HTTP_HEADERS
239+
// The actual merging happens in MergeCustomHeadersStage
229240
List<String> userAgentHeaders = result.headers().get(HEADER_USER_AGENT);
230-
assertThat(userAgentHeaders).hasSize(2);
231-
assertThat(userAgentHeaders).containsExactly("CustomAgent/1.0", "AnotherAgent/2.0");
241+
assertThat(userAgentHeaders).isNull();
232242
}
233243

244+
234245
private static HttpClientDependencies dependencies(String clientUserAgent) {
235246
return dependencies(clientUserAgent, null, null);
236247
}

‎test/codegen-generated-classes-test/src/test/java/software/amazon/awssdk/services/useragent/CustomUserAgentHeaderTest.java‎

Lines changed: 26 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -33,10 +33,10 @@
3333
import software.amazon.awssdk.core.interceptor.Context;
3434
import software.amazon.awssdk.core.interceptor.ExecutionAttributes;
3535
import software.amazon.awssdk.core.interceptor.ExecutionInterceptor;
36+
import software.amazon.awssdk.http.SdkHttpRequest;
3637
import software.amazon.awssdk.regions.Region;
3738
import software.amazon.awssdk.services.restjsonendpointproviders.RestJsonEndpointProvidersClient;
3839
import software.amazon.awssdk.services.restjsonendpointproviders.RestJsonEndpointProvidersClientBuilder;
39-
import software.amazon.awssdk.utils.StringUtils;
4040

4141
/**
4242
* Functional tests verifying custom User-Agent header preservation.
@@ -112,6 +112,27 @@ void executeRequest_withCustomUserAgentList_shouldPreserveAllValues(List<String>
112112
assertThat(userAgentList).isEqualTo(customUserAgentList);
113113
}
114114

115+
// ========== Header via Interceptors ==========
116+
117+
@Test
118+
void executeRequest_withInterceptorAddingUserAgent_shouldAddSdkDefaultUserAgent() {
119+
RestJsonEndpointProvidersClient client =
120+
defaultClientBuilder().overrideConfiguration(o -> o
121+
.addExecutionInterceptor(interceptor)
122+
.addExecutionInterceptor(new ExecutionInterceptor() {
123+
@Override
124+
public SdkHttpRequest modifyHttpRequest(Context.ModifyHttpRequest context,
125+
ExecutionAttributes executionAttributes) {
126+
return context.httpRequest().toBuilder()
127+
.putHeader(USER_AGENT_HEADER, "custom-agent")
128+
.build();
129+
}
130+
})).build();
131+
132+
executeRequestExpectingInterception(client);
133+
assertUserAgentContains(SDK_USER_AGENT_PREFIX);
134+
}
135+
115136
// ========== Edge Case Tests ==========
116137

117138
@Test
@@ -147,30 +168,21 @@ void buildClient_withNullListUserAgent_shouldThrowNullPointerException() {
147168
.hasMessageContaining("values must not be null");
148169
}
149170

150-
/**
151-
* Verifies that empty User-Agent list results in SDK default User-Agent.
152-
*
153-
* <p><b>Behavioral Change:</b> Previously as in when UserAgentApplyStage was done before MergeCustomHeaderStage, explicitly
154-
* setting User-Agent Header to empty String or empty list would delete the SDK User-Agent. Current behavior ensures SDK
155-
* User-Agent is always present when User-Agent Header is emptyString/EmptyList/Null.
156-
*/
157171
@Test
158172
void executeRequest_withEmptyListUserAgent_shouldResultInSdkUserAgentHeader() {
159173
RestJsonEndpointProvidersClient client = clientWithCustomUserAgentList(Collections.emptyList());
160174
executeRequestExpectingInterception(client);
161175

162176
List<String> userAgentList = getCapturedUserAgentList();
163-
assertThat(userAgentList)
164-
.hasSize(1)
165-
.anyMatch(ua -> ua.startsWith(SDK_USER_AGENT_PREFIX));
177+
assertThat(userAgentList).isNull();
166178
}
167179

168180
@Test
169181
void executeRequest_withEmptyCustomUserAgent_shouldStoreSdkUserAgent() {
170182
RestJsonEndpointProvidersClient client = clientWithCustomUserAgent("");
171183
executeRequestExpectingInterception(client);
172184

173-
assertUserAgentContains(SDK_USER_AGENT_PREFIX);
185+
assertUserAgentContains("");
174186
}
175187

176188
@Test
@@ -179,14 +191,12 @@ void executeRequest_withNullStringUserAgent_shouldStoreAsSdkUserAgent() {
179191
executeRequestExpectingInterception(client);
180192

181193
List<String> userAgentList = getCapturedUserAgentList();
182-
assertThat(userAgentList).hasSize(1);
183194
assertThat(userAgentList)
184195
.hasSize(1)
185196
.allSatisfy(ua -> {
186-
assertThat(ua).isNotNull();
187-
assertThat(ua).startsWith(SDK_USER_AGENT_PREFIX);
188-
});
197+
assertThat(ua).isNull();
189198

199+
});
190200
}
191201

192202
private void assertUserAgentContains(String expected) {

0 commit comments

Comments
 (0)