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

Default request header headers overwriten with specific client configuration and shared by all clients #1001

Closed
martinchaves opened this issue Mar 14, 2024 · 1 comment
Assignees
Labels
bug Something isn't working
Milestone

Comments

@martinchaves
Copy link

Spring Cloud version: 4.0.6

When defining some configuration using the default client all the clients share the same "default request headers" with the merge of all configurations.

IMO the problem was introduced by the commit 2b07f21
Specifically by on line 328 by taking the default client defaultRequestHeaders and then on line 330 modifying with the defaultRequestHeaders from the specific client configuration if the clientConfig is not null.

Actual code:

Map<String, Collection<String>> defaultRequestHeaders = defaultConfig != null
        ? defaultConfig.getDefaultRequestHeaders() : new HashMap<>();
if (clientConfig != null) {
    defaultRequestHeaders.putAll(clientConfig.getDefaultRequestHeaders());
}
if (!defaultRequestHeaders.isEmpty()) {
    addDefaultRequestHeaders(defaultRequestHeaders, builder);
}

I believe that doing a deep copy of the map should be enough to fix.

Something like:

Map<String, Collection<String>> defaultRequestHeaders = new HashMap<>();
if (defaultConfig != null) {
    defaultConfig.getDefaultRequestHeaders().forEach((k, v) -> defaultRequestHeaders.put(k, new ArrayList<>(v)));
}
if (clientConfig != null) {
    clientConfig.getDefaultRequestHeaders().forEach((k, v) -> defaultRequestHeaders.put(k, new ArrayList<>(v)));
}
if (!defaultRequestHeaders.isEmpty()) {
    addDefaultRequestHeaders(defaultRequestHeaders, builder);
}

In my tests I found that version 4.0.4 and 4.1.0 are not affected.

Sample code
application.yaml

spring:
  cloud:
    openfeign:
      client:
        config:
          default:
            # could be any property as long as the *default* is not null on the configuration properties.
            logger-level: FULL 
          testClientA:
            default-request-headers:
              x-custom-header: "from client A"
          testClientB:
            default-request-headers:
              x-custom-header: "from client B"

Controller.java

@RestController
public class Controller {
  @GetMapping(value = "/a", produces = APPLICATION_JSON_VALUE)
  public Map<String, List<String>> headersA(@RequestHeader HttpHeaders headers) {
    return new HashMap<>(headers);
  }

  @GetMapping(value = "/b", produces = APPLICATION_JSON_VALUE)
  public Map<String, List<String>> headersB(@RequestHeader HttpHeaders headers) {
    return new HashMap<>(headers);
  }
}

TestClientA.java

@FeignClient(name = "testClientA", url = "http://localhost:8080", contextId = "testClientA")
public interface TestClientA {

  @GetMapping("/a")
  Map<String, List<String>> headersA();
}

TestClientB.java

@FeignClient(name = "testClientB", url = "http://localhost:8080", contextId = "testClientB")
public interface TestClientB {
  @GetMapping("/b")
  Map<String, List<String>> headersB();
}

Tests.java

@SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.DEFINED_PORT)
class Tests {
  @Autowired
  private TestClientA testClientA;
  @Autowired
  private TestClientB testClientB;

  @Test
  void testClientA() {
    assertThat(testClientA.headersA())
            .isNotNull()
            .contains(entry("x-custom-header", List.of("from client A")));

  }

  @Test
  void testClientB() {
    assertThat(testClientB.headersB())
            .isNotNull()
            .contains(entry("x-custom-header", List.of("from client B")));

  }
}
@OlgaMaciaszek
Copy link
Collaborator

Hi @martinchaves, thanks for reporting the bug. Will fix it.

@OlgaMaciaszek OlgaMaciaszek added bug Something isn't working and removed in progress labels Mar 19, 2024
@OlgaMaciaszek OlgaMaciaszek added this to the 4.1.1 milestone Mar 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
No open projects
Status: Done
Development

No branches or pull requests

3 participants