Skip to content

Commit 6d725f2

Browse files
committed
Address review comments
1 parent bd27de1 commit 6d725f2

5 files changed

Lines changed: 41 additions & 25 deletions

File tree

core/aws-core/src/main/java/software/amazon/awssdk/awscore/internal/AwsExecutionContextBuilder.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -161,7 +161,7 @@ private AwsExecutionContextBuilder() {
161161
ExecutionInterceptorChain executionInterceptorChain =
162162
new ExecutionInterceptorChain(clientConfig.option(SdkClientOption.EXECUTION_INTERCEPTORS));
163163

164-
executionAttributes.putAttribute(SdkInternalExecutionAttribute.AUTH_SCHEME_BEFORE_INTERCEPTORS,
164+
executionAttributes.putAttribute(SdkInternalExecutionAttribute.AUTH_SCHEME_SNAPSHOT_PRE_INTERCEPTORS,
165165
executionAttributes.getAttribute(SdkInternalExecutionAttribute.SELECTED_AUTH_SCHEME));
166166

167167
InterceptorContext interceptorContext = InterceptorContext.builder()

core/sdk-core/src/main/java/software/amazon/awssdk/core/http/auth/AuthSchemeResolver.java

Lines changed: 32 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -127,32 +127,43 @@ public static SelectedAuthScheme<? extends Identity> selectAuthScheme(
127127

128128
/**
129129
* Merge properties from any pre-existing auth scheme into the selected one.
130+
*
131+
* After auth scheme resolution produces a fresh selectedAuthScheme, this method ensures that any signer properties
132+
* explicitly set by interceptors (e.g., signing region override) take priority over the resolved values.
130133
*/
131134
public static <T extends Identity> SelectedAuthScheme<T> mergePreExistingAuthSchemeProperties(
132135
SelectedAuthScheme<T> selectedAuthScheme,
133136
ExecutionAttributes executionAttributes) {
134137

138+
// The "existing" auth scheme is what's currently on SELECTED_AUTH_SCHEME - potentially modified by interceptors.
135139
SelectedAuthScheme<?> existingAuthScheme =
136140
executionAttributes.getAttribute(SdkInternalExecutionAttribute.SELECTED_AUTH_SCHEME);
137141

138142
if (existingAuthScheme == null) {
139143
return selectedAuthScheme;
140144
}
141145

142-
SelectedAuthScheme<?> beforeInterceptors =
143-
executionAttributes.getAttribute(SdkInternalExecutionAttribute.AUTH_SCHEME_BEFORE_INTERCEPTORS);
146+
// Snapshot taken before interceptors ran — used to detect what interceptors changed.
147+
SelectedAuthScheme<?> authSchemeBeforeInterceptors =
148+
executionAttributes.getAttribute(SdkInternalExecutionAttribute.AUTH_SCHEME_SNAPSHOT_PRE_INTERCEPTORS);
144149

145-
if (beforeInterceptors != null &&
146-
beforeInterceptors.authSchemeOption() == existingAuthScheme.authSchemeOption()) {
150+
// If the auth scheme option is the same object reference before and after
151+
// interceptors, no interceptor modified it — skip the merge entirely.
152+
if (authSchemeBeforeInterceptors != null &&
153+
authSchemeBeforeInterceptors.authSchemeOption() == existingAuthScheme.authSchemeOption()) {
147154
return selectedAuthScheme;
148155
}
149156

157+
// Start with the freshly resolved auth scheme as the base.
150158
AuthSchemeOption.Builder mergedOption = selectedAuthScheme.authSchemeOption().toBuilder();
151159

160+
// For each signer property on the interceptor-modified scheme:
161+
// If the interceptor changed it (differs from pre-interceptor snapshot), apply interceptor override
162+
// If unchanged (same as before interceptors) only add if not already on the resolved scheme
152163
existingAuthScheme.authSchemeOption().forEachSignerProperty(new AuthSchemeOption.SignerPropertyConsumer() {
153164
@Override
154165
public <S> void accept(SignerProperty<S> key, S value) {
155-
if (wasModifiedByInterceptor(beforeInterceptors, key, value)) {
166+
if (wasModifiedByInterceptor(authSchemeBeforeInterceptors, key, value)) {
156167
mergedOption.putSignerProperty(key, value);
157168
} else {
158169
mergedOption.putSignerPropertyIfAbsent(key, value);
@@ -169,12 +180,13 @@ public <S> void accept(SignerProperty<S> key, S value) {
169180
);
170181
}
171182

172-
private static <T> boolean wasModifiedByInterceptor(SelectedAuthScheme<?> beforeInterceptors,
183+
/**
184+
* Returns true if the given property value differs from what it was before interceptors ran,
185+
* meaning an interceptor explicitly changed it.
186+
*/
187+
private static <T> boolean wasModifiedByInterceptor(SelectedAuthScheme<?> authSchemeBeforeInterceptors,
173188
SignerProperty<T> key, T currentValue) {
174-
if (beforeInterceptors == null) {
175-
return true;
176-
}
177-
T originalValue = beforeInterceptors.authSchemeOption().signerProperty(key);
189+
T originalValue = authSchemeBeforeInterceptors.authSchemeOption().signerProperty(key);
178190
return !Objects.equals(originalValue, currentValue);
179191
}
180192

@@ -183,35 +195,39 @@ private static <T> boolean wasModifiedByInterceptor(SelectedAuthScheme<?> before
183195
* Called after endpoint resolution, which may have overwritten properties that interceptors set.
184196
*/
185197
public static void applyInterceptorModifiedProperties(SelectedAuthScheme<?> currentScheme,
186-
SelectedAuthScheme<?> beforeInterceptors,
187-
SelectedAuthScheme<?> afterInterceptors,
188-
ExecutionAttributes attrs) {
198+
SelectedAuthScheme<?> authSchemeBeforeInterceptors,
199+
SelectedAuthScheme<?> afterInterceptors,
200+
ExecutionAttributes attrs) {
189201
if (afterInterceptors == null) {
190202
return;
191203
}
192-
doApplyInterceptorModifiedProperties(currentScheme, beforeInterceptors, afterInterceptors, attrs);
204+
doApplyInterceptorModifiedProperties(currentScheme, authSchemeBeforeInterceptors, afterInterceptors, attrs);
193205
}
194206

195207
@SuppressWarnings("unchecked")
196208
private static <T extends Identity> void doApplyInterceptorModifiedProperties(
197209
SelectedAuthScheme<T> currentScheme,
198-
SelectedAuthScheme<?> beforeInterceptors,
210+
SelectedAuthScheme<?> authSchemeBeforeInterceptors,
199211
SelectedAuthScheme<?> afterInterceptors,
200212
ExecutionAttributes attrs) {
201213

214+
// Start with the current endpoint resolved auth scheme as the base.
202215
AuthSchemeOption.Builder mergedOption = currentScheme.authSchemeOption().toBuilder();
203216
boolean[] changed = {false};
204217

218+
// For each property on the post-interceptor scheme, check if the interceptor changed it.
219+
// If yes, apply it onto the current scheme.
205220
afterInterceptors.authSchemeOption().forEachSignerProperty(new AuthSchemeOption.SignerPropertyConsumer() {
206221
@Override
207222
public <S> void accept(SignerProperty<S> key, S value) {
208-
if (wasModifiedByInterceptor(beforeInterceptors, key, value)) {
223+
if (wasModifiedByInterceptor(authSchemeBeforeInterceptors, key, value)) {
209224
mergedOption.putSignerProperty(key, value);
210225
changed[0] = true;
211226
}
212227
}
213228
});
214229

230+
// Only update SELECTED_AUTH_SCHEME if at least one property was re-applied.
215231
if (changed[0]) {
216232
attrs.putAttribute(SdkInternalExecutionAttribute.SELECTED_AUTH_SCHEME,
217233
new SelectedAuthScheme<>(currentScheme.identity(),

core/sdk-core/src/main/java/software/amazon/awssdk/core/interceptor/SdkInternalExecutionAttribute.java

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -209,16 +209,16 @@ public final class SdkInternalExecutionAttribute extends SdkExecutionAttribute {
209209
* Used by {@code AuthSchemeResolver#mergePreExistingAuthSchemeProperties} to detect which signer properties
210210
* were explicitly modified by interceptors (and should therefore override the freshly-resolved values).
211211
*/
212-
public static final ExecutionAttribute<SelectedAuthScheme<?>> AUTH_SCHEME_BEFORE_INTERCEPTORS =
213-
new ExecutionAttribute<>("AuthSchemeBeforeInterceptors");
212+
public static final ExecutionAttribute<SelectedAuthScheme<?>> AUTH_SCHEME_SNAPSHOT_PRE_INTERCEPTORS =
213+
new ExecutionAttribute<>("AuthSchemeSnapshotPreInterceptors");
214214

215215
/**
216216
* Snapshot of {@link #SELECTED_AUTH_SCHEME} taken after interceptors run but before auth scheme resolution.
217-
* Together with {@link #AUTH_SCHEME_BEFORE_INTERCEPTORS}, this allows detecting which signer properties
217+
* Together with {@link #AUTH_SCHEME_SNAPSHOT_PRE_INTERCEPTORS}, this allows detecting which signer properties
218218
* were explicitly modified by interceptors so they can be re-applied after endpoint resolution.
219219
*/
220-
public static final ExecutionAttribute<SelectedAuthScheme<?>> AUTH_SCHEME_AFTER_INTERCEPTORS =
221-
new ExecutionAttribute<>("AuthSchemeAfterInterceptors");
220+
public static final ExecutionAttribute<SelectedAuthScheme<?>> AUTH_SCHEME_SNAPSHOT_POST_INTERCEPTORS =
221+
new ExecutionAttribute<>("AuthSchemeSnapshotPostInterceptors");
222222

223223
/**
224224
* The supported compression algorithms for an operation, and whether the operation is streaming or not.

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ public SdkHttpFullRequest.Builder execute(SdkHttpFullRequest.Builder request, Re
7676
SelectedAuthScheme<? extends Identity> selectedAuthScheme =
7777
AuthSchemeResolver.selectAuthScheme(authOptions, authSchemes, identityProviders, metricCollector);
7878

79-
executionAttributes.putAttribute(SdkInternalExecutionAttribute.AUTH_SCHEME_AFTER_INTERCEPTORS,
79+
executionAttributes.putAttribute(SdkInternalExecutionAttribute.AUTH_SCHEME_SNAPSHOT_POST_INTERCEPTORS,
8080
executionAttributes.getAttribute(SdkInternalExecutionAttribute.SELECTED_AUTH_SCHEME));
8181

8282
selectedAuthScheme = AuthSchemeResolver.mergePreExistingAuthSchemeProperties(selectedAuthScheme, executionAttributes);

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -151,10 +151,10 @@ private static void reapplyInterceptorModifiedAuthProperties(ExecutionAttributes
151151
return;
152152
}
153153
SelectedAuthScheme<?> beforeInterceptors =
154-
attrs.getAttribute(SdkInternalExecutionAttribute.AUTH_SCHEME_BEFORE_INTERCEPTORS);
154+
attrs.getAttribute(SdkInternalExecutionAttribute.AUTH_SCHEME_SNAPSHOT_PRE_INTERCEPTORS);
155155

156156
SelectedAuthScheme<?> afterInterceptors =
157-
attrs.getAttribute(SdkInternalExecutionAttribute.AUTH_SCHEME_AFTER_INTERCEPTORS);
157+
attrs.getAttribute(SdkInternalExecutionAttribute.AUTH_SCHEME_SNAPSHOT_POST_INTERCEPTORS);
158158
if (afterInterceptors == null) {
159159
return;
160160
}

0 commit comments

Comments
 (0)