[BUG] [Java Retrofit 2] Path parameters not encoded according to OpenAPI specification

[kiwi-oss]

Bug Report Checklist

• Have you provided a full/minimal spec to reproduce the issue?
• Have you validated the input using an OpenAPI validator (example)?
• Have you tested with the latest master to confirm the issue still exists?
• Have you searched for related issues/PRs?
• What's the actual output vs expected output?
• [Optional] Sponsorship to speed up the bug fix or feature request (example)

Description

For endpoints using path parameters, the generator for Java using the Retrofit 2 library creates a client that encodes characters in the parameter value according to the definition of path segments in RFC 3986.

This means that @, : and a set of other characters are not percent-encoded while e.g. / and ? are.

This does not conform to the stricter RFC 6570 that should be used for parameter expansion in this case according to the OpenAPI 3.0.3 specification:

• The definition of parameter objects declares that path parameters should be serialized according to the simple style by default
• The simple style is defined by RFC 6570
• According to that simple string expansion, only characters in the unreserved set are allowed
• The unreserved set however contains only -, ., _, ~ as special characters.

A request containing @ in the path parameter value in fact broke for one server that received requests according to the API specification. That's how I found the issue.

Therefore, the generator should create code that expands path parameters as defined in the OpenAPI specification, i.e. according to RFC 6570, not RFC 3986. This means that anything except alphanumeric characters and -, ., _, ~ should be percent-encoded.

openapi-generator version

The issue occurs both in OpenAPI generator 4.2.3 and 5.2.1, as well as with the latest Docker image which is 5.3.0-SNAPSHOT.

OpenAPI declaration file content or url

openapi: 3.0.3
info:
  version: 0.0.1
  title: Path parameter example
servers:
  - url: localhost:8080
paths:
  "/objects/{id}":
    get:
      operationId: fetchObject
      parameters:
        - name: id
          in: path
          required: true
          schema:
            type: string
      responses:
        "200":
          description: Success

Generation Details

❯ java -jar openapi-generator-cli-5.2.1.jar generate \
        --generator-name java \
        --library retrofit2 \
        --input-spec api.yaml \
        --output java-retrofit2-path-segment-encoding-example \
        --additional-properties useRxJava=true

Steps to reproduce

1. Put the OpenAPI specification above in a file called api.yaml
2. Create a folder java-retrofit2-path-segment-encoding-example
3. Run the CLI command above
4. Modify java-retrofit2-path-segment-encoding-example/src/test/java/org/openapitools/client/api/DefaultApiTest.java such that the test method looks as follows:

   @Test
   public void fetchObjectTest() {
       api.fetchObject("idContaining@and/and?and:Characters").subscribe();
   }

5. Start Wiremock server:

   ❯ java -jar wiremock-jre8-standalone-2.31.0.jar --verbose

6. Run the test:

   ❯ cd java-retrofit2-path-segment-encoding-example && mvn test

7. Look up the request in the Wiremock output:

   2021-10-21 17:23:26.891 Request received:
   127.0.0.1 - GET /objects/idContaining@and%2Fand%3Fand:Characters
   

As can be seen, @ and : are not percent-encoded.

Related issues/PRs

• [BUG] [Ruby] client escapes all path parameters #3119 mentions making escaping optional
• [JAVA][Client][RestTemplate] Query parameters not properly URL-encoded #644 is about encoding in query parameters
• [BUG][KOTLIN] Request parameters in Kotlin client are not URL encoded #10552 concerns not encoding / in path parameters

Suggest a fix

The generated client method looks as follows:

  @GET("objects/{id}")
  Observable<Void> fetchObject(
    @retrofit2.http.Path("id") String id
  );

The documentation for @Path says that a string converter is used to convert the value, then it is URL-encoded.

Although one can customise the string converter, this doesn't sound reasonable as it's meant for type conversion, not encoding, and I suspect it's also used in other places which could cause unexpected effects.

From a superficial look in the code, the URL encoding seems to be hard-coded and not customisable:

• When building the request factory, the parameter handler is created
• The handler converts the parameter value to a string and adds it to the request builder
• When adding it, the variable is replaced with the value after encoding it according to hard-coded rules

Therefore, it seems that @Path(encoded = true) would have to be set and the parameter value encoded according to RFC 6570 before passing it to the actual client method.

[wing328]

Thanks for reporting the issue. A workaround is to use other libraries (e.g. okhttp-gson, jersey2) for the time being.

Of course, we welcome PRs to address this particular bug found in the retrofit2 library