Feign: Feign HeaderTemplates adds an extra space to date headers after commas

Created on 10 Sep 2020  ·  6Comments  ·  Source: OpenFeign/feign

HeaderTemplates#expand(Map) adds space after each comma in headers. This causes an issue when converting date that uses the EEE, d MMM yyyy HH:mm:ss Z format.

Example
Client code sends Wed, 4 Jul 2001 12:08:56 -0700
Provider expects Wed, 4 Jul 2001 12:08:56 -0700
Provider receives Wed, 4 Jul 2001 12:08:56 -0700 please note the extra space between the , and 4

bug

Most helpful comment

hi, i made pr #1295 for fix this issue, but CI build failed because of The command "./travis/no-git-changes.sh" exited with 1
i don't know how to make it build successfully

Looks like it's complaining about code formatting; did you see this in the build output?

Please run 'mvn clean install' locally to format files.

At a guess I'd say it's because you have two blank lines above your new unit test method, and the formatter expects only one. But I guess mvn clean install will fix this for you.

Thank for you reply. you are right, it is caused by formatting, but it's because there are blank lines in import statement, not unit test method

All 6 comments

@muhamadto Can you please provide a minimal working example that demonstrates this issue?

We've just been hit by this issue too, it seems to affect any expanded header with a comma. Example code attached. The log output shows that the provided date value has an additional space inserted after the comma:

12:34:33.537 [main] DEBUG feign.Logger - [SampleApi#run] ---> GET https://reqres.in/api/users?page=2 HTTP/1.1
12:34:33.538 [main] DEBUG feign.Logger - [SampleApi#run] Date: Thu,  24 Sep 2020 10:34:09 GMT
12:34:33.538 [main] DEBUG feign.Logger - [SampleApi#run] ---> END HTTP (0-byte body)

The issue was likely caused by this change: https://github.com/OpenFeign/feign/pull/1203

Downgrading to 10.9 (last version before this change was made) has fixed for us for now.

feign-header-bug.zip

hi, i made pr #1295 for fix this issue, but CI build failed because of The command "./travis/no-git-changes.sh" exited with 1

i don't know how to make it build successfully

hi, i made pr #1295 for fix this issue, but CI build failed because of The command "./travis/no-git-changes.sh" exited with 1

i don't know how to make it build successfully

Looks like it's complaining about code formatting; did you see this in the build output?

Please run 'mvn clean install' locally to format files.

At a guess I'd say it's because you have two blank lines above your new unit test method, and the formatter expects only one. But I guess mvn clean install will fix this for you.

hi, i made pr #1295 for fix this issue, but CI build failed because of The command "./travis/no-git-changes.sh" exited with 1
i don't know how to make it build successfully

Looks like it's complaining about code formatting; did you see this in the build output?

Please run 'mvn clean install' locally to format files.

At a guess I'd say it's because you have two blank lines above your new unit test method, and the formatter expects only one. But I guess mvn clean install will fix this for you.

Thank for you reply. you are right, it is caused by formatting, but it's because there are blank lines in import statement, not unit test method

I'm beginning to see that we'll have to "bite the bullet" and upgrade the Template handling sooner than Feign 11. The root cause of all of these issues is that our Template Handling relies on String manipulation for templates that can support Collections of values. Single values work fine.

Since HTTP Headers are generally considered Collection based, this problem will continue to appear. I can see that in #1298 an attempt is make to clear out extra spaces, but that solution will work for the example provided only and not generally across all headers. I'm going to link this to the template update issue in #1019 and see if we can start by separating general URI templates with Headers first.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

firebook picture firebook  ·  5Comments

DogSunny picture DogSunny  ·  4Comments

Tal24 picture Tal24  ·  5Comments

nmaoez picture nmaoez  ·  3Comments

budaimartin picture budaimartin  ·  4Comments