feign
feign copied to clipboard
Missing = character for query parameters when value is empty
Hello, I have been upgrading feign from 10.1.0 to 11.8 and I noticed a change of behavior when empty parameters are passed. Here is an example with a dummy interface :
public interface FeignClient {
@RequestLine("GET /path?param={param}")
void call(@Param("param") String value);
}
On the previous version, when calling the client with an empty string as the parameter value, feign would perform a request on :
/path
With feign 11.10, the actual request is now performed on :
/path?param
I understand that it is an expected behavior that empty strings are not ignored anymore. However I'm surprised that the '=' symbol is removed from the query.
As I checked the RFC 6570 standard, I understood that the ideal target would be to write the request line as
@RequestLine("GET /path{?param}")
when this is supported. In the standard the '=' signs would be present for empty values.
I am curious to get your opinion on this. Do you consider the current behavior as a defect or an expected behavior ?
To give more context I detected this because of unit tests that use PACT to align on a REST contract. PACT's mock server fails to parse the query as it looks explicitly for key/value pairs separated with '&' and '='
I attach a zip with a basic maven project to reproduce the case : feign-query-case.zip
I've duplicated this issue and written 2 unit tests that are failing on master in my branch here: https://github.com/JKomoroski/feign/tree/issue/1807/handle_empty_parameters
I will see if I can find what is going wrong.
There is a unit test checking that this behavior is expected.
It appears to have been changed in commit 2d761cb6a92efc6b4a95e7092c945f47431b76cd.
public void queryMapIterableValuesExpanded() throws Exception {
server.enqueue(new MockResponse());
TestInterface api = new TestInterfaceBuilder().target("http://localhost:" + server.getPort());
Map<String, Object> queryMap = new LinkedHashMap<>();
queryMap.put("name", Arrays.asList("Alice", "Bob"));
queryMap.put("fooKey", "fooValue");
queryMap.put("emptyListKey", new ArrayList<String>());
queryMap.put("emptyStringKey", ""); // empty values are ignored.
api.queryMap(queryMap);
assertThat(server.takeRequest())
.hasPath("/?name=Alice&name=Bob&fooKey=fooValue&emptyStringKey");
}
I don't think this behavior is what the RFC specifies for this case.. Section 3.2.2 seems to indicate that for simple text expansion, an empty string value should not have this effect.
I believe this unit test was incorrectly updated to match the behavior of the code after the implementation change. The code appears to behave this way to account for a relative uri with a query string:
public void resolveTemplateWithRelativeUriWithQuery() {
RequestTemplate template = new RequestTemplate()
.method(HttpMethod.GET)
.uri("/wsdl/testcase?wsdl")
.target("https://api.example.com");
assertThat(template).hasUrl("https://api.example.com/wsdl/testcase?wsdl");
}
I'll see if I can put together a fix.
This is correct. Check the Readme
~~https://github.com/OpenFeign/feign#undefined-vs-empty-values~~
Adding more context. This is expected behavior and not a bug. When we have a query string that has no value, it's treated as a pure
parameter and thus, no =
is present. It looks like the documentation is out of date.
Yeah. I see that flag in the Query template. It's very odd the passing a null
results in no query*, but ""
results in a pure param. It's a bit of a "magic number"
It seems to me that we could probably handle these better if we made clear distinctions in the code for: Query Literals Query Expressions Pure Query Literals Pure Query Expressions
I'd need to think about it and play with implementations to avoid breaking changes.
Many thanks for your analysis and your feedbacks !
I have also a slightly different case but on this topic : it is when the parameter is a list :
@RequestLine("GET /pathForList?params={params}")
void callWithList(@Param("params") List<String> values);
When the client is called with an empty list, the resulting request line is :
GET /path/pathForList?params
In the section 2.3 of the RFC, I see that empty lists should be ignored.
As I try to play with it I now see a behavior that looks weird :
- If the list is filled with a single empty string element, the result is :
GET /path/pathForList
- If the list is filled with a null element, the result is :
GET /path/pathForList?params
It seems to me we should rather have :
- For empty lists, or lists filled with null elements :
GET /path/pathForList
- Empty string items in lists not ignored, and appended as
GET /path/pathForList?params=
What do you think ?
I have the same problem, any custom solution?
I face same issue. For my rest-facade base, I cannot pass an empty string as query parameter from the QueryMap like below:
@GetMapping("locations/countries/") String fetchCountries( @SpringQueryMap queryParamList) { ... }
Then the intercepted URI is: {restFacade.baseurl}/locations/countries?siteId=US&query&languageId=EN
Is there any work-around for this please? Basic tests with setting the key as <query=> does not work as "=" or " " gets encoded to %3D or %20 .