Question for https://github.com/OpenFeign/feign/pull/882.

[hiasince]

I have some question for this PR #882.

We are using it with spring. When we were using Feign before upgrade to 10.2.0, There was any problem with using ampersand(&) for @QueryParam value. But after upgrade to 10.2.0, We have problem Because of this implementation.

What I want to ask is What is the advantage for Preserving reserved characters on the query string? I cannot use reserved characters for @QueryParam anymore unless supplement additional code. I'm all for it that change encoding algorithm for '+' and ' '. But I think We have to encode reserved characters. Because It is Reserved. It used for special meaning in defining url syntax. If reserved characters are maintains on @QueryParam, It will work unexpectedly or It will occur bug, without any Error message or Warning message.

I understood this PR like Feign wants to say to user "Please do not use any reserved characters."
Is it correct? or Did you have any other purpose?

[kdavisk6]

@hiasince

Before #882, our handling of reserved characters was inconsistent at best and downright strange at its worst.

To that end, we recognize that it's impossible for us to understand every use case for every user, so we've decided to default to encoding known reserved characters where appropriate and leave the rest up to the users. If you want to bypass this behavior, we ask that you pct-encode the values first and use encoded = true on the annotation.

Now to address your specific issue around ampersands, those are reserved characters and if you want to use them as values in a uri template, we must encode them per RFC 6570. Do you have a specific example where this is an issue?

[hiasince]

@kdavisk6

Thanks for your reply.

In our project, We send request with value contain ampersands through Feign. That value is marked as @QueryParam . we send it with many other @QueryParam. we request like below.

https://system.test.com/system/request?nextUrl=https://cloud.test.com/test/sendno/certify/callback-popup?appKey=testKey&token=token

And then we use nextUrl parameter for response. But we find that value of nextUrl has been changed unexpectedly.

https://sms.cloud.toast.com/toastsms/sendno/certify/callback-popup?appKey=testKey

I think this problem occur because of preserving ampersands. It recognize &token=token as another query parameter.

We solved this problem by encoding that string by java.net.URLEncode like this. URLEncode.encode(nextUrl, "UTF-8");

[kdavisk6]

@hiasince

Encoding the value before sending is the correct way to handle your situation. When in doubt, pct-encode it. Glad I could help.

[hiasince]

@kdavisk6

Ok. You think encoding the suspicious value before sending is right way to solve this problem.

But, I still have some question mark on that. Before change, it worked without any problem.
If you've got some problem with using urlEncoder to handle reserved characters, Is it right way to use urlEncoder before sending when I have suspicious value?

I think fixing + problem is right, But I don't know why you changed algorithm with handling other reserved characters. It make problem on program work normally.

I know that Feign used urlEncoder as default. If you've got any Issue with using urlEncoder, Do you have a specific example? except +, whitespace problem. I agree with that.

[kdavisk6]

@hiasince

I recommend that you read the original issue that prompted these changes: #879.
URLEncoder is not appropriate for encoding values on a URI and should be used only with HTML Forms. Per the javadoc:

Utility class for HTML form encoding. This class contains static methods for converting a String to the application/x-www-form-urlencoded MIME format.

This is a common misconception when it comes to URI encoding. For a complete breakdown of which characters are allowed where in a URI, see the RFC 3986 - Uniform Resource Identifier: Appendix A.

Our position on this matter is that Feign will no longer maintain it's own set of rules, but adhere to RFC 6570 - URI Templates explicitly and fall back to RFC 3986 - Uniform Resource Identifier when necessary.

[hiasince]

@kdavisk6

Thanks for your reply! You've just make my head clear.

I have some misconception between HTML form encoding and URI resource encoding. I thought that updating to #882 is for maintain Feign's own set of rules for URI encoding.

[kdavisk6]

Great. Glad I could help.