From be1989d404b655e30447dcc4edfeeacc73842ea2 Mon Sep 17 00:00:00 2001 From: Ronald Holshausen Date: Wed, 17 Apr 2024 17:28:56 +1000 Subject: [PATCH] fix: IndexOutOfBoundsException when query param without value #1788 --- .../dius/pact/consumer/groovy/BaseBuilder.kt | 2 +- .../pact/consumer/dsl/PactDslRequestBase.kt | 2 +- .../consumer/dsl/PactDslRequestWithPath.kt | 2 +- .../com/dius/pact/core/matchers/Mismatches.kt | 4 +- .../dius/pact/core/matchers/QueryMatcher.kt | 12 +++--- .../com/dius/pact/core/model/BaseRequest.kt | 7 ++-- .../au/com/dius/pact/core/model/PactReader.kt | 28 ++++++++------ .../au/com/dius/pact/core/model/Request.kt | 4 +- .../core/model/RequestResponseInteraction.kt | 7 +++- .../com/dius/pact/core/model/V4HttpParts.kt | 2 +- .../pact/core/model/BaseRequestSpec.groovy | 38 +++++++++++++++++++ .../pact/core/model/PactReaderKtSpec.groovy | 38 +++++++++++++++++++ .../RequestResponseInteractionSpec.groovy | 12 ++++++ 13 files changed, 128 insertions(+), 30 deletions(-) create mode 100644 core/model/src/test/groovy/au/com/dius/pact/core/model/BaseRequestSpec.groovy create mode 100644 core/model/src/test/groovy/au/com/dius/pact/core/model/PactReaderKtSpec.groovy 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 index 513ebee981..82700f2a89 100644 --- 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> { + ): Map> { 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 index 2c4044b5c9..ef0a771b47 100644 --- 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> = mutableMapOf() @JvmField - var query: MutableMap> = mutableMapOf() + var query: MutableMap> = 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 index b78b0b6dfb..4e7cd02a67 100644 --- 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>, - query: MutableMap>, + query: MutableMap>, 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 index 6ad5ce52ad..3b7e4dc8f7 100755 --- 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 index 569ac4f847..6058d0a903 100644 --- 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 { return if (context.matcherDefined(listOf(parameter))) { @@ -29,8 +29,8 @@ object QueryMatcher : KLogging() { private fun compareQueryParameterValues( parameter: String, - expected: List, - actual: List, + expected: List, + actual: List, path: List, context: MatchingContext ): List { @@ -52,8 +52,8 @@ object QueryMatcher : KLogging() { @JvmStatic fun compareQuery( parameter: String, - expected: List, - actual: List, + expected: List, + actual: List, context: MatchingContext ): List { 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 index 2c757d4a11..079525c8a8 100644 --- 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> { + @JvmStatic + fun parseQueryParametersToMap(query: JsonValue?): Map> { 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 index 88c6e45c01..296bb7dc41 100644 --- 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> { +fun queryStringToMap(query: String?, decode: Boolean = true): Map> { 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>()) { 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>()) { 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 index b7440c9fd2..d89cd013ba 100644 --- 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> + val query: MutableMap> override val headers: MutableMap> 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> = mutableMapOf(), + override var query: MutableMap> = mutableMapOf(), override var headers: MutableMap> = 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 index 4d35c3610f..9f2ebfdc39 100644 --- 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 { + private fun mapToQueryStr(query: Map>): 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 index 1910d5255f..51fbef595f 100644 --- 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> { data class HttpRequest @JvmOverloads constructor( override var method: String = "GET", override var path: String = "/", - override var query: MutableMap> = mutableMapOf(), + override var query: MutableMap> = mutableMapOf(), override var headers: MutableMap> = 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 new file mode 100644 index 0000000000..1bd60b909b --- /dev/null +++ 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 new file mode 100644 index 0000000000..8200949904 --- /dev/null +++ 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 index 76628fb03d..b3b6394271 100644 --- 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: