feign: Change of URL encoding behaviour doesn't seem right

This issue is probably related to #1189 but is focused on the encoding of path variables instead. We’ve recently upgraded to Spring Cloud to Hoxton.SR3 which includes a bump of Spring Cloud OpenFeign from 2.2.1.RELEASE to 2.2.2.RELEASE. This patch version bumps OpenFeign from 10.4.0 to 10.7.4 which seems to contain quite a few important changes.

In particular, there is a change of behaviour around url encoding (#1138) which in our case has broken a couple of things with using path variables. I’ve created a sandbox project at https://github.com/sguillope/openfeign-url-encoding-issue to make it easier to reproduce the issue.

The 2 problems we’ve hit are as follows when using a path variable:

The colon : character now gets url-encoded to %3A. While not really incorrect, it’s not actually required as a colon is a valid character in a path segment (https://tools.ietf.org/html/rfc3986#section-3.3), except for one case which shouldn’t apply here. This only broke our integration tests, not production code, so we can easily fix that.
The most problematic one is the handling of the forward slash / character. With the latest version, it doesn’t get url-encoded in path variables. Fortunately this was caught by our integration tests, otherwise it would have broken our production code. We can’t pre-encode it because it will then get double-encoded as %252F instead of %2F.

To summarise:

Input	Result	Expected
`a:path/variable`	`a%3Apath/variable`	`a:path%2Fvariable`
`path/variable`	`path/variable`	`path%2Fvariable`
`a:path`	`a%3Apath`	`a:path`

Let me know if you need more details. Thanks

About this issue

Original URL
State: open
Created 4 years ago
Reactions: 1
Comments: 16 (5 by maintainers)

Commits related to this issue

feat(JF-520): Fix Feign escaping colons (github.com/OpenFeign/feign/issues/1190) — committed to levigo/neverpile-eureka-java-client by Crow08 a year ago

Most upvoted comments

Hi @kdavisk6, thanks for the thorough reply, much appreciated.

Unfortunately because we use Spring Cloud OpenFeign, I believe there isn’t much control we have over the decodeSlash parameter (arguably that’s a Spring Cloud problem)

That said, I’ve added more test cases to my test project sguillope/openfeign-url-encoding-issue with using pre-encoded path variables.

Test Case	Input	Result	Expected
Encoded `/`	`path%2Fvariable`	`path/variable`	`path%2Fvariable`
Encoded `:` and `/`	`a%3Apath%2Fvariable`	`a%3Apath/variable`	`a%3Apath%2Fvariable`
Unencoded `:` and encoded `/`	`a:path%2Fvariable`	`a%3Apath%252Fvariable`	`a:path%2Fvariable`
Encoded `:` and unencoded `/`	`a%3Apath/variable`	`a%253Apath/variable`	?

For some reason, when Feign detects the variable as already being encoded, it ends up decoding %2F back to /.

As you can see without control over the decodeSlash behaviour it’s impossible to get the / to be encoded.

sguillope on Mar 16, 2020

Same issue here, in my case I was trying to integrate with Google Analitycs 4, they require parameters with “:” in the requests.

I solved it by changing this

@FeignClient
interface AwesomeClient {

    @PostMapping("/awesome_request")
    fun request(
        @RequestParam("param_with_colon") param: String
    )
}

To this

@FeignClient
interface AwesomeClient {

    @PostMapping("/awesome_request")
    fun request(
        @SpringQueryMap(encoded = true) params: ParamWithColon
    )
}

class ParamWithColon(
    val param_with_colon: String 
)

apereznavarro99 on Jan 25, 2022

this behaviour is also true when you have either a path parameter or a query parameter that might be base64 encoded which usually contains a = sign that is swapped by %3D and may change the outcome. for now, the solution for this is to have a request interceptor

kotlin

...
val URL_ENCODED_EQUAL_SIGN: String = "%3D"
...

class UrlEncodingFixInterceptor : RequestInterceptor {
    override fun apply(template: RequestTemplate) {
        template.uri(template.url().replace(URL_ENCODED_EQUAL_SIGN, "="))
    }
}

val feignBuilder = HystrixFeign.builder().....requestInterceptor(UrlEncodingFixInterceptor())...

AndreBritoGit on Aug 17, 2022

Hello, can I ask you is here any progress?

honzastefan on Dec 7, 2021

So there is no way to specify that one does not want to encode : in a path-param? 🤔

Klaboe on Mar 23, 2020