OpenFeign / feign

Feign makes writing java http clients easier
Apache License 2.0
9.51k stars 1.93k forks source link

Encoding issue for query parameter #1133

Closed ehildebrandt closed 4 years ago

ehildebrandt commented 4 years ago

With version 10.2.3 the URL encoding logic changed (see https://github.com/OpenFeign/feign/pull/882).

It seems that in some cases, the query parameters are not encoded correctly. Some special characters like, e.g. equal sign (=) or quotes ('), are not encoded.

Here an example: given query parameter: "select from foo where bar = '123'" expected encoded query parameter: "select%20%2A%20from%20foo%20where%20bar%20%3D%20%27123%27" actual encoded query parameter: "select%20%20from%20foo%20where%20bar%20=%20'123'"

switchYello commented 4 years ago

can you given a simple test case,i don‘t understand. i test here,

        String a = "select * from foo where bar = '123'";
        System.out.println(UriUtils.encode(a));

result

select%20%2A%20from%20foo%20where%20bar%20%3D%20%27123%27
ehildebrandt commented 4 years ago

Test code: String a = "select * from foo where bar = '123'"; System.out.println(UriUtils.queryEncode(a, Charsets.UTF_8));

result: select%20*%20from%20foo%20where%20bar%20=%20'123'

This is an issue for clients using SpringMvcContract like e.g.: @RequestMapping(value = "/some-path/", method = GET, consumes = APPLICATION_JSON_VALUE, produces = APPLICATION_JSON_VALUE) QueryResponse<T> query(@NonNull @RequestParam("q") String query);

kdavisk6 commented 4 years ago

From what I can see, the issue you are experiencing is due to inconsistent encoding specifically on a query string. The = character is valid when present within the query segment of a url. We recognize this and leave the character as is during template expansion.

In the near term, my recommendation is to encode the = character in the string before submitting it.

I'll associate this issue with a number of other odd encoding issues that I am currently working on.

kdavisk6 commented 4 years ago

@ehildebrandt I'm pretty sure I have this fixed. Can you please try 10.7.1-SNAPSHOT?

ehildebrandt commented 4 years ago

It works. Thanks.