fix: default headers were being added twice to requests to the pact b…

…roker #1242

core/model/src/main/kotlin/au/com/dius/pact/core/model/PactReader.kt

@@ -352,11 +352,11 @@ object DefaultPactReader : PactReader, KLogging() {
     } else if (source is InputStream || source is Reader || source is File) {
       return loadPactFromFile(source)
     } else if (source is BrokerUrlSource) {
-      return HttpClient.newHttpClient(options["authentication"], URI(source.pactBrokerUrl), mutableMapOf()).first.use {
+      return HttpClient.newHttpClient(options["authentication"], URI(source.pactBrokerUrl)).first.use {
         loadPactFromUrl(source, options, it)
       }
     } else if (source is PactBrokerResult) {
-      return HttpClient.newHttpClient(options["authentication"], URI(source.pactBrokerUrl), mutableMapOf()).first.use {
+      return HttpClient.newHttpClient(options["authentication"], URI(source.pactBrokerUrl)).first.use {
         loadPactFromUrl(BrokerUrlSource.fromResult(source, options, source.tag), options, it)
       }
     } else if (source is URL || source is UrlPactSource) {

core/pactbroker/src/main/kotlin/au/com/dius/pact/core/pactbroker/HalClient.kt

@@ -196,8 +196,8 @@ open class HalClient @JvmOverloads constructor(
         HttpClient.logger.warn { "Authentication options needs to be a list of values, ignoring." }
       }
       val uri = URI(baseUrl)
-      val result = HttpClient.newHttpClient(options["authentication"], uri, defaultHeaders,
-        this.maxPublishRetries, this.publishRetryInterval)
+      val result = HttpClient.newHttpClient(options["authentication"], uri, this.maxPublishRetries,
+        this.publishRetryInterval)
       httpClient = result.first
 
       if (System.getProperty(PREEMPTIVE_AUTHENTICATION) == "true") {

core/pactbroker/src/main/kotlin/au/com/dius/pact/core/pactbroker/util/HttpClientUtils.kt

@@ -38,8 +38,13 @@ object HttpClientUtils {
   }
 
   fun pathCombiner(builder: URIBuilder, url: String): URI {
-    return if (builder.getPath() != null) {
-      builder.setPath(builder.getPath() + url).build()
+    val path = builder.path
+    return if (path != null) {
+      if (path.endsWith("/") && url.startsWith("/")) {
+        builder.setPath(path.trimEnd('/') + url).build()
+      } else {
+        builder.setPath(path + url).build()
+      }
     } else {
       builder.setPath(url).build()
     }

core/pactbroker/src/test/groovy/au/com/dius/pact/core/pactbroker/HalClientSpec.groovy

@@ -9,13 +9,11 @@ import org.apache.http.HttpHost
 import org.apache.http.HttpResponse
 import org.apache.http.ProtocolVersion
 import org.apache.http.client.methods.CloseableHttpResponse
-import org.apache.http.client.methods.HttpGet
 import org.apache.http.client.methods.HttpPost
 import org.apache.http.entity.ContentType
 import org.apache.http.entity.StringEntity
 import org.apache.http.impl.client.BasicCredentialsProvider
 import org.apache.http.impl.client.CloseableHttpClient
-import org.apache.http.impl.client.SystemDefaultCredentialsProvider
 import org.apache.http.message.BasicHeader
 import org.apache.http.message.BasicStatusLine
 import spock.lang.Shared
@@ -67,21 +65,6 @@ class HalClientSpec extends Specification {
     client.httpContext == null
   }
 
-  def 'For bearer token authentication scheme adds an authorization header to all requests'() {
-    given:
-    client.options = [authentication: ['bearer', '1234']]
-
-    when:
-    client.setupHttpClient()
-    def request = client.initialiseRequest(new HttpGet('/'))
-
-    then:
-    client.httpClient.credentialsProvider instanceof SystemDefaultCredentialsProvider
-    client.httpContext == null
-    client.defaultHeaders == [Authorization: 'Bearer 1234']
-    request.getFirstHeader('Authorization').value == 'Bearer 1234'
-  }
-
   @RestoreSystemProperties
   def 'populates the auth cache if preemptive authentication system property is enabled'() {
     given:

core/pactbroker/src/test/groovy/au/com/dius/pact/core/pactbroker/util/HttpClientUtilsSpec.groovy

@@ -24,5 +24,4 @@ class HttpClientUtilsSpec extends Specification {
     'no port'                 | 'http://localhost'      | '/path/with spaces'                      | 'http://localhost/path/with%20spaces'
     'Extra path'              | 'http://localhost/sub'  | '/extraPath/with spaces'                 | 'http://localhost/sub/extraPath/with%20spaces'
   }
-
 }

core/support/src/main/kotlin/au/com/dius/pact/core/support/HttpClient.kt

@@ -39,19 +39,19 @@ object HttpClient : KLogging() {
   fun newHttpClient(
     authentication: Any?,
     uri: URI,
-    defaultHeaderStore: MutableMap<String, String>,
     maxPublishRetries: Int = 5,
     publishRetryInterval: Int = 3000
   ): Pair<CloseableHttpClient, CredentialsProvider?> {
     val retryStrategy = CustomServiceUnavailableRetryStrategy(maxPublishRetries, publishRetryInterval)
     val builder = HttpClients.custom().useSystemProperties().setServiceUnavailableRetryStrategy(retryStrategy)
 
+    val defaultHeaders = mutableMapOf<String, String>()
     val credsProvider = when (authentication) {
       is Auth -> {
         when (authentication) {
           is Auth.BasicAuthentication -> basicAuth(uri, authentication.username, authentication.password, builder)
           is Auth.BearerAuthentication -> {
-            defaultHeaderStore["Authorization"] = "Bearer " + authentication.token
+            defaultHeaders["Authorization"] = "Bearer " + authentication.token
             SystemDefaultCredentialsProvider()
           }
         }
@@ -68,7 +68,7 @@ object HttpClient : KLogging() {
           }
           "bearer" -> {
             if (authentication.size > 1) {
-              defaultHeaderStore["Authorization"] = "Bearer " + authentication[1].toString()
+              defaultHeaders["Authorization"] = "Bearer " + authentication[1].toString()
             } else {
               logger.warn { "Bearer token authentication requires a token, ignoring." }
             }
@@ -83,7 +83,7 @@ object HttpClient : KLogging() {
       else -> SystemDefaultCredentialsProvider()
     }
 
-    builder.setDefaultHeaders(defaultHeaderStore.map { BasicHeader(it.key, it.value) })
+    builder.setDefaultHeaders(defaultHeaders.map { BasicHeader(it.key, it.value) })
     return builder.build() to credsProvider
   }
 

core/support/src/test/groovy/au/com/dius/pact/core/support/HttpClientSpec.groovy

@@ -0,0 +1,20 @@
+package au.com.dius.pact.core.support
+
+import spock.lang.Specification
+
+class HttpClientSpec extends Specification {
+
+  def 'when creating a new http client, add any authentication as default headers'() {
+    given:
+    URI uri = new URI('http://localhost')
+    def authentication = ['bearer', '1234abcd']
+
+    when:
+    def result = HttpClient.INSTANCE.newHttpClient(authentication, uri, 1, 1)
+    def defaultHeaders = result.component1().closeables[0].this$0.defaultHeaders
+
+    then:
+    defaultHeaders[0].name == 'Authorization'
+    defaultHeaders[0].value == 'Bearer 1234abcd'
+  }
+}

gradle.properties

@@ -3,7 +3,7 @@ groovy2Version=2.5.10
 kotlinVersion=1.3.72
 httpBuilderVersion=1.0.4
 commonsLang3Version=3.4
-httpClientVersion=4.5.5
+httpClientVersion=4.5.13
 scalaVersion=2.13.2
 specs2Version=4.9.4
 scalaTestVersion=3.1.2

provider/junit5spring/src/test/java/au/com/dius/pact/provider/spring/junit5/FooControllerTest.java

@@ -0,0 +1,35 @@
+package au.com.dius.pact.provider.spring.junit5;
+
+import au.com.dius.pact.provider.junit5.PactVerificationContext;
+import au.com.dius.pact.provider.junit5.PactVerificationInvocationContextProvider;
+import au.com.dius.pact.provider.junitsupport.IgnoreNoPactsToVerify;
+import au.com.dius.pact.provider.junitsupport.Provider;
+import au.com.dius.pact.provider.junitsupport.loader.PactBroker;
+import au.com.dius.pact.provider.junitsupport.loader.PactBrokerAuth;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.TestTemplate;
+import org.junit.jupiter.api.extension.ExtendWith;
+import org.springframework.boot.test.context.SpringBootTest;
+import org.springframework.boot.web.server.LocalServerPort;
+
+/*
+  This is a test for issue https://github.com/pact-foundation/pact-jvm/issues/1242
+ */
+@SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT)
+@Provider("Foo Provider")
+@PactBroker(scheme="https", host = "test.pactflow.io", port = "443", authentication = @PactBrokerAuth(token = "anyToken"))
+@IgnoreNoPactsToVerify(ignoreIoErrors = "true")
+class FooControllerTest {
+
+  @LocalServerPort
+  int port;
+
+  @BeforeEach
+  void setup(PactVerificationContext context) {
+  }
+
+  @TestTemplate
+  @ExtendWith(PactVerificationInvocationContextProvider.class)
+  void pactVerificationTestTemplate(PactVerificationContext context) {
+  }
+}