diff --git a/conjure-java-jaxrs-client/src/main/java/com/palantir/conjure/java/client/jaxrs/DialogueFeignClient.java b/conjure-java-jaxrs-client/src/main/java/com/palantir/conjure/java/client/jaxrs/DialogueFeignClient.java index de98b3118..54eca4768 100644 --- a/conjure-java-jaxrs-client/src/main/java/com/palantir/conjure/java/client/jaxrs/DialogueFeignClient.java +++ b/conjure-java-jaxrs-client/src/main/java/com/palantir/conjure/java/client/jaxrs/DialogueFeignClient.java @@ -72,7 +72,7 @@ final class DialogueFeignClient implements feign.Client { private static final String REQUEST_URL_PATH_PARAM = "request-url"; private static final Splitter PATH_SPLITTER = Splitter.on('/'); private static final Splitter QUERY_SPLITTER = Splitter.on('&').omitEmptyStrings(); - private static final Splitter QUERY_VALUE_SPLITTER = Splitter.on('='); + private static final char QUERY_KEY_VALUE_SEPARATOR = '='; private final ConjureRuntime runtime; private final Channel channel; @@ -386,6 +386,7 @@ private final class FeignEndpoint implements Endpoint { } @Override + @SuppressWarnings("CyclomaticComplexity") public void renderPath(ListMultimap params, UrlBuilder url) { List requestUrls = params.get(REQUEST_URL_PATH_PARAM); Preconditions.checkState( @@ -417,14 +418,17 @@ public void renderPath(ListMultimap params, UrlBuilder url) { if (queryParamsStart != -1) { String querySegments = trailing.substring(queryParamsStart + 1); for (String querySegment : QUERY_SPLITTER.split(querySegments)) { - List keyValuePair = QUERY_VALUE_SPLITTER.splitToList(querySegment); - if (keyValuePair.size() != 2) { + int equalsIndex = querySegment.indexOf(QUERY_KEY_VALUE_SEPARATOR); + if (equalsIndex > 0) { + String key = querySegment.substring(0, equalsIndex); + String value = querySegment.substring(equalsIndex + 1); + url.queryParam(urlDecode(key), urlDecode(value)); + } else { throw new SafeIllegalStateException( "Expected two parameters", - SafeArg.of("parameters", keyValuePair.size()), - UnsafeArg.of("values", keyValuePair)); + SafeArg.of("parameters", equalsIndex == -1 ? 0 : 1), + UnsafeArg.of("values", querySegment)); } - url.queryParam(urlDecode(keyValuePair.get(0)), urlDecode(keyValuePair.get(1))); } } } diff --git a/conjure-java-jaxrs-client/src/test/java/com/palantir/conjure/java/client/jaxrs/JaxRsClientDialogueEndpointTest.java b/conjure-java-jaxrs-client/src/test/java/com/palantir/conjure/java/client/jaxrs/JaxRsClientDialogueEndpointTest.java index 1395d4463..fe1bbe64f 100644 --- a/conjure-java-jaxrs-client/src/test/java/com/palantir/conjure/java/client/jaxrs/JaxRsClientDialogueEndpointTest.java +++ b/conjure-java-jaxrs-client/src/test/java/com/palantir/conjure/java/client/jaxrs/JaxRsClientDialogueEndpointTest.java @@ -16,10 +16,13 @@ package com.palantir.conjure.java.client.jaxrs; +import static com.palantir.logsafe.testing.Assertions.assertThatLoggableExceptionThrownBy; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.assertj.core.api.InstanceOfAssertFactories.collection; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -35,6 +38,9 @@ import com.palantir.dialogue.Request; import com.palantir.dialogue.Response; import com.palantir.dialogue.UrlBuilder; +import com.palantir.logsafe.SafeArg; +import com.palantir.logsafe.UnsafeArg; +import com.palantir.logsafe.exceptions.SafeIllegalStateException; import jakarta.ws.rs.Consumes; import jakarta.ws.rs.GET; import jakarta.ws.rs.POST; @@ -95,6 +101,63 @@ public void testQueryParameterCollection() { verify(urlBuilder).queryParam("query", "a+b"); } + @Test + public void testQueryParameters() { + Channel channel = stubNoContentResponseChannel(); + StubService service = JaxRsClient.create(StubService.class, channel, runtime); + service.collectionOfQueryParams(List.of("", "=", "value", "test=value")); + + ArgumentCaptor endpointCaptor = ArgumentCaptor.forClass(Endpoint.class); + ArgumentCaptor requestCaptor = ArgumentCaptor.forClass(Request.class); + verify(channel).execute(endpointCaptor.capture(), requestCaptor.capture()); + assertThat(requestCaptor.getValue().pathParameters().asMap()) + .hasSize(1) + .extractingByKey("request-url") + .asInstanceOf(collection(String.class)) + .containsExactly("dialogue://feign/foo/params?query=&query=%3D&query=value&query=test%3Dvalue"); + Endpoint endpoint = endpointCaptor.getValue(); + UrlBuilder urlBuilder = mock(UrlBuilder.class); + endpoint.renderPath( + ImmutableListMultimap.of( + "request-url", "dialogue://feign/foo/params?query=&query==&query=value&query=test=value"), + urlBuilder); + verify(urlBuilder).queryParam("query", ""); + verify(urlBuilder).queryParam("query", "="); + verify(urlBuilder).queryParam("query", "test=value"); + } + + @Test + public void testInvalidQueryParameters() { + Channel channel = stubNoContentResponseChannel(); + StubService service = JaxRsClient.create(StubService.class, channel, runtime); + service.collectionOfQueryParams(List.of()); + + ArgumentCaptor endpointCaptor = ArgumentCaptor.forClass(Endpoint.class); + ArgumentCaptor requestCaptor = ArgumentCaptor.forClass(Request.class); + verify(channel).execute(endpointCaptor.capture(), requestCaptor.capture()); + Endpoint endpoint = endpointCaptor.getValue(); + UrlBuilder urlBuilder = mock(UrlBuilder.class); + assertThatLoggableExceptionThrownBy(() -> endpoint.renderPath( + ImmutableListMultimap.of("request-url", "dialogue://feign/foo/params?="), urlBuilder)) + .isInstanceOf(SafeIllegalStateException.class) + .hasLogMessage("Expected two parameters") + .args() + .containsExactlyInAnyOrder(SafeArg.of("parameters", 1), UnsafeArg.of("values", "=")); + assertThatLoggableExceptionThrownBy(() -> endpoint.renderPath( + ImmutableListMultimap.of("request-url", "dialogue://feign/foo/params?=value"), urlBuilder)) + .isInstanceOf(SafeIllegalStateException.class) + .hasLogMessage("Expected two parameters") + .args() + .containsExactlyInAnyOrder(SafeArg.of("parameters", 1), UnsafeArg.of("values", "=value")); + assertThatLoggableExceptionThrownBy(() -> endpoint.renderPath( + ImmutableListMultimap.of("request-url", "dialogue://feign/foo/params?query"), urlBuilder)) + .isInstanceOf(SafeIllegalStateException.class) + .hasLogMessage("Expected two parameters") + .args() + .containsExactlyInAnyOrder(SafeArg.of("parameters", 0), UnsafeArg.of("values", "query")); + verify(urlBuilder, never()).queryParam(any(), any()); + } + @Test public void testPostWithBody() { Channel channel = stubNoContentResponseChannel();