Matrix parameters are not encoded correctly

[nickcodefresh]

(This leads on from https://github.com/spring-cloud/spring-cloud-openfeign/issues/334 - apologies but at the time I didn't have enough spare time to create a sample application, but I now have).

When I try to use matrix parameters and Hoxton.SR7, they are encoded incorrectly. For example, assuming I have the folloing Feign client interface:

@FeignClient(url = "http://localhost:8080", name="name")
public interface FeignDemo {
    @GetMapping(value = "/api/server{account}")
    void server(@MatrixVariable("account") String account);
}

when making requests the URL is resolved as http://localhost:8080/api/server%3Baccount%3Da rather than the expected http://localhost:8080/api/server;account=a`:

2020-08-17 17:01:45.519 ERROR 23616 --- [nio-8080-exec-2] o.a.c.c.C.[.[.[/].[dispatcherServlet]    : Servlet.service() for servlet [dispatcherServlet] in context with path [] threw exception [Request processing failed; nested exception is feign.FeignException$BadRequest: [400] during [GET] to [http://localhost:8080/api/server%3Baccount%3Da] [FeignDemo#server(String)]: [{"timestamp":"2020-08-17T16:01:45.484+00:00","status":400,"error":"Bad Request","message":"","path":"/api/server%3Baccount%3Da"}]] with root cause

feign.FeignException$BadRequest: [400] during [GET] to [http://localhost:8080/api/server%3Baccount%3Da] [FeignDemo#server(String)]: [{"timestamp":"2020-08-17T16:01:45.484+00:00","status":400,"error":"Bad Request","message":"","path":"/api/server%3Baccount%3Da"}]

I have written the sample project https://github.com/nickcodefresh/spring-cloud-openfeign-334 to demo this issue. If you start the application and hit http://localhost:8080/api/client you'll see the error.

[oliverlockwood]

See also #307.

@OlgaMaciaszek is there any chance of this getting looked at any time soon? To be clear it's been a problem in all versions of Spring-Cloud since Hoxton.SR2 so we are currently stuck on that version despite the 5 subsequent releases 😕

[nickcodefresh]

@OlgaMaciaszek any update on this? Unfortunately the project I'm working on uses matrix parameters extensively and this has become a upgrade blocker.

[OlgaMaciaszek]

@oliverlockwood @nickcodefresh will take a look at it now.

[OlgaMaciaszek]

@nickcodefresh this example does not work if I just curl http://localhost:8080/api/server;account=a either (sends a 404). Please provide a demo that behaves fine with positive scenario, but fails with the encoded version. @oliverlockwood you, too, can provide a sample that reproduces the issue if you want me to have a look at it asap.

[nickcodefresh]

Hi Olga,

I have now updated my example project the demonstrate the issue. It turns out that if a Feign interface calls a Spring Boot controller, then everything is fine. If a Feign interface calls an external web server, then this is when you see the issue.

To see this, run com.example.matrixdemo.MatrixdemoApplication in my example project and then execute the unit tests class com.ooyala.matrixdemo.FeignDemoTest.

The example Spring Boot project provides 2 sets of end-points: the client controller, which calls a web server via Feign, and a server controller that receives the requests. When you run the unit tests, a WireMock server is created, which has the same signiture as the Spring Boot server controller.

Client Request	Server	Server Request	Result
/client/internalMatrixParamsMap	Spring Boot	/server/matrixParamsMap{matrixVars}	PASS
/client/internalMatrixParams	Spring Boot	/server/matrixParam{account}{name}	PASS
/client/wiremockMatrixParamsMap	WireMock	/server/matrixParams;account=a;name=n	FAIL
/client/wiremockMatrixParams	WireMock	/server/matrixParams;account=a;name=n	FAIL

The request that is made from the client should be /server/matrixParams;account=a;name=n but due to the encoding this becomes /server/matrixParams%3Baccount%3Da%3Bname%3D. I'm guessing that Spring has the abilty to unencode the matrix parameters, unlike a non-Spring service. Also note that request even before it's encoded is /server/matrixParams;name=[n];account=[a], the square brackets obviously being incorrect.

[nickcodefresh]

Can you confirm @OlgaMaciaszek that the example provided now demonstrates the issuue for you?

[oliverlockwood]

@OlgaMaciaszek any update on this, please?

[OlgaMaciaszek]

Thanks @nickcodefresh. Will have a look at it this week.

[OlgaMaciaszek]

Hi @nickcodefresh, sorry for not coming back to you earlier - have ended up being out of office for some time. Reviewing it now.

[OlgaMaciaszek]

Thanks for the updated sample. I was able to reproduce the issue. In fact, in light of the RFC-3986,, I agree that those characters should not be encoded. Will work on fixing the issue next week.

[nickcodefresh]

Hi @OlgaMaciaszek, any update on this?

[OlgaMaciaszek]

Hi @nickcodefresh . Really sorry for the delay. Had to finish some things that needed to make it to the M5 release. However, I'm working on it now and will add an update here soon.

[OlgaMaciaszek]

@nickcodefresh Have gone through the code. The encoding takes place in feign-core and it will probably require some changes there to get this issue fixed. I will create an issue in the core OpenFeign repo.

[OlgaMaciaszek]

Issue created: https://github.com/OpenFeign/feign/issues/1319.