From 7abd6b9d7e39bac442256040cbd11ad4bc562f92 Mon Sep 17 00:00:00 2001 From: Enno <458526+ennru@users.noreply.github.com> Date: Thu, 12 Sep 2019 19:06:06 +0200 Subject: [PATCH] AWS S3: make raw request URI contains path and query (#1923) * make raw request URI contains path and query * Shut down actor systems in tests --- .../akka/stream/alpakka/s3/impl/HttpRequests.scala | 6 +++--- s3/src/test/resources/logback-test.xml | 4 +++- .../akka/stream/alpakka/s3/impl/DiskBufferSpec.scala | 2 ++ .../stream/alpakka/s3/impl/HttpRequestsSpec.scala | 6 +++--- .../akka/stream/alpakka/s3/impl/MarshallingSpec.scala | 11 +++++++++-- .../stream/alpakka/s3/impl/MemoryBufferSpec.scala | 2 ++ .../akka/stream/alpakka/s3/impl/S3StreamSpec.scala | 5 ++++- .../stream/alpakka/s3/impl/SplitAfterSizeSpec.scala | 2 ++ .../akka/stream/alpakka/s3/impl/auth/SignerSpec.scala | 11 +++++++++-- .../stream/alpakka/s3/impl/auth/StreamUtilsSpec.scala | 8 +++++--- .../alpakka/s3/scaladsl/S3ClientIntegrationSpec.scala | 4 ++++ .../alpakka/s3/scaladsl/S3IntegrationSpec.scala | 3 +++ .../stream/alpakka/s3/scaladsl/S3WireMockBase.scala | 2 +- 13 files changed, 50 insertions(+), 16 deletions(-) diff --git a/s3/src/main/scala/akka/stream/alpakka/s3/impl/HttpRequests.scala b/s3/src/main/scala/akka/stream/alpakka/s3/impl/HttpRequests.scala index 1f5f40e226..78e389023a 100644 --- a/s3/src/main/scala/akka/stream/alpakka/s3/impl/HttpRequests.scala +++ b/s3/src/main/scala/akka/stream/alpakka/s3/impl/HttpRequests.scala @@ -217,13 +217,13 @@ import scala.concurrent.{ExecutionContext, Future} } private def rawRequestUri(uri: Uri): String = { - val rawUri = uri.toString + val rawUri = uri.toHttpRequestTargetOriginForm.toString val rawPath = uri.path.toString() if (rawPath.contains("+")) { val fixedPath = rawPath.replaceAll("\\+", "%2B") - - rawUri.replace(rawPath, fixedPath) + require(rawUri startsWith rawPath) + fixedPath + rawUri.drop(rawPath.length) } else { rawUri } diff --git a/s3/src/test/resources/logback-test.xml b/s3/src/test/resources/logback-test.xml index 1ef81cf71e..ec52eb4ed2 100644 --- a/s3/src/test/resources/logback-test.xml +++ b/s3/src/test/resources/logback-test.xml @@ -7,9 +7,11 @@ + + - \ No newline at end of file + diff --git a/s3/src/test/scala/akka/stream/alpakka/s3/impl/DiskBufferSpec.scala b/s3/src/test/scala/akka/stream/alpakka/s3/impl/DiskBufferSpec.scala index c1f69fb9f7..3efcf09b5d 100644 --- a/s3/src/test/scala/akka/stream/alpakka/s3/impl/DiskBufferSpec.scala +++ b/s3/src/test/scala/akka/stream/alpakka/s3/impl/DiskBufferSpec.scala @@ -30,6 +30,8 @@ class DiskBufferSpec(_system: ActorSystem) implicit val materializer = ActorMaterializer(ActorMaterializerSettings(system).withDebugLogging(true)) + override protected def afterAll(): Unit = TestKit.shutdownActorSystem(system) + "DiskBuffer" should "emit a chunk on its output containing the concatenation of all input values" in { val result = Source(Vector(ByteString(1, 2, 3, 4, 5), ByteString(6, 7, 8, 9, 10, 11, 12), ByteString(13, 14))) diff --git a/s3/src/test/scala/akka/stream/alpakka/s3/impl/HttpRequestsSpec.scala b/s3/src/test/scala/akka/stream/alpakka/s3/impl/HttpRequestsSpec.scala index 25e5f829c3..84b5307595 100644 --- a/s3/src/test/scala/akka/stream/alpakka/s3/impl/HttpRequestsSpec.scala +++ b/s3/src/test/scala/akka/stream/alpakka/s3/impl/HttpRequestsSpec.scala @@ -15,7 +15,7 @@ import akka.stream.ActorMaterializer import akka.stream.alpakka.s3.headers.{CannedAcl, ServerSideEncryption, StorageClass} import akka.stream.alpakka.s3.{ApiVersion, BufferType, MemoryBufferType, MetaHeaders, S3Headers, S3Settings} import akka.stream.scaladsl.Source -import akka.testkit.{SocketUtil, TestProbe} +import akka.testkit.{SocketUtil, TestKit, TestProbe} import com.amazonaws.auth.{AWSCredentialsProvider, AWSStaticCredentialsProvider, AnonymousAWSCredentials} import com.amazonaws.regions.AwsRegionProvider import org.scalatest.concurrent.ScalaFutures @@ -184,7 +184,7 @@ class HttpRequestsSpec extends FlatSpec with Matchers with ScalaFutures { val req = HttpRequests.getDownloadRequest(location) req.uri.authority.host.toString shouldEqual "bucket.s3.amazonaws.com" req.uri.path.toString shouldEqual "/test%20folder/1%20+%202%20=%203" - req.headers should contain(`Raw-Request-URI`("https://bucket.s3.amazonaws.com/test%20folder/1%20%2B%202%20=%203")) + req.headers should contain(`Raw-Request-URI`("/test%20folder/1%20%2B%202%20=%203")) } it should "support download requests with keys containing spaces with path-style access in other regions" in { @@ -375,7 +375,7 @@ class HttpRequestsSpec extends FlatSpec with Matchers with ScalaFutures { probe.expectMsgType[HttpRequest] materializer.shutdown() - system.terminate() + TestKit.shutdownActorSystem(system) } it should "add two (source, range) headers to multipart upload (copy) request when byte range populated" in { diff --git a/s3/src/test/scala/akka/stream/alpakka/s3/impl/MarshallingSpec.scala b/s3/src/test/scala/akka/stream/alpakka/s3/impl/MarshallingSpec.scala index d5aff1ea55..a28f85bfb9 100644 --- a/s3/src/test/scala/akka/stream/alpakka/s3/impl/MarshallingSpec.scala +++ b/s3/src/test/scala/akka/stream/alpakka/s3/impl/MarshallingSpec.scala @@ -12,17 +12,24 @@ import akka.stream.{ActorMaterializer, ActorMaterializerSettings} import akka.stream.alpakka.s3.ListBucketResultContents import akka.testkit.TestKit import org.scalatest.concurrent.ScalaFutures -import org.scalatest.{FlatSpecLike, Matchers} +import org.scalatest.{BeforeAndAfterAll, FlatSpecLike, Matchers} import scala.collection.immutable.Seq -class MarshallingSpec(_system: ActorSystem) extends TestKit(_system) with FlatSpecLike with Matchers with ScalaFutures { +class MarshallingSpec(_system: ActorSystem) + extends TestKit(_system) + with FlatSpecLike + with Matchers + with ScalaFutures + with BeforeAndAfterAll { def this() = this(ActorSystem("MarshallingSpec")) implicit val materializer = ActorMaterializer(ActorMaterializerSettings(system).withDebugLogging(true)) implicit val ec = materializer.executionContext + override protected def afterAll(): Unit = TestKit.shutdownActorSystem(system) + val xmlString = """ | | bucket diff --git a/s3/src/test/scala/akka/stream/alpakka/s3/impl/MemoryBufferSpec.scala b/s3/src/test/scala/akka/stream/alpakka/s3/impl/MemoryBufferSpec.scala index 6838d1c4de..07b17db598 100644 --- a/s3/src/test/scala/akka/stream/alpakka/s3/impl/MemoryBufferSpec.scala +++ b/s3/src/test/scala/akka/stream/alpakka/s3/impl/MemoryBufferSpec.scala @@ -27,6 +27,8 @@ class MemoryBufferSpec(_system: ActorSystem) implicit val materializer = ActorMaterializer(ActorMaterializerSettings(system).withDebugLogging(true)) + override protected def afterAll(): Unit = TestKit.shutdownActorSystem(system) + "MemoryBuffer" should "emit a chunk on its output containg the concatenation of all input values" in { val result = Source(Vector(ByteString(1, 2, 3, 4, 5), ByteString(6, 7, 8, 9, 10, 11, 12), ByteString(13, 14))) .via(new MemoryBuffer(200)) diff --git a/s3/src/test/scala/akka/stream/alpakka/s3/impl/S3StreamSpec.scala b/s3/src/test/scala/akka/stream/alpakka/s3/impl/S3StreamSpec.scala index 115036b187..7ff10e9479 100644 --- a/s3/src/test/scala/akka/stream/alpakka/s3/impl/S3StreamSpec.scala +++ b/s3/src/test/scala/akka/stream/alpakka/s3/impl/S3StreamSpec.scala @@ -16,7 +16,7 @@ import akka.util.ByteString import com.amazonaws.auth.{AWSStaticCredentialsProvider, BasicAWSCredentials} import com.amazonaws.regions.AwsRegionProvider import org.scalatest.concurrent.{IntegrationPatience, ScalaFutures} -import org.scalatest.{FlatSpecLike, Matchers, PrivateMethodTester} +import org.scalatest.{BeforeAndAfterAll, FlatSpecLike, Matchers, PrivateMethodTester} import scala.concurrent.Future @@ -24,6 +24,7 @@ class S3StreamSpec(_system: ActorSystem) extends TestKit(_system) with FlatSpecLike with Matchers + with BeforeAndAfterAll with PrivateMethodTester with ScalaFutures with IntegrationPatience { @@ -33,6 +34,8 @@ class S3StreamSpec(_system: ActorSystem) def this() = this(ActorSystem("S3StreamSpec")) implicit val materializer = ActorMaterializer(ActorMaterializerSettings(system).withDebugLogging(true)) + override protected def afterAll(): Unit = TestKit.shutdownActorSystem(system) + "Non-ranged downloads" should "have two (host and synthetic raw-request-uri) headers" in { val requestHeaders = PrivateMethod[HttpRequest]('requestHeaders) diff --git a/s3/src/test/scala/akka/stream/alpakka/s3/impl/SplitAfterSizeSpec.scala b/s3/src/test/scala/akka/stream/alpakka/s3/impl/SplitAfterSizeSpec.scala index 78e809761d..9665672174 100644 --- a/s3/src/test/scala/akka/stream/alpakka/s3/impl/SplitAfterSizeSpec.scala +++ b/s3/src/test/scala/akka/stream/alpakka/s3/impl/SplitAfterSizeSpec.scala @@ -26,6 +26,8 @@ class SplitAfterSizeSpec(_system: ActorSystem) implicit val materializer = ActorMaterializer(ActorMaterializerSettings(system).withDebugLogging(true)) + override protected def afterAll(): Unit = TestKit.shutdownActorSystem(system) + final val MaxChunkSize = 1024 "SplitAfterSize" should "yield a single empty substream on no input" in assertAllStagesStopped { Source diff --git a/s3/src/test/scala/akka/stream/alpakka/s3/impl/auth/SignerSpec.scala b/s3/src/test/scala/akka/stream/alpakka/s3/impl/auth/SignerSpec.scala index 85309a6475..37f9c6b0c0 100644 --- a/s3/src/test/scala/akka/stream/alpakka/s3/impl/auth/SignerSpec.scala +++ b/s3/src/test/scala/akka/stream/alpakka/s3/impl/auth/SignerSpec.scala @@ -19,14 +19,19 @@ import com.amazonaws.auth.{ BasicAWSCredentials, BasicSessionCredentials } -import org.scalatest.{FlatSpecLike, Matchers} +import org.scalatest.{BeforeAndAfterAll, FlatSpecLike, Matchers} import org.scalatest.concurrent.ScalaFutures import org.scalatest.OptionValues._ import org.scalatest.time.{Millis, Seconds, Span} import scala.compat.java8.OptionConverters._ -class SignerSpec(_system: ActorSystem) extends TestKit(_system) with FlatSpecLike with Matchers with ScalaFutures { +class SignerSpec(_system: ActorSystem) + extends TestKit(_system) + with FlatSpecLike + with Matchers + with BeforeAndAfterAll + with ScalaFutures { def this() = this(ActorSystem("SignerSpec")) implicit val defaultPatience = @@ -38,6 +43,8 @@ class SignerSpec(_system: ActorSystem) extends TestKit(_system) with FlatSpecLik new BasicAWSCredentials("AKIDEXAMPLE", "wJalrXUtnFEMI/K7MDENG+bPxRfiCYEXAMPLEKEY") ) + override protected def afterAll(): Unit = TestKit.shutdownActorSystem(system) + def signingKey(dateTime: ZonedDateTime) = SigningKey(dateTime, credentials, CredentialScope(dateTime.toLocalDate, "us-east-1", "iam")) diff --git a/s3/src/test/scala/akka/stream/alpakka/s3/impl/auth/StreamUtilsSpec.scala b/s3/src/test/scala/akka/stream/alpakka/s3/impl/auth/StreamUtilsSpec.scala index bdf42c2747..8de65ab4fd 100644 --- a/s3/src/test/scala/akka/stream/alpakka/s3/impl/auth/StreamUtilsSpec.scala +++ b/s3/src/test/scala/akka/stream/alpakka/s3/impl/auth/StreamUtilsSpec.scala @@ -31,6 +31,11 @@ class StreamUtilsSpec(_system: ActorSystem) implicit val defaultPatience = PatienceConfig(timeout = Span(5, Seconds), interval = Span(30, Millis)) + override protected def afterAll(): Unit = { + fs.close() + TestKit.shutdownActorSystem(system) + } + val fs = Jimfs.newFileSystem("FileSourceSpec", Configuration.unix()) val TestText = { @@ -78,7 +83,4 @@ class StreamUtilsSpec(_system: ActorSystem) result should contain theSameElementsInOrderAs dis.getMessageDigest.digest() } } - - override def afterAll(): Unit = fs.close() - } diff --git a/s3/src/test/scala/akka/stream/alpakka/s3/scaladsl/S3ClientIntegrationSpec.scala b/s3/src/test/scala/akka/stream/alpakka/s3/scaladsl/S3ClientIntegrationSpec.scala index 0ff4bf0623..d004461847 100644 --- a/s3/src/test/scala/akka/stream/alpakka/s3/scaladsl/S3ClientIntegrationSpec.scala +++ b/s3/src/test/scala/akka/stream/alpakka/s3/scaladsl/S3ClientIntegrationSpec.scala @@ -6,6 +6,7 @@ package akka.stream.alpakka.s3.scaladsl import akka.actor.ActorSystem import akka.stream.ActorMaterializer +import akka.testkit.TestKit import org.scalatest.concurrent.{IntegrationPatience, ScalaFutures} import org.scalatest._ @@ -19,4 +20,7 @@ trait S3ClientIntegrationSpec implicit val system: ActorSystem implicit val materializer = ActorMaterializer() + + override protected def afterAll(): Unit = TestKit.shutdownActorSystem(system) + } diff --git a/s3/src/test/scala/akka/stream/alpakka/s3/scaladsl/S3IntegrationSpec.scala b/s3/src/test/scala/akka/stream/alpakka/s3/scaladsl/S3IntegrationSpec.scala index 8f0192c0e2..7f1724a8a6 100644 --- a/s3/src/test/scala/akka/stream/alpakka/s3/scaladsl/S3IntegrationSpec.scala +++ b/s3/src/test/scala/akka/stream/alpakka/s3/scaladsl/S3IntegrationSpec.scala @@ -13,6 +13,7 @@ import akka.stream.ActorMaterializer import akka.stream.alpakka.s3.BucketAccess.{AccessGranted, NotExists} import akka.stream.alpakka.s3._ import akka.stream.scaladsl.{Keep, Sink, Source} +import akka.testkit.TestKit import akka.util.ByteString import com.amazonaws.auth.{AWSStaticCredentialsProvider, BasicAWSCredentials} import com.typesafe.config.ConfigFactory @@ -47,6 +48,8 @@ trait S3IntegrationSpec extends FlatSpecLike with BeforeAndAfterAll with Matcher val objectValue = "Some String" val metaHeaders: Map[String, String] = Map("location" -> "Africa", "datatype" -> "image") + override protected def afterAll(): Unit = TestKit.shutdownActorSystem(actorSystem) + def config() = ConfigFactory.parseString(""" |alpakka.s3.aws.region { | provider = static diff --git a/s3/src/test/scala/akka/stream/alpakka/s3/scaladsl/S3WireMockBase.scala b/s3/src/test/scala/akka/stream/alpakka/s3/scaladsl/S3WireMockBase.scala index 44fd037262..df6d80cafa 100644 --- a/s3/src/test/scala/akka/stream/alpakka/s3/scaladsl/S3WireMockBase.scala +++ b/s3/src/test/scala/akka/stream/alpakka/s3/scaladsl/S3WireMockBase.scala @@ -20,7 +20,7 @@ import com.typesafe.config.ConfigFactory abstract class S3WireMockBase(_system: ActorSystem, _wireMockServer: WireMockServer) extends TestKit(_system) { - def this(mock: WireMockServer) = + private def this(mock: WireMockServer) = this(ActorSystem(getCallerName(getClass), config(mock.port()).withFallback(ConfigFactory.load())), mock) def this() = this(initServer())