spring-cloud-openfeign icon indicating copy to clipboard operation
spring-cloud-openfeign copied to clipboard

Matrix parameters are not encoded correctly

Open nickcodefresh opened this issue 4 years ago • 14 comments

(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.

nickcodefresh avatar Aug 17 '20 16:08 nickcodefresh

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 😕

oliverlockwood avatar Aug 24 '20 12:08 oliverlockwood

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

nickcodefresh avatar Sep 14 '20 09:09 nickcodefresh

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

OlgaMaciaszek avatar Sep 16 '20 13:09 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.

OlgaMaciaszek avatar Sep 16 '20 17:09 OlgaMaciaszek

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 avatar Sep 21 '20 15:09 nickcodefresh

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

nickcodefresh avatar Sep 29 '20 11:09 nickcodefresh

@OlgaMaciaszek any update on this, please?

oliverlockwood avatar Oct 08 '20 13:10 oliverlockwood

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

OlgaMaciaszek avatar Oct 12 '20 14:10 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 avatar Oct 26 '20 17:10 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.

OlgaMaciaszek avatar Oct 28 '20 16:10 OlgaMaciaszek

Hi @OlgaMaciaszek, any update on this?

nickcodefresh avatar Nov 09 '20 11:11 nickcodefresh

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 avatar Nov 26 '20 10:11 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 avatar Nov 26 '20 15:11 OlgaMaciaszek

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

OlgaMaciaszek avatar Nov 26 '20 15:11 OlgaMaciaszek