Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

$ not support in the name of query parameters #1036

Closed
pvreeburg opened this issue Aug 9, 2019 · 1 comment · Fixed by #1139
Closed

$ not support in the name of query parameters #1036

pvreeburg opened this issue Aug 9, 2019 · 1 comment · Fixed by #1139
Assignees
Labels
bug Unexpected or incorrect behavior

Comments

@pvreeburg
Copy link

I encountered an issue with the processing of query parameters that contain a $ in their name. These apparently aren't processed as expressions but as literals.

I'm currently using FeignJaxRs2 versions '10.2.3'.

Example :

public interface TestClient {
    @GET
    @Path("/test")
    String apiCall(@QueryParam("$fields") String value);
}

public void test_with_empty_name() {
    String result = testClient.apiCall("field1,field2");
}

results in the URI :

.../test?$fields={$fields}

instead of:

.../test?$fields=field1,field2

It appears to go wrong in the class feign.template.Expressions.
The regex used to validate the name of the queryparameter doesn't currently allow $.

@kdavisk6
Copy link
Member

TL;DR - Please see #894 for an explanation and a possible workaround for now.

Explanation:
Part of the changes we've made in the past few releases have changed the way that we recognize and support uri template expressions. In practice this means that template expression variable names that include reserved characters, as defined in RFC 6570, may result in unexpected behavior. The RFC states that those values should be pct-encoded, but we don't enforce that.

If time permits, or others want to help, we can look into relaxing these restrictions more, but I'm afraid that there are more use cases that others are now depending on that will cause grief in other areas.

I know it's not an real answer, but it's an honest one.

@kdavisk6 kdavisk6 added documentation Issues that require updates to our documentation question General usage or 'how-to' questions labels Aug 12, 2019
@kdavisk6 kdavisk6 added bug Unexpected or incorrect behavior and removed documentation Issues that require updates to our documentation question General usage or 'how-to' questions labels Dec 26, 2019
@kdavisk6 kdavisk6 self-assigned this Dec 26, 2019
kdavisk6 added a commit to kdavisk6/feign that referenced this issue Dec 26, 2019
Fixes OpenFeign#1036

Relaxed the regular expression used to determine if an expression
is valid to support additional expression variable names.  We will
no longer restrict what an expression name can be.
kdavisk6 added a commit that referenced this issue Dec 27, 2019
Fixes #1036

Relaxed the regular expression used to determine if an expression
is valid to support additional expression variable names.  We will
no longer restrict what an expression name can be.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Unexpected or incorrect behavior
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants