[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.