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

Path parameter having special characters like brackets and parentheses is not encoded properly in retrofit2 #4312

Open
kirangodishala opened this issue Mar 5, 2025 · 1 comment

Comments

@kirangodishala
Copy link

kirangodishala commented Mar 5, 2025

I am from Spinnaker community. We have recently migrated some of our services from retrofit1 to retrofit2. We have hundreds of apis overall.
After migration we are facing challenges with path parameter encoding.

With retrofit1, the path param [foo] b:ar (baz) used to be encoded to %5Bfoo%5D%20b%3Aar%20%28baz%29 and api has no issues.
But after migrating to retrofit2, the encoded value became [foo]%20b:ar%20(baz) and api rejects it with 400.

Is there any plan to fix this issue? What is the work around you suggest?

@JakeWharton
Copy link
Collaborator

RFC 3986 section 3.3 defines a path as comprised of pchars:

pchar         = unreserved / pct-encoded / sub-delims / ":" / "@"

which then references

unreserved    = ALPHA / DIGIT / "-" / "." / "_" / "~"
sub-delims    = "!" / "$" / "&" / "'" / "(" / ")"
                 / "*" / "+" / "," / ";" / "="

So we are not required to encode parenthesis, but are supposed to encode square brackets.

However, this behavior is inherited from OkHttp which current does not encode square brackets per square/okhttp#6790. Since we are built OkHttp, we inherit its behavior. Now technically that's not true for path encoding as we copy OkHttp's implementation, but otherwise use its HttpUrl.Builder to construct HTTP URLs. The illusion is that we use OkHttp's HttpUrl.Builder for everything, however, so I would prefer to align with its behavior.

Perhaps you could plead your case on their issue, and then once OkHttp changes we will change as well.

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

No branches or pull requests

2 participants