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

bug: multi-value headers don't propagate to subgraphs correctly #4153

Closed
nmoutschen opened this issue Nov 6, 2023 · 1 comment · Fixed by #4154
Closed

bug: multi-value headers don't propagate to subgraphs correctly #4153

nmoutschen opened this issue Nov 6, 2023 · 1 comment · Fixed by #4154

Comments

@nmoutschen
Copy link
Contributor

Describe the bug

When sending multiple headers with the same name but different values to the Router, there's only one header entry that remains in the request to the subgraph

To Reproduce
Steps to reproduce the behavior:

  1. Setup a test router using the experimental_logging functionality and header propagation set to forward all headers, with a subgraph that logs all requests
  2. Send a request with 2 headers using the same name
  3. See that the supergraph request headers have 2 headers of the same name
  4. See that the subgraph request headers only have 1
  5. See that the subgraph only sees one too

Expected behavior

Header propagation should propagate all values for multi-value headers.

Output
If applicable, add output to help explain your problem.

Desktop (please complete the following information):

  • OS: [e.g. iOS]
  • Version [e.g. 22]

Additional context
Add any other context about the problem here.

@garypen
Copy link
Contributor

garypen commented Nov 6, 2023

Probably need to replace insert() with append() here:

headers.insert(rename.as_ref().unwrap_or(named), value.clone());

and here:
headers.insert(name, value.clone());

Need to carefully examine that plugin because there may be other instances where insert() should be append()

@Geal Geal closed this as completed in #4154 Nov 8, 2023
Geal pushed a commit that referenced this issue Nov 8, 2023
Uses `HeaderMap.append` instead of `insert` to prevent removing values
from multi-value headers.

__Note__: This might break implementations that require on a single
header value being passed.

Fixes #4153
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants