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

HTTP Header casing is modified #3964

Open
javaes opened this issue Jan 23, 2020 · 9 comments
Open

HTTP Header casing is modified #3964

javaes opened this issue Jan 23, 2020 · 9 comments

Comments

@javaes
Copy link

javaes commented Jan 23, 2020

Bug Report

What is the issue?

When doing an HTTP call to my service, the headers of my HTTP requests are modified. All header keys are changed to lower case.

Even though the HTTP standard says that headers are not case sensitive, this can be implemented differently in some servers. Using linkerd makes it impossible to integrate with these services which depend on proper header key casing.

How can it be reproduced?

Deploy two applications with linkerd proxies enabled. Application A is calling application B with an HTTP request which contains headers which have upper case fields. When inspecting the incoming request in application B, you will see that all header keys are lower cased now.

When calling Application B directly without passing through the linkerd proxy the logs still show the original headers.

Environment

  • Kubernetes Version: v1.15.7
  • Cluster Environment: Dedicated Setup
  • Host OS: Core OS
  • Linkerd version: 2.6.1

Possible solution

  • an additional annotation would be feasible for us to deactivate the lower-casing
@ihcsim
Copy link
Contributor

ihcsim commented Jan 23, 2020

Relevant #2878.

@javaes
Copy link
Author

javaes commented Jan 24, 2020

Sorry for duplicating the linked issue. Even though it's closed because of being RFC compliant, I think this is still a relevant issue because this behavior is not expected by proxy users and it basically breaks our integration with 3rd party APIs which we can't change. Please consider re-opening the original issue.

@christianhuening
Copy link
Contributor

@javaes Can you say how the expected behaviour on the 3rd party side is? Do they expect Title-Case or any other format which can be baked into a transformational rule?

@javaes
Copy link
Author

javaes commented Jan 24, 2020

@christianhuening the 3rd party where I've encountered this issue is expecting title cased header keys, but I think this would still be a bold assumption to make for more 3rd party APIs since the project is about integrating a lot of APIs.

@JacobHenner
Copy link
Contributor

I'd be willing to investigate what would be required to address this, but I want to first confirm that the proposal(s) would not be dismissed for the reason mentioned in #2878 (comment):

in this case we are RFC compliant and changing this behavior would impact proxy performance

@olix0r
Copy link
Member

olix0r commented Apr 19, 2022

@JacobHenner it won't be rejected out of hand, but I also view it as fairly low priority and won't be able to devote much time (in the short-term, at least) to getting these changes vetted: this change only would only exist to satisfy HTTP endpoints that don't actually honor the HTTP RFCs. That's a losing battle, in my opinion.

Furthermore, because Linkerd transports in-mesh HTTP traffic over HTTP/2, it's going to be pretty complex to preserve header casing (since HTTP/2 is explicitly case-agnostic) for in-mesh traffic.

I know there's been some discussion in the hyper & http crates, so it may be more feasible than the last time we looked, but I worry about the complexity of this change.

If you're willing to do the work to come with a proposal of what this change would actually entail (from a code point of view), the benchmarking to compare behavior, etc, we can probably help/guide. But I can't promise that we'll want to take the change if we feel the complexity/maintenance burden outweighs the benefits.

@olix0r
Copy link
Member

olix0r commented Apr 19, 2022

Maybe a better question to come back to: what do you actually want from Linkerd in these cases--mTLS? load balancing? metrics? Would it be suitable to have Linkerd handle this communication opaquely (as TCP) so it doesn't even parse the HTTP headers?

@JacobHenner
Copy link
Contributor

@JacobHenner it won't be rejected out of hand, but I also view it as fairly low priority and won't be able to devote much time (in the short-term, at least) to getting these changes vetted: this change only would only exist to satisfy HTTP endpoints that don't actually honor the HTTP RFCs. That's a losing battle, in my opinion.

Understandable, thanks for clarifying. In that case I'll first focus on the other options we have available, and revisit this if it becomes strictly necessary.

Maybe a better question to come back to: what do you actually want from Linkerd in these cases--mTLS? load balancing? metrics?

mTLS at minimum, but preferably the maximum feature set possible for each type of destination.

Would it be suitable to have Linkerd handle this communication opaquely (as TCP) so it doesn't even parse the HTTP headers?

In some cases this would suffice (e.g. mTLS is the only required feature), and in others this would be less-than-desirable (e.g. when devs expect the more advanced features to function).

That said, my biggest concern is that devs might not realize that their application(s) depend on non-RFC-conforming case-sensitive HTTP headers, so they likely won't know to mark ports opaque until the unexpected header casing change causes a service disruption that's investigated and debugged. This complicates rolling out linkerd, as enabling proxy injection at the namespace level (without affirmative consent from each application owner) is potentially a more dangerous operation.

@olix0r
Copy link
Member

olix0r commented Apr 20, 2022

@JacobHenner That's helpful context. It's unfortunate that this feature is basically at odds with request multiplexing over HTTP/2. We could potentially support a mode where headers are forcibly transformed to Title-Case instead of lower-case.

Otherwise, I think you'd have to disable request multiplexing for HTTP/1. If someone has time to see what it takes to have the proxy (configure Hyper to) preserve the original case, we could entertain that change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Help Wanted
  
To do
Development

No branches or pull requests

6 participants