fix: IndexOutOfBoundsException when query param without value #1788

pact-foundation · Apr 22, 2024 · a311746 · a311746
1 parent ab90ca3
commit a311746
Show file tree

Hide file tree

Showing 13 changed files with 128 additions and 30 deletions.
diff --git a/consumer/groovy/src/main/kotlin/au/com/dius/pact/consumer/groovy/BaseBuilder.kt b/consumer/groovy/src/main/kotlin/au/com/dius/pact/consumer/groovy/BaseBuilder.kt
@@ -130,7 +130,7 @@ open class BaseBuilder(
     query: Any,
     matchers: MatchingRules,
     generators: Generators
-  ): Map<String, List<String>> {
+  ): Map<String, List<String?>> {
     return if (query is Map<*, *>) {
       query.entries.associate { (key, value) ->
         when (value) {

diff --git a/consumer/src/main/kotlin/au/com/dius/pact/consumer/dsl/PactDslRequestBase.kt b/consumer/src/main/kotlin/au/com/dius/pact/consumer/dsl/PactDslRequestBase.kt
@@ -37,7 +37,7 @@ open class PactDslRequestBase(
   @JvmField
   var requestHeaders: MutableMap<String, List<String>> = mutableMapOf()
   @JvmField
-  var query: MutableMap<String, List<String>> = mutableMapOf()
+  var query: MutableMap<String, List<String?>> = mutableMapOf()
   @JvmField
   var requestBody = missing()
   @JvmField

diff --git a/consumer/src/main/kotlin/au/com/dius/pact/consumer/dsl/PactDslRequestWithPath.kt b/consumer/src/main/kotlin/au/com/dius/pact/consumer/dsl/PactDslRequestWithPath.kt
@@ -55,7 +55,7 @@ open class PactDslRequestWithPath : PactDslRequestBase {
                        path: String,
                        requestMethod: String,
                        requestHeaders: MutableMap<String, List<String>>,
-                       query: MutableMap<String, List<String>>,
+                       query: MutableMap<String, List<String?>>,
                        requestBody: OptionalBody,
                        requestMatchers: MatchingRules,
                        requestGenerators: Generators,

diff --git a/core/matchers/src/main/kotlin/au/com/dius/pact/core/matchers/Mismatches.kt b/core/matchers/src/main/kotlin/au/com/dius/pact/core/matchers/Mismatches.kt
@@ -88,8 +88,8 @@ data class MethodMismatch(val expected: String, val actual: String) : Mismatch()
 
 data class QueryMismatch(
   val queryParameter: String,
-  val expected: String,
-  val actual: String,
+  val expected: String?,
+  val actual: String?,
   val mismatch: String? = null,
   val path: String = "/"
 ) : Mismatch() {

diff --git a/core/matchers/src/main/kotlin/au/com/dius/pact/core/matchers/QueryMatcher.kt b/core/matchers/src/main/kotlin/au/com/dius/pact/core/matchers/QueryMatcher.kt
@@ -8,8 +8,8 @@ object QueryMatcher : KLogging() {
 
   private fun compare(
     parameter: String,
-    expected: String,
-    actual: String,
+    expected: String?,
+    actual: String?,
     context: MatchingContext
   ): List<QueryMismatch> {
     return if (context.matcherDefined(listOf(parameter))) {
@@ -29,8 +29,8 @@ object QueryMatcher : KLogging() {
 
   private fun compareQueryParameterValues(
     parameter: String,
-    expected: List<String>,
-    actual: List<String>,
+    expected: List<String?>,
+    actual: List<String?>,
     path: List<String>,
     context: MatchingContext
   ): List<QueryMismatch> {
@@ -52,8 +52,8 @@ object QueryMatcher : KLogging() {
   @JvmStatic
   fun compareQuery(
     parameter: String,
-    expected: List<String>,
-    actual: List<String>,
+    expected: List<String?>,
+    actual: List<String?>,
     context: MatchingContext
   ): List<QueryMismatch> {
     val path = listOf(parameter)

diff --git a/core/model/src/main/kotlin/au/com/dius/pact/core/model/BaseRequest.kt b/core/model/src/main/kotlin/au/com/dius/pact/core/model/BaseRequest.kt
@@ -1,6 +1,5 @@
 package au.com.dius.pact.core.model
 
-import au.com.dius.pact.core.support.Json
 import au.com.dius.pact.core.support.json.JsonValue
 import java.io.ByteArrayOutputStream
 import javax.mail.internet.InternetHeaders
@@ -45,12 +44,14 @@ abstract class BaseRequest : HttpPart() {
   fun isMultipartFileUpload() = determineContentType().isMultipartFormData()
 
   companion object {
-    fun parseQueryParametersToMap(query: JsonValue?): Map<String, List<String>> {
+    @JvmStatic
+    fun parseQueryParametersToMap(query: JsonValue?): Map<String, List<String?>> {
       return when (query) {
         null -> emptyMap()
         is JsonValue.Object -> query.entries.entries.associate { entry ->
           val list = when (val value = entry.value) {
-            is JsonValue.Array -> value.values.map { Json.toString(it) }
+            is JsonValue.Array -> value.values.map { it.asString() }
+            is JsonValue.StringValue -> listOf(value.toString())
             else -> emptyList()
           }
           entry.key to list

diff --git a/core/model/src/main/kotlin/au/com/dius/pact/core/model/PactReader.kt b/core/model/src/main/kotlin/au/com/dius/pact/core/model/PactReader.kt
@@ -152,22 +152,28 @@ private fun basicAuth(baseUrl: String, username: String, password: String, build
  * Parses the query string into a Map
  */
 @JvmOverloads
-fun queryStringToMap(query: String?, decode: Boolean = true): Map<String, List<String>> {
+fun queryStringToMap(query: String?, decode: Boolean = true): Map<String, List<String?>> {
   return if (query.isNullOrEmpty()) {
     emptyMap()
   } else {
     query.split("&")
-      .filter { it.isNotEmpty() }.map { val nv = it.split("=", limit = 2); nv[0] to nv[1] }
-      .fold(mutableMapOf<String, MutableList<String>>()) { map, nameAndValue ->
-      val name = if (decode) URLDecoder.decode(nameAndValue.first, "UTF-8") else nameAndValue.first
-      val value = if (decode) URLDecoder.decode(nameAndValue.second, "UTF-8") else nameAndValue.second
-      if (map.containsKey(name)) {
-        map[name]!!.add(value)
-      } else {
-        map[name] = mutableListOf(value)
+      .filter { it.isNotEmpty() }
+      .map {
+        val nv = it.split("=", limit = 2)
+        val value = if (nv.size > 1) nv[1] else null
+        nv[0] to value
+      }
+      .fold(mutableMapOf<String, MutableList<String?>>()) { map, nameAndValue ->
+        val name = if (decode) URLDecoder.decode(nameAndValue.first, "UTF-8") else nameAndValue.first
+        val value = if (nameAndValue.second != null && decode) URLDecoder.decode(nameAndValue.second, "UTF-8")
+          else nameAndValue.second
+        if (map.containsKey(name)) {
+          map[name]!!.add(value)
+        } else {
+          map[name] = mutableListOf(value)
+        }
+        map
       }
-      map
-    }
   }
 }
 

diff --git a/core/model/src/main/kotlin/au/com/dius/pact/core/model/Request.kt b/core/model/src/main/kotlin/au/com/dius/pact/core/model/Request.kt
@@ -16,7 +16,7 @@ import io.github.oshai.kotlinlogging.KLogging
 interface IRequest: IHttpPart {
   var method: String
   var path: String
-  val query: MutableMap<String, List<String>>
+  val query: MutableMap<String, List<String?>>
   override val headers: MutableMap<String, List<String>>
   override var body: OptionalBody
   override val matchingRules: MatchingRules
@@ -41,7 +41,7 @@ interface IRequest: IHttpPart {
 class Request @Suppress("LongParameterList") @JvmOverloads constructor(
   override var method: String = DEFAULT_METHOD,
   override var path: String = DEFAULT_PATH,
-  override var query: MutableMap<String, List<String>> = mutableMapOf(),
+  override var query: MutableMap<String, List<String?>> = mutableMapOf(),
   override var headers: MutableMap<String, List<String>> = mutableMapOf(),
   override var body: OptionalBody = OptionalBody.missing(),
   override var matchingRules: MatchingRules = MatchingRulesImpl(),

diff --git a/core/model/src/main/kotlin/au/com/dius/pact/core/model/RequestResponseInteraction.kt b/core/model/src/main/kotlin/au/com/dius/pact/core/model/RequestResponseInteraction.kt
@@ -137,9 +137,12 @@ open class RequestResponseInteraction @JvmOverloads constructor(
       return map
     }
 
-    private fun mapToQueryStr(query: Map<String, List<String>>): String {
+    private fun mapToQueryStr(query: Map<String, List<String?>>): String {
       return query.entries.joinToString("&") { (k, v) ->
-        v.joinToString("&") { "$k=${URLEncoder.encode(it, "UTF-8")}" }
+        v.joinToString("&") {
+          if (it != null) "$k=${URLEncoder.encode(it, "UTF-8")}"
+          else k
+        }
       }
     }
 

diff --git a/core/model/src/main/kotlin/au/com/dius/pact/core/model/V4HttpParts.kt b/core/model/src/main/kotlin/au/com/dius/pact/core/model/V4HttpParts.kt
@@ -28,7 +28,7 @@ private fun headersFromJson(json: JsonValue): Map<String, List<String>> {
 data class HttpRequest @JvmOverloads constructor(
   override var method: String = "GET",
   override var path: String = "/",
-  override var query: MutableMap<String, List<String>> = mutableMapOf(),
+  override var query: MutableMap<String, List<String?>> = mutableMapOf(),
   override var headers: MutableMap<String, List<String>> = mutableMapOf(),
   override var body: OptionalBody = OptionalBody.missing(),
   override val matchingRules: MatchingRules = MatchingRulesImpl(),

diff --git a/core/model/src/test/groovy/au/com/dius/pact/core/model/BaseRequestSpec.groovy b/core/model/src/test/groovy/au/com/dius/pact/core/model/BaseRequestSpec.groovy
@@ -0,0 +1,38 @@
+package au.com.dius.pact.core.model
+
+import au.com.dius.pact.core.support.json.JsonParser
+import au.com.dius.pact.core.support.json.JsonValue
+import spock.lang.Specification
+
+class BaseRequestSpec extends Specification {
+  def 'parseQueryParametersToMap'() {
+    expect:
+    BaseRequest.parseQueryParametersToMap(json) == value
+
+    where:
+
+    json                                 | value
+    null                                 | [:]
+    JsonValue.Null.INSTANCE              | [:]
+    JsonValue.True.INSTANCE              | [:]
+    JsonValue.False.INSTANCE             | [:]
+    new JsonValue.Integer(100)           | [:]
+    new JsonValue.Decimal(100.0)         | [:]
+    new JsonValue.Array([])              | [:]
+    new JsonValue.StringValue('a=1&b=2') | [a: ['1'], b: ['2']]
+  }
+
+  def 'parseQueryParametersToMap - with a JSON map'() {
+    expect:
+    BaseRequest.parseQueryParametersToMap(JsonParser.parseString(json).asObject()) == value
+
+    where:
+
+    json                | value
+    '{}'                | [:]
+    '{"a": "1"}'        | [a: ['1']]
+    '{"a": ["1"]}'      | [a: ['1']]
+    '{"a": ["", ""]}'   | [a: ['', '']]
+    '{"a": [null, ""]}' | [a: [null, '']]
+  }
+}
diff --git a/core/model/src/test/groovy/au/com/dius/pact/core/model/PactReaderKtSpec.groovy b/core/model/src/test/groovy/au/com/dius/pact/core/model/PactReaderKtSpec.groovy
@@ -0,0 +1,38 @@
+package au.com.dius.pact.core.model
+
+import spock.lang.Issue
+import spock.lang.Specification
+
+import static au.com.dius.pact.core.model.PactReaderKt.queryStringToMap
+
+class PactReaderKtSpec extends Specification {
+  def 'parsing a query string'() {
+    expect:
+    queryStringToMap(query) == result
+
+    where:
+
+    query          | result
+    null           | [:]
+    ''             | [:]
+    'p=1'          | [p: ['1']]
+    'p=1&q=2'      | [p: ['1'], q: ['2']]
+    'p=1&q=2&p=3'  | [p: ['1', '3'], q: ['2']]
+    'p=1&q=2=&p=3' | [p: ['1', '3'], q: ['2=']]
+    '&&'           | [:]
+  }
+
+  @Issue('#1788')
+  def 'parsing a query string with empty or missing values'() {
+    expect:
+    queryStringToMap(query) == result
+
+    where:
+
+    query        | result
+    'p='         | [p: ['']]
+    'p=1&q=&p=3' | [p: ['1', '3'], q: ['']]
+    'p&q=1&q=2'  | [p: [null], q: ['1', '2']]
+    'p&p&p'      | [p: [null, null, null]]
+  }
+}
diff --git a/core/model/src/test/groovy/au/com/dius/pact/core/model/RequestResponseInteractionSpec.groovy b/core/model/src/test/groovy/au/com/dius/pact/core/model/RequestResponseInteractionSpec.groovy
@@ -135,6 +135,18 @@ class RequestResponseInteractionSpec extends Specification {
       'include[]=course_image&include[]=favorites'
   }
 
+  @Issue('#1788')
+  def 'correctly encodes the query parameters with no values or empty ones'() {
+    given:
+    request.query = [p: [null, null, null], q: ['', '', '']]
+
+    when:
+    def map = interaction.toMap(PactSpecVersion.V2)
+
+    then:
+    map.request.query == 'p&p&p&q=&q=&q='
+  }
+
   @Issue('#1611')
   def 'supports empty bodies'() {
     expect: