[BUG][Java][Client][Feign] does not properly encode query parameters when using a parameter map
Created by: davide-maestroni
Bug Report Checklist
-
Have you provided a full/minimal spec to reproduce the issue? -
Have you validated the input using an OpenAPI validator (example)? -
What's the version of OpenAPI Generator used? -
Have you search for related issues/PRs? -
What's the actual output vs expected output?
Description
The generate Feign Java client does not properly encode query parameters when using a parameter map.
openapi-generator version
4.1.0
OpenAPI declaration file content or url
Test declaration file:
openapi: '3.0'
info:
title: Feign parameter map issue
description: Feign parameter map issue test
version: '0.0.1'
servers:
- url: https://example.com
paths:
/names:
get:
operationId: getNames
description: Returns the resource names.
parameters:
- name: query
in: query
description: A free text search query.
style: simple
schema:
type: string
minLength: 2
tags:
- NamesGet
responses:
200:
description: The name list.
content:
application/json:
schema:
type: array
items:
type: string
default:
description: Unexpected error.
content:
application/json:
schema:
type: string
Command line used for generation
java -cp openapi-generator-cli-4.1.0.jar org.openapitools.codegen.OpenAPIGenerator generate -i paramMapIssue.yaml -g java --library feign --additional-properties "feignVersion=10.x" -o src
Steps to reproduce
Here is a small unit test to reproduce the issue:
@Test
public void paramMapIssue() throws IOException, InterruptedException {
MockWebServer server = new MockWebServer();
server.enqueue(new MockResponse().setResponseCode(200).setBody("[]"));
server.enqueue(new MockResponse().setResponseCode(200).setBody("[]"));
server.start();
try {
String endpoint = "http://localhost:" + server.getPort();
NamesGetApi namesGetApi = new ApiClient().setBasePath(endpoint).buildClient(NamesGetApi.class);
{
namesGetApi.getNames("this is a test");
RecordedRequest recordedRequest = server.takeRequest();
assertThat(recordedRequest.getRequestUrl().queryParameterValues("query"))
.containsExactly("this is a test");
}
{
namesGetApi.getNames(new GetNamesQueryParams().query("this is a test"));
RecordedRequest recordedRequest = server.takeRequest();
assertThat(recordedRequest.getRequestUrl().queryParameterValues("query"))
.containsExactly("this is a test");
}
} finally {
server.shutdown();
}
}
The first assert is successful while the second fails. The printed logs:
INFO: MockWebServer[50764] starting to accept connections
Aug 13, 2019 6:07:07 PM okhttp3.mockwebserver.MockWebServer$SocketHandler processOneRequest
INFO: MockWebServer[50764] received request: GET /names?query=this%20is%20a%20test HTTP/1.1 and responded: HTTP/1.1 200 OK
Aug 13, 2019 6:07:07 PM okhttp3.mockwebserver.MockWebServer$SocketHandler processOneRequest
INFO: MockWebServer[50764] received request: GET /names?query=this%2Bis%2Ba%2Btest HTTP/1.1 and responded: HTTP/1.1 200 OK
Aug 13, 2019 6:07:07 PM okhttp3.mockwebserver.MockWebServer acceptConnections
INFO: MockWebServer[50764] done accepting connections: Socket closed
As you can see, when the parameter map is used, the blank spaces in the string are converted to a + sign and then escaped with %2B
, which results in an incorrect decoding on the server side.
As per Feign documentation:
What about plus? +
Per the URI specification, a + sign is allowed in both the path and query segments of a URI, however, handling of the symbol on the query can be inconsistent. In some legacy systems, the + is equivalent to the a space. Feign takes the approach of modern systems, where a + symbol should not represent a space and is explicitly encoded as %2B when found on a query string.
If you wish to use + as a space, then use the literal character or encode the value directly as %5.0.0
Suggest a fix
Even if not very elegant, a possible solution is to add .replaceAll("//+", "%20")
every time the java.net.URLEncoder
is used in the EncodingUtils
file.