Skip to content

Commit

Permalink
feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
aajtodd committed Oct 20, 2023
1 parent fb94a1b commit 85aa20c
Show file tree
Hide file tree
Showing 5 changed files with 8 additions and 16 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ public class AwsSigningConfig(builder: Builder) {
* This is an opt-in configuration because these intermediate outputs may contain sensitive fields bound
* to headers, URI, or query parameters.
*/
public val enableTraceLogging: Boolean = builder.enableTraceLogging
public val logRequest: Boolean = builder.logRequest

public fun toBuilder(): Builder = Builder().also {
it.region = region
Expand All @@ -186,7 +186,7 @@ public class AwsSigningConfig(builder: Builder) {
it.signedBodyHeader = signedBodyHeader
it.credentials = credentials
it.expiresAfter = expiresAfter
it.enableTraceLogging = enableTraceLogging
it.logRequest = logRequest
}

public class Builder {
Expand All @@ -203,7 +203,7 @@ public class AwsSigningConfig(builder: Builder) {
public var signedBodyHeader: AwsSignedBodyHeader = AwsSignedBodyHeader.NONE
public var credentials: Credentials? = null
public var expiresAfter: Duration? = null
public var enableTraceLogging: Boolean = false
public var logRequest: Boolean = false

public fun build(): AwsSigningConfig = AwsSigningConfig(this)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ internal class DefaultAwsSignerImpl(
require(config.algorithm == AwsSigningAlgorithm.SIGV4) { "${config.algorithm} support is not yet implemented" }

val canonical = canonicalizer.canonicalRequest(request, config)
if (config.enableTraceLogging) {
if (config.logRequest) {
logger.trace { "Canonical request:\n${canonical.requestString}" }
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ public class AwsHttpSigner(private val config: Config) : HttpSigner {
shouldSignHeader = config.shouldSignHeader

signedBodyHeader = contextSignedBodyHeader ?: config.signedBodyHeader
enableTraceLogging = attributes.getOrNull(SdkClientOption.LogMode)?.isEnabled(LogMode.LogSigning) == true
logRequest = attributes.getOrNull(SdkClientOption.LogMode)?.isEnabled(LogMode.LogRequest) == true

// SDKs are supposed to default to signed payload _always_ when possible (and when `unsignedPayload` trait
// isn't present). The only exception is when the customer explicitly disables signed payloads (via Config.isUnsignedPayload).
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,13 +52,6 @@ public sealed class LogMode(private val mask: Int) {
override fun toString(): String = "LogResponseWithBody"
}

/**
* Log intermediate signing details (e.g. canonical request, string to sign, etc)
*/
public object LogSigning : LogMode(0x10) {
override fun toString(): String = "LogSigning"
}

internal class Composite(mask: Int) : LogMode(mask)

public operator fun plus(mode: LogMode): LogMode = Composite(mask or mode.mask)
Expand All @@ -80,7 +73,6 @@ public sealed class LogMode(private val mask: Int) {
LogRequestWithBody,
LogResponse,
LogResponseWithBody,
LogSigning,
)

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ class LogModeTest {
fun testToString() {
val mode = LogMode.allModes().reduce { acc, curr -> acc + curr }
assertTrue { LogMode.allModes().all { mode.isEnabled(it) } }
val expected = "LogRequest|LogRequestWithBody|LogResponse|LogResponseWithBody|LogSigning"
val expected = "LogRequest|LogRequestWithBody|LogResponse|LogResponseWithBody"
assertEquals(expected, mode.toString())
}

Expand All @@ -45,8 +45,8 @@ class LogModeTest {
@Test
fun testFromStringComposite() {
assertEquals(
LogMode.fromString("LogRequest|LogRequestWithBody|LogResponse|LogSigning"),
(LogMode.LogRequest + LogMode.LogRequestWithBody + LogMode.LogResponse + LogMode.LogSigning),
LogMode.fromString("LogRequest|LogRequestWithBody|LogResponse"),
(LogMode.LogRequest + LogMode.LogRequestWithBody + LogMode.LogResponse),
)
}

Expand Down

0 comments on commit 85aa20c

Please sign in to comment.