Skip to content

Commit

Permalink
Fix context leak on call to AmazonS3.generatePresignedUrl (#8815)
Browse files Browse the repository at this point in the history
  • Loading branch information
laurit authored Jun 28, 2023
1 parent f25cd63 commit fac2253
Show file tree
Hide file tree
Showing 4 changed files with 46 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,9 @@ testing {
implementation("com.amazonaws:aws-java-sdk-kinesis:1.11.0")
implementation("com.amazonaws:aws-java-sdk-dynamodb:1.11.0")
implementation("com.amazonaws:aws-java-sdk-sns:1.11.0")

// needed by S3
implementation("javax.xml.bind:jaxb-api:2.3.1")
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,11 @@
package io.opentelemetry.javaagent.instrumentation.awssdk.v1_11

import com.amazonaws.AmazonWebServiceClient
import com.amazonaws.ClientConfiguration
import com.amazonaws.Request
import com.amazonaws.auth.BasicAWSCredentials
import com.amazonaws.auth.NoOpSigner
import com.amazonaws.auth.SignerFactory
import com.amazonaws.handlers.RequestHandler2
import com.amazonaws.regions.Regions
import com.amazonaws.services.s3.AmazonS3Client
Expand Down Expand Up @@ -110,4 +113,20 @@ class Aws1ClientTest extends AbstractAws1ClientTest implements AgentTestTrait {
}
}
}

def "calling generatePresignedUrl does not leak context"() {
setup:
SignerFactory.registerSigner("noop", NoOpSigner)
def client = AmazonS3ClientBuilder.standard()
.withRegion(Regions.US_EAST_1)
.withClientConfiguration(new ClientConfiguration().withSignerOverride("noop"))
.build()

when:
client.generatePresignedUrl("someBucket", "someKey", new Date())

then:
// expecting no active span after call to generatePresignedUrl
!Span.current().getSpanContext().isValid()
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ import com.amazonaws.auth.AWSCredentialsProviderChain
import com.amazonaws.auth.BasicAWSCredentials
import com.amazonaws.auth.EnvironmentVariableCredentialsProvider
import com.amazonaws.auth.InstanceProfileCredentialsProvider
import com.amazonaws.auth.NoOpSigner
import com.amazonaws.auth.SignerFactory
import com.amazonaws.auth.SystemPropertiesCredentialsProvider
import com.amazonaws.auth.profile.ProfileCredentialsProvider
import com.amazonaws.handlers.RequestHandler2
Expand Down Expand Up @@ -272,4 +274,18 @@ class Aws0ClientTest extends AgentInstrumentationSpecification {
}
}
}

def "calling generatePresignedUrl does not leak context"() {
setup:
SignerFactory.registerSigner("noop", NoOpSigner)
def client = new AmazonS3Client(new ClientConfiguration().withSignerOverride("noop"))
.withEndpoint("${server.httpUri()}")

when:
client.generatePresignedUrl("someBucket", "someKey", new Date())

then:
// expecting no active span after call to generatePresignedUrl
!Span.current().getSpanContext().isValid()
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,14 @@ final class TracingRequestHandler extends RequestHandler2 {
@Override
@SuppressWarnings("deprecation") // deprecated class to be updated once published in new location
public void beforeRequest(Request<?> request) {
// GeneratePresignedUrlRequest doesn't result in actual request, beforeRequest is the only
// method called for it. Span created here would never be ended and scope would be leaked when
// running with java agent.
if ("com.amazonaws.services.s3.model.GeneratePresignedUrlRequest"
.equals(request.getOriginalRequest().getClass().getName())) {
return;
}

Context parentContext = Context.current();
if (!requestInstrumenter.shouldStart(parentContext, request)) {
return;
Expand Down

0 comments on commit fac2253

Please sign in to comment.