Multi-value header routing support #1494
Comments
|
To clarify the only real difference seems to be in the client being used, ie. cURL vs Chrome. Other than that both requests seem to be identical. |
|
We've enabled Trace logging for the Microsoft source to see if that gives us any additional pointers to go on. Here is the logging for a Chrome requests: And the cURL requests give me the following logging: As you can see the logs are nearly identical, except that in the first scenario it selects |
|
As this thing was driving me crazy we ended up building our own Yarp version with some additional logging in the Specifically we've added an This is probably not related to YARP, but more of an issue downstream in the runtime somewhere. Maybe @davidfowl has a clue of what's going on here? |
|
That is weird, I wonder what is the difference of the request on the wire. Can you check it out in Wireshark or Fiddler easily? |
|
Triage: It seems the problem is in format of multiple values -- either 1 header with comma-separated values or multiple headers. We should likely figure out design for multi-value headers matching. We will decide on 1.1 vs. later based on complexity. |
|
You're hitting two issues:
1 will likely be addressed in 7.0. 2 would be expanded functionality in YARP. The interesting question there is what would it take? Exists should already work. reverse-proxy/src/ReverseProxy/Routing/HeaderMatcherPolicy.cs Lines 77 to 85 in d9f186a For each of the others I think we'd want to enumerate each header, as well as split on commas, and then apply the match condition to the individual values. This is a lot slower and more expensive, so it might be opt-in. @jmezach as for workarounds you have two options. |
Tratcher
changed the title
Tratcher
added
Type: Enhancement
|
We've added a middleware which basically boils down to the following: if (context.Request.Headers.TryGetValue(HeaderNames.Cookie, out var requestHeaderValues))
{
// Only do something if there is more than one value
if (requestHeaderValues.Count > 1)
{
// This might not be the most efficient way of doing this, but let's see if it works first
context.Request.Headers[HeaderNames.Cookie] = requestHeaderValues.ToString();
}
}That seems to resolve our immediate issue for now. We've added some #if's so that when we'll eventually upgrade to .NET 7 we'll be triggered to re-evaluate whether this is still needed or not. One thing we were wondering though is whether this is the most optimal way to do this. Obviously we're allocating a string here. Maybe there's a more performant way of doing this that we're not aware of? @Tratcher Thanks for the quick response by the way. |
|
Note cookies should be The perf should be OK unless you're expecting many/large cookies. You could switch to a StringBuilder. |
|
The behavior that makes some intuitive sense to me would such that multiple request values act as OR.
Preudo-code: var tryMatchMode = matchMode == NotContains ? Contains : matchMode;
foreach (var requestValue in requestHeaderValues)
{
foreach (var expectedValue in expectedHeaderValues)
{
if (TryMatch(requestValue, expectedValue, tryMatchMode, comparison))
{
return matchMode != NotContains;
}
}
}
return matchMode == NotContains;Examples:
Note: everything here most likely applies to @Tratcher thoughts? |
|
This might not apply to queries since those come pre-parsed. |
|
Yeah, you are right, we should treat these two the same. I'll try it out and see what kind of a perf penalty this actually is so we can determine if having the flag is worth the effort. |
IHeaderDictionary.GetCommaSeparatedValues handles the necessary parsing, including comma splitting and quoted strings. The only header that should need to be special cased is Cookie, if we wanted to support that. |
Cookies is what I had in mind. Looking at the signature |
Yeah, but you might want to use it to write the first version and tests before optimizing 😁. |
Describe the bug
We've been looking at a very weird issue in YARP's routing almost the entire day and we still haven't quite pinpointed yet what the problem is, but we can consistently reproduce the issue in our test environments.
The problem stems from the fact that we have two routes, which are mostly identical. Here is route1:
{ "RouteId": "route1", "Match": { "Path": "/api/{**catch-all}", }, "ClusterId": "api", "CorsPolicy": "default", "Transforms": [ { "PathRemovePrefix": "/api" }, { "X-Forwarded": "Off" } ] }And route 2:
{ "RouteId": "route2", "Match": { "Path": "/api/{**catch-all}", "Headers": [ { "Name": "Cookie", "Values": [ ".AspNetCore.Cookies" ], "Mode": "Contains" } ] }, "ClusterId": "api", "AuthorizationPolicy": "CookieAuthentication", "CorsPolicy": "default", "Transforms": [ { "PathRemovePrefix": "/api" }, { "X-Forwarded": "Off" } ] }As you can see both routes are nearly identical, but
route2has an additional match based on Headers. This has been working fine for us so far while running on Windows and mostly using HTTP/1.1. We are now trying to migrate this to run on Linux containers on Kubernetes with HTTP/2 and that's where we are seeing something interesting. In particularroute2never seems to be chosen at least not while sending requests from Chrome and Edge. Sending a request with cURL using the exact same headers though seems to work just fine, which is what makes this such a weird issue.For example, here is the output of the HttpRequestLogging middleware for a request done using Chrome:
{ "@t": "2022-01-05T14:12:16.3714830Z", "@m": "Request:\nProtocol: HTTP/2\nMethod: GET\nScheme: https\nPathBase: \nPath: /api/user\nAccept: text/html,application/xhtml+xml,application/xml;q=0.9,image/avif,image/webp,image/apng,*/*;q=0.8,application/signed-exchange;v=b3;q=0.9\nHost: services.apollo.zeus.corp\nUser-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/96.0.4664.110 Safari/537.36\n:method: [Redacted]\nAccept-Encoding: gzip, deflate, br\nAccept-Language: nl-NL,nl;q=0.9,en-US;q=0.8,en;q=0.7\nCache-Control: max-age=0\nCookie: .AspNetCore.Cookies=chunks-2,.AspNetCore.CookiesC1=<sensitive-value>,.AspNetCore.CookiesC2=<sensitive-value>\nUpgrade-Insecure-Requests: [Redacted]\nsec-ch-ua: [Redacted]\nsec-ch-ua-mobile: [Redacted]\nsec-ch-ua-platform: [Redacted]\nsec-fetch-site: [Redacted]\nsec-fetch-mode: [Redacted]\nsec-fetch-user: [Redacted]\nsec-fetch-dest: [Redacted]", "@i": "893de8da", "Protocol": "HTTP/2", "Method": "GET", "Scheme": "https", "PathBase": "", "Path": "/api/user", "Accept": "text/html,application/xhtml+xml,application/xml;q=0.9,image/avif,image/webp,image/apng,*/*;q=0.8,application/signed-exchange;v=b3;q=0.9", "Host": "services.apollo.zeus.corp", "User-Agent": "Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/96.0.4664.110 Safari/537.36", ":method": "[Redacted]", "Accept-Encoding": "gzip, deflate, br", "Accept-Language": "nl-NL,nl;q=0.9,en-US;q=0.8,en;q=0.7", "Cache-Control": "max-age=0", "Cookie": ".AspNetCore.Cookies=chunks-2,.AspNetCore.CookiesC1=<sensitive-value>,.AspNetCore.CookiesC2=<sensitive-value>", "Upgrade-Insecure-Requests": "[Redacted]", "sec-ch-ua": "[Redacted]", "sec-ch-ua-mobile": "[Redacted]", "sec-ch-ua-platform": "[Redacted]", "sec-fetch-site": "[Redacted]", "sec-fetch-mode": "[Redacted]", "sec-fetch-user": "[Redacted]", "sec-fetch-dest": "[Redacted]", "HttpRequestLog": "Request:\nProtocol: HTTP/2\nMethod: GET\nScheme: https\nPathBase: \nPath: /api/user\nAccept: text/html,application/xhtml+xml,application/xml;q=0.9,image/avif,image/webp,image/apng,*/*;q=0.8,application/signed-exchange;v=b3;q=0.9\nHost: services.apollo.zeus.corp\nUser-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/96.0.4664.110 Safari/537.36\n:method: [Redacted]\nAccept-Encoding: gzip, deflate, br\nAccept-Language: nl-NL,nl;q=0.9,en-US;q=0.8,en;q=0.7\nCache-Control: max-age=0\nCookie: .AspNetCore.Cookies=chunks-2,.AspNetCore.CookiesC1=<sensitive-value>,.AspNetCore.CookiesC2=<sensitive-value>\nUpgrade-Insecure-Requests: [Redacted]\nsec-ch-ua: [Redacted]\nsec-ch-ua-mobile: [Redacted]\nsec-ch-ua-platform: [Redacted]\nsec-fetch-site: [Redacted]\nsec-fetch-mode: [Redacted]\nsec-fetch-user: [Redacted]\nsec-fetch-dest: [Redacted]", "EventId": { "Id": 1, "Name": "RequestLog" }, "SourceContext": "Microsoft.AspNetCore.HttpLogging.HttpLoggingMiddleware", "RequestId": "0HMEGAFLMA6A6:00000001", "RequestPath": "/api/user", "ConnectionId": "0HMEGAFLMA6A6", "MachineName": "apigateway-f8dcddfd6-lc2gn", "ProcessId": 1, "ThreadId": 33, "LogicalServiceName": "API Gateway" }Here is the same request done using cURL (by copying the request from Chrome Developer Tools as a cURL command):
{ "@t": "2022-01-05T14:10:13.6241996Z", "@m": "Request:\nProtocol: HTTP/2\nMethod: GET\nScheme: https\nPathBase: \nPath: /api/user\nAccept: text/html,application/xhtml+xml,application/xml;q=0.9,image/avif,image/webp,image/apng,*/*;q=0.8,application/signed-exchange;v=b3;q=0.9\nHost: services.apollo.zeus.corp\nUser-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/96.0.4664.110 Safari/537.36\n:method: [Redacted]\nAccept-Encoding: deflate, gzip, br, zstd\nAccept-Language: nl-NL,nl;q=0.9,en-US;q=0.8,en;q=0.7\nCache-Control: max-age=0\nCookie: .AspNetCore.Cookies=chunks-2,.AspNetCore.CookiesC1=<sensitive-value>,.AspNetCore.CookiesC2=<sensitive-value>\nUpgrade-Insecure-Requests: [Redacted]\nauthority: [Redacted]\nsec-ch-ua-mobile: [Redacted]\nsec-ch-ua-platform: [Redacted]\nsec-fetch-site: [Redacted]\nsec-fetch-mode: [Redacted]\nsec-fetch-user: [Redacted]\nsec-fetch-dest: [Redacted]", "@i": "893de8da", "Protocol": "HTTP/2", "Method": "GET", "Scheme": "https", "PathBase": "", "Path": "/api/user", "Accept": "text/html,application/xhtml+xml,application/xml;q=0.9,image/avif,image/webp,image/apng,*/*;q=0.8,application/signed-exchange;v=b3;q=0.9", "Host": "services.apollo.zeus.corp", "User-Agent": "Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/96.0.4664.110 Safari/537.36", ":method": "[Redacted]", "Accept-Encoding": "deflate, gzip, br, zstd", "Accept-Language": "nl-NL,nl;q=0.9,en-US;q=0.8,en;q=0.7", "Cache-Control": "max-age=0", "Cookie": ".AspNetCore.Cookies=chunks-2,.AspNetCore.CookiesC1=<sensitive-value>,.AspNetCore.CookiesC2=<sensitive-value>", "Upgrade-Insecure-Requests": "[Redacted]", "authority": "[Redacted]", "sec-ch-ua-mobile": "[Redacted]", "sec-ch-ua-platform": "[Redacted]", "sec-fetch-site": "[Redacted]", "sec-fetch-mode": "[Redacted]", "sec-fetch-user": "[Redacted]", "sec-fetch-dest": "[Redacted]", "HttpRequestLog": "Request:\nProtocol: HTTP/2\nMethod: GET\nScheme: https\nPathBase: \nPath: /api/user\nAccept: text/html,application/xhtml+xml,application/xml;q=0.9,image/avif,image/webp,image/apng,*/*;q=0.8,application/signed-exchange;v=b3;q=0.9\nHost: services.apollo.zeus.corp\nUser-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/96.0.4664.110 Safari/537.36\n:method: [Redacted]\nAccept-Encoding: deflate, gzip, br, zstd\nAccept-Language: nl-NL,nl;q=0.9,en-US;q=0.8,en;q=0.7\nCache-Control: max-age=0\nCookie: .AspNetCore.Cookies=chunks-2,.AspNetCore.CookiesC1=<sensitive-value>,.AspNetCore.CookiesC2=<sensitive-value>\nUpgrade-Insecure-Requests: [Redacted]\nauthority: [Redacted]\nsec-ch-ua-mobile: [Redacted]\nsec-ch-ua-platform: [Redacted]\nsec-fetch-site: [Redacted]\nsec-fetch-mode: [Redacted]\nsec-fetch-user: [Redacted]\nsec-fetch-dest: [Redacted]", "EventId": { "Id": 1, "Name": "RequestLog" }, "SourceContext": "Microsoft.AspNetCore.HttpLogging.HttpLoggingMiddleware", "RequestId": "0HMEGAGUUK0CE:00000001", "RequestPath": "/api/user", "ConnectionId": "0HMEGAGUUK0CE", "MachineName": "apigateway-f8dcddfd6-bfznx", "ProcessId": 1, "ThreadId": 41, "LogicalServiceName": "API Gateway" }For that first request
route1is chosen as the endpoint, while for the second requestroute2is chosen. So far we haven't been able to figure out what makes these requests different.To Reproduce
We've tried to create a small repro, but so far haven't been able to. Even with a minimal .NET 6 project with YARP added to it and putting that into a Docker container and doing a request with cURL works just fine.
Further technical details
The text was updated successfully, but these errors were encountered: