headers set by the subgraph service should not be propagated from the client query by the headers plugin

[heathprovost]

Describe the bug
Requests that already have the accept header set to the value application/json, application/graphql-response+json on the inbound request to the router are getting multiple accept headers appended to the request incorrectly.

In other words if the original request contains this:

accept: application/json, application/graphql-response+json

The router ends up sending this to the subgraph:

accept: application/json, application/graphql-response+json
accept: application/json
accept: application/graphql-response+json

Unfortunately, the default behavior of the C# GraphQL.Client is to set the accept headers exactly this way : https://github.com/graphql-dotnet/graphql-client/blob/fbf37dfff2830e36a45296079bac27ec60fad550/src/GraphQL.Client/GraphQLHttpRequest.cs#L40

This behavior started in v1.35.0 of the router, and seems to me to be attributable to this changeset: 9a44bed#diff-6ef5a208ca8622f30eef88f75c18566e0304d59856b66293dcd6811555e6382e which changed the method used to set the Accept header from an .insert() to an .append(). The older behavior of just using .insert() avoided the issue because it simply overwrote the entire accept header, which is probably not the desired behavior either.

To Reproduce

1. Setup services normally (using router or cloud router v1.35.0)
2. Submit a request including the header Accept: application/json, application/graphql-response+json
3. View the request coming out of the router, it is incorrectly appending additional headers when it should not be.

Expected behavior
The router should see that the headers it is trying to set are already there and should do nothing in that scenario.

Additional context
This issue causes breakage with subgraphs running inside of lambdas behind ALBs in AWS because of how request headers are marshaled when they are converted from an http request to a lambda request. AWS lambda does not properly marshal the requests when they contain both multivalue headers AND multiple of the same header key (it works with either or, but not both) .

We are currently working around this issue by overriding the default behavior of the C# GraphQL.Client and stripping the Accept header off before the request goes out. Not a great solution obviously.

[Geal]

do you have any header propagation rules in the router's configuration? https://www.apollographql.com/docs/router/configuration/header-propagation

[heathprovost]

We are currently just doing this:

headers:
  all:
    request:
      - propagate:
          matching: .*

I realize this is another way we could probably work around the issue, but it did not seem like a solution.

[Geal]

ok so the issue here is that headers that are set by the router as part of the subgraph request, like Accept or Accept-Encoding, should not be part of propagated headers

[heathprovost]

ok so the issue here is that headers that are set by the router as part of the subgraph request, like Accept or Accept-Encoding, should not be part of propagated headers

I dont think so no. They should be part of the propogated headers, they just should not be blindly added as well. In other words these headers should be forwarded if and when they exist, and when they do NOT exist they should be added to the outbound request.

What is happening now is that the router is doing both. It is propagating them from the inbound request AND adding them again.

What should happen (imo) is that the router should only add them if they do not already exist on the inbound request.

Alternately, don't try to set the Accept header at all inside the router and require the caller to do it.

[abernix]

This probably deserves some more investigation, and perhaps should be considered together with behaviors in #4420.

[Geal]

the Accept and Accept-Encoding headers must not be propagated from the client query, because they define what the router can accept in the subgraph response (The values are actually hardcoded:

router/apollo-router/src/services/subgraph_service.rs

Lines 746 to 757 in 4027132

	request
	.headers_mut()
	.insert(CONTENT_TYPE, APPLICATION_JSON_HEADER_VALUE.clone());
	request
	.headers_mut()
	.append(ACCEPT, APPLICATION_JSON_HEADER_VALUE.clone());
	request
	.headers_mut()
	.append(ACCEPT, APP_GRAPHQL_JSON.clone());
	request
	.headers_mut()
	.insert(ACCEPT_ENCODING, ACCEPTED_ENCODINGS.clone());

). In that matter the router cannot be considered like a proxy that forwards the client query with minimal modification to a backend, it is doing its own requests to the subgraphs.
And this allows us to handle cases like a client supporting compression while a subgraph does not.

do you see a case where it is necessary to override the router headers to forward those clients headers unchanged?