Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG][Java][Client][Feign] does not properly encode query parameters when using a parameter map #3635

Closed
5 tasks done
davide-maestroni opened this issue Aug 13, 2019 · 2 comments · Fixed by #3948
Closed
5 tasks done

Comments

@davide-maestroni
Copy link
Contributor

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 %20

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.

@auto-labeler
Copy link

auto-labeler bot commented Aug 13, 2019

👍 Thanks for opening this issue!
🏷 I have applied any labels matching special text in your issue.

The team will review the labels and make any necessary changes.

@macjohnny macjohnny changed the title [BUG][Java][Client] [BUG][Java][Client][Feign] does not properly encode query parameters when using a parameter map Aug 14, 2019
@macjohnny
Copy link
Member

@davide-maestroni do you want to provide a fix for this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants