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

Adding URI segment specific encoding #882

Merged
merged 4 commits into from
Feb 1, 2019

Conversation

kdavisk6
Copy link
Member

Fixes #879

URI encoding introduced in Feign 10.x was refactored to be more in line
with URI and URI Template specifications respectively. One change was to
ensure that certain reserved characters were not encoded incorrectly.

The result was that path segment specific reserved characters were being
preserved on the query string as well. This change updates the UriTemplate
and Expression classes to recognize the segment of the URI that is being processed
and apply the segment specific encoding correctly.

One important change regarding the + sign. Per the URI specification, a + sign
is allowed in both the path and query segments of a URI, however, handling of
the symbol on the query can be inconsistent. In some legacy systems, the + is
equivalent to the a space. Feign takes the approach of modern systems, where a
+ symbol should not represent a space and is explicitly encoded as %2B when
found on a query string.

If you wish to use + as a space, then use the literal character or encode
the value directly as %20

Fixes OpenFeign#879

URI encoding introduced in Feign 10.x was refactored to be more in line
with URI and URI Template specifications respectively.  One change was to
ensure that certain reserved characters were not encoded incorrectly.

The result was that path segment specific reserved characters were being
preserved on the query string as well.  This change updates the `UriTemplate`
and `Expression` classes to recognize the segment of the URI that is being processed
and apply the segment specific encoding correctly.

One important change regarding the `+` sign.  Per the URI specification, a `+` sign
is allowed in both the path and query segments of a URI, however, handling of
the symbol on the query can be inconsistent.  In some legacy systems, the `+` is
equivalent to the a space.  Feign takes the approach of modern systems, where a
`+` symbol should not reprsent a space and is explicitly encoded as `%2B` when
found on a query string.

If you wish to use `+` as a space, then use the literal ` ` character or encode
the value directly as `%20`
@kdavisk6 kdavisk6 added bug Unexpected or incorrect behavior waiting for feedback Issues waiting for a response from either to the author or other maintainers regression Bugs and issues related to unintended breaking changes labels Jan 30, 2019
@kdavisk6 kdavisk6 requested a review from velo January 30, 2019 15:16
Copy link
Member

@velo velo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lgtm

@kdavisk6 kdavisk6 added ready to merge Will be merged if no other member ask for changes and removed waiting for feedback Issues waiting for a response from either to the author or other maintainers labels Feb 1, 2019
@kdavisk6 kdavisk6 merged commit 29935b2 into OpenFeign:master Feb 1, 2019
@kdavisk6 kdavisk6 deleted the gh-879-aggressive-encoding branch February 1, 2019 21:54
@dragontree101
Copy link

when the pr will be release? i hope to upgrade feign's version to fix this bug. thanks

@kdavisk6
Copy link
Member Author

I'm in the process of cleaning up a few other things. I hope to have this released in the next week or two.

@kdavisk6
Copy link
Member Author

@dragontree101 10.2.0 should be available now.

@sabareeshkkanan
Copy link

We are using it with spring boot and it is not available yet. We have to override with explicit build number for open-feign. Thank you

velo pushed a commit that referenced this pull request Oct 7, 2024
* Adding URI segment specific encoding

Fixes #879

URI encoding introduced in Feign 10.x was refactored to be more in line
with URI and URI Template specifications respectively.  One change was to
ensure that certain reserved characters were not encoded incorrectly.

The result was that path segment specific reserved characters were being
preserved on the query string as well.  This change updates the `UriTemplate`
and `Expression` classes to recognize the segment of the URI that is being processed
and apply the segment specific encoding correctly.

One important change regarding the `+` sign.  Per the URI specification, a `+` sign
is allowed in both the path and query segments of a URI, however, handling of
the symbol on the query can be inconsistent.  In some legacy systems, the `+` is
equivalent to the a space.  Feign takes the approach of modern systems, where a
`+` symbol should not reprsent a space and is explicitly encoded as `%2B` when
found on a query string.

If you wish to use `+` as a space, then use the literal ` ` character or encode
the value directly as `%20`
velo pushed a commit that referenced this pull request Oct 8, 2024
* Adding URI segment specific encoding

Fixes #879

URI encoding introduced in Feign 10.x was refactored to be more in line
with URI and URI Template specifications respectively.  One change was to
ensure that certain reserved characters were not encoded incorrectly.

The result was that path segment specific reserved characters were being
preserved on the query string as well.  This change updates the `UriTemplate`
and `Expression` classes to recognize the segment of the URI that is being processed
and apply the segment specific encoding correctly.

One important change regarding the `+` sign.  Per the URI specification, a `+` sign
is allowed in both the path and query segments of a URI, however, handling of
the symbol on the query can be inconsistent.  In some legacy systems, the `+` is
equivalent to the a space.  Feign takes the approach of modern systems, where a
`+` symbol should not reprsent a space and is explicitly encoded as `%2B` when
found on a query string.

If you wish to use `+` as a space, then use the literal ` ` character or encode
the value directly as `%20`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Unexpected or incorrect behavior ready to merge Will be merged if no other member ask for changes regression Bugs and issues related to unintended breaking changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

upgrade spring-cloud-release from F to G, feign urlencode error
4 participants