Handle multivalue X-Forwarded-Proto header #5242
Conversation
|
@brettsam I pushed another change to only register this middleware in linux consumption. It shouldn't be in the CLI or the runtime generically like it was. |
| @@ -29,7 +29,7 @@ public async Task Invoke(HttpContext httpContext) | |||
|
|
|||
| if (httpContext.Request.Headers.TryGetValue(ForwardedProtocolHeader, out value)) | |||
| { | |||
| httpContext.Request.Scheme = value; | |||
| httpContext.Request.Scheme = value.FirstOrDefault(); | |||
There was a problem hiding this comment.
Should this be .First() so that if there's an empty value it blows up here rather than causing a NullReferenceException further down the pipeline? (Unless a null for request. Scheme is valid?)
Unless you want to treat empty header values as valid (which seems to be quite the discussion) in which case should it fallback to not overwriting the scheme if there's no value?
There was a problem hiding this comment.
Good point. I don't think a null scheme, but I think it makes sense to not overwrite it if the header happens to be empty for any reason. Thanks @NickDarvey
ahmelsayed
force-pushed
the
ahmels/x-f-proto
branch
from
15abdf2
to
e1b0c2d
Compare
| @@ -29,7 +29,11 @@ public async Task Invoke(HttpContext httpContext) | |||
|
|
|||
| if (httpContext.Request.Headers.TryGetValue(ForwardedProtocolHeader, out value)) | |||
| { | |||
| httpContext.Request.Scheme = value.FirstOrDefault(); | |||
| var scheme = value.FirstOrDefault(); | |||
| @@ -29,7 +29,11 @@ public async Task Invoke(HttpContext httpContext) | |||
|
|
|||
| if (httpContext.Request.Headers.TryGetValue(ForwardedProtocolHeader, out value)) | |||
| { | |||
| httpContext.Request.Scheme = value.FirstOrDefault(); | |||
| var scheme = value.FirstOrDefault(); | |||
| if (scheme != null) | |||
There was a problem hiding this comment.
Could we end up with an empty string here as well?
There was a problem hiding this comment.
as NickDarvey mentioned, an empty header value is a malformed header. TryGetValue returns false if the header is passed as empty from the client, so I was assuming aspnet will do validation on these things before passing the request to the application. !string.IsNullOrEmpty() won't hurt though, even if technically not valid per https://tools.ietf.org/html/rfc7230#section-3.2
There was a problem hiding this comment.
Makes sense, just wanted to make sure it's something we had validated/thought about.
ahmelsayed
force-pushed
the
ahmels/x-f-proto
branch
from
e1b0c2d
to
02b4f06
Compare
ahmelsayed
force-pushed
the
ahmels/x-f-proto
branch
from
02b4f06
to
28ba9d9
Compare
Closes #5198