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

Truncated JSON header value with @Headers #1464

Closed
daczczcz1 opened this issue Jul 21, 2021 · 17 comments · Fixed by #1776
Closed

Truncated JSON header value with @Headers #1464

daczczcz1 opened this issue Jul 21, 2021 · 17 comments · Fixed by #1776
Labels
help wanted Issues we need help with tackling

Comments

@daczczcz1
Copy link

Observed on versions: 11.2, 11.4
Not reproducible on version: 10.1.0

When passing a JSON string as @Param to @Headers the string gets truncated.

Example:

{"some": "value", "other": "value"}

Gets truncated to:

{"some": "value", "other"}

Code to reproduce: https://github.com/daczczcz1/feign-bug-reproduction

@kppullin
Copy link

We've run into this as well (using the RequestTemplate.header() method, but I'm guessing still hits the same templating logic as @Headers). Escaping the braces using percent encoding or double bracing ({{) doesn't help, as the content is not unescaped when it lands on the wire.

@vinz486
Copy link

vinz486 commented Nov 22, 2021

The annotation based @Header should be different from the .header(...) method. A solution could be a .headerRaw(...) method.

@dagerber
Copy link

This breaks our code. We're adding a custom JSON structure to a HEADER which now gets truncated.
Please add another method as suggested above which adds a plain string as value without parsing

@lavoiej1
Copy link

Same problem here with the @RequestHeader annotation of spring-cloud-starter-openfeign library. The 3.1.1 version uses feign-core 11.8.

@JKomoroski
Copy link
Contributor

Ran into this today after upgrading from an older version to 11.8

@skpandey91
Copy link

Any solution to this problem?
We are blocked.

@janson653
Copy link

any update?

@velo velo added the help wanted Issues we need help with tackling label Jun 22, 2022
@suryamurali1991
Copy link

Blocked by the same problem. Any update or a workaround to resolve this please.

@velo
Copy link
Member

velo commented Jun 23, 2022

Hi @suryamurali1991 looking for volunteers to help fixing this issue.

@SikoraAdam
Copy link

@velo I've seen the PR was close, any chance the issue gonna be attended soon?

@velo
Copy link
Member

velo commented Jul 7, 2022

@SikoraAdam it's still waiting for someone to make the necessary changes. I'm willing to review it and get the release out of the door. Feel free to take a look on the comments we left at the PR and make the necessary changes

@Samylots
Copy link

Samylots commented Sep 7, 2022

I confirm that we are also impacted by this issue.
I have temporarely forced our project to get the 10.12 version to make it work.

@RussellTaylor83
Copy link

RussellTaylor83 commented Sep 30, 2022

Same problem for us. To sidestep it temporarily we're pulling in feign-core 11.0

<dependency>
<groupId>io.github.openfeign</groupId>
<artifactId>feign-core</artifactId>
<version>11.0</version>
</dependency>

@JKomoroski
Copy link
Contributor

I ran a git bisect on this issue and narrowed it down to the following commit: f8ad8670e3dc73862f52eec475464a301e8c41cc

That commit did some much needed work. No way to revert it. The issue is not in the HeaderTemplate expand function, nor is it in the Expression or Simple Expression code. As far as I can see the RequestTemplate appears to be expanding the header template more than once. Resulting in an expression being expanded to JSON and then the Json being treated as a template and the Expression expansion going off the rails there.

I have a fork and commit with the duplication unit tests here:
JKomoroski@0f686eb

Working on a sensible fix.

kdavisk6 added a commit to kdavisk6/feign that referenced this issue Oct 6, 2022
This change adds a new `appendHeader` internal method to `RequestTemplate`
allowing for already resolved headers to be added to the resolved `RequestTemplate`
preventing duplicate expression processing by using another new method
`HeaderTemplate.literal` and `HeaderTemplate.appendLiteral` respectively.

I chose this route as it isolates the change to be applied only after the
original `HeaderTemplate` has been resolved.  While it does expose new
public `HeaderTemplate` APIs, I feel that is an OK trade off, allowing
a new escape-hatch for situations where URI template processing is not
acceptable for Header values.
kdavisk6 added a commit that referenced this issue Oct 6, 2022
This change adds a new `appendHeader` internal method to `RequestTemplate`
allowing for already resolved headers to be added to the resolved `RequestTemplate`
preventing duplicate expression processing by using another new method
`HeaderTemplate.literal` and `HeaderTemplate.appendLiteral` respectively.

I chose this route as it isolates the change to be applied only after the
original `HeaderTemplate` has been resolved.  While it does expose new
public `HeaderTemplate` APIs, I feel that is an OK trade off, allowing
a new escape-hatch for situations where URI template processing is not
acceptable for Header values.
@xiaohundun
Copy link

Could you please let me know if this fix has been released?

@JKomoroski
Copy link
Contributor

Could you please let me know if this fix has been released?
Yes it was.
https://github.com/OpenFeign/feign/releases/tag/12.0

@xiaohundun
Copy link

Could you please let me know if this fix has been released?
Yes it was.
https://github.com/OpenFeign/feign/releases/tag/12.0

Thank you.

velo pushed a commit that referenced this issue Oct 7, 2024
This change adds a new `appendHeader` internal method to `RequestTemplate`
allowing for already resolved headers to be added to the resolved `RequestTemplate`
preventing duplicate expression processing by using another new method
`HeaderTemplate.literal` and `HeaderTemplate.appendLiteral` respectively.

I chose this route as it isolates the change to be applied only after the
original `HeaderTemplate` has been resolved.  While it does expose new
public `HeaderTemplate` APIs, I feel that is an OK trade off, allowing
a new escape-hatch for situations where URI template processing is not
acceptable for Header values.
velo pushed a commit that referenced this issue Oct 8, 2024
This change adds a new `appendHeader` internal method to `RequestTemplate`
allowing for already resolved headers to be added to the resolved `RequestTemplate`
preventing duplicate expression processing by using another new method
`HeaderTemplate.literal` and `HeaderTemplate.appendLiteral` respectively.

I chose this route as it isolates the change to be applied only after the
original `HeaderTemplate` has been resolved.  While it does expose new
public `HeaderTemplate` APIs, I feel that is an OK trade off, allowing
a new escape-hatch for situations where URI template processing is not
acceptable for Header values.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Issues we need help with tackling
Projects
None yet