Skip to content

Allow headers to match on ReferenceEquals before OrdinalIgnoreCase #9341

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.

Sign up for GitHub

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

Merged
merged 6 commits into from
Apr 18, 2019
Merged

Allow headers to match on ReferenceEquals before OrdinalIgnoreCase #9341

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
bbd1c8a
Allow headers to match on ReferenceEquals before trying OrdinalIgnore…
benaadams Apr 13, 2019
82f897c
Use HeaderNames more consistently
benaadams Apr 14, 2019
506456c
Feedbacks
benaadams Apr 15, 2019
c9d21ce
Internal the consts
benaadams Apr 16, 2019
6fef5a7
Merge remote-tracking branch 'refs/remotes/aspnet/master' into refere…
benaadams Apr 18, 2019
8db0ab3
undo accidental change
benaadams Apr 18, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/Antiforgery/src/Internal/DefaultAntiforgery.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -263,12 +263,12 @@ private void SaveCookieTokenAndHeader(HttpContext httpContext, string cookieToke
_tokenStore.SaveCookieToken(httpContext, cookieToken);
}

if (!_options.SuppressXFrameOptionsHeader && !httpContext.Response.Headers.ContainsKey("X-Frame-Options"))
if (!_options.SuppressXFrameOptionsHeader && !httpContext.Response.Headers.ContainsKey(HeaderNames.XFrameOptions))
{
// Adding X-Frame-Options header to prevent ClickJacking. See
// http://tools.ietf.org/html/draft-ietf-websec-x-frame-options-10
// for more information.
httpContext.Response.Headers["X-Frame-Options"] = "SAMEORIGIN";
httpContext.Response.Headers[HeaderNames.XFrameOptions] = "SAMEORIGIN";
}
}

Expand Down
3 changes: 2 additions & 1 deletion src/Hosting/Hosting/src/GenericHost/GenericWebHostedService.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
using Microsoft.Extensions.Logging;
using Microsoft.Extensions.Options;
using Microsoft.Extensions.StackTrace.Sources;
using Microsoft.Net.Http.Headers;

namespace Microsoft.AspNetCore.Hosting.Internal
{
Expand Down Expand Up @@ -184,7 +185,7 @@ private RequestDelegate BuildErrorPageApplication(Exception exception)
return context =>
{
context.Response.StatusCode = 500;
context.Response.Headers["Cache-Control"] = "no-cache";
context.Response.Headers[HeaderNames.CacheControl] = "no-cache";
return errorPage.ExecuteAsync(context);
};
}
Expand Down
13 changes: 4 additions & 9 deletions src/Hosting/Hosting/src/Internal/HostingApplicationDiagnostics.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,6 @@ internal class HostingApplicationDiagnostics
private const string DeprecatedDiagnosticsEndRequestKey = "Microsoft.AspNetCore.Hosting.EndRequest";
private const string DiagnosticsUnhandledExceptionKey = "Microsoft.AspNetCore.Hosting.UnhandledException";

private const string RequestIdHeaderName = "Request-Id";
private const string CorrelationContextHeaderName = "Correlation-Context";
private const string TraceParentHeaderName = "traceparent";
private const string TraceStateHeaderName = "tracestate";

private readonly DiagnosticListener _diagnosticListener;
private readonly ILogger _logger;

Expand Down Expand Up @@ -238,22 +233,22 @@ private Activity StartActivity(HttpContext httpContext)
{
var activity = new Activity(ActivityName);

if (!httpContext.Request.Headers.TryGetValue(TraceParentHeaderName, out var requestId))
if (!httpContext.Request.Headers.TryGetValue(HeaderNames.TraceParent, out var requestId))
{
httpContext.Request.Headers.TryGetValue(RequestIdHeaderName, out requestId);
httpContext.Request.Headers.TryGetValue(HeaderNames.RequestId, out requestId);
}

if (!StringValues.IsNullOrEmpty(requestId))
{
activity.SetParentId(requestId);
if (httpContext.Request.Headers.TryGetValue(TraceStateHeaderName, out var traceState))
if (httpContext.Request.Headers.TryGetValue(HeaderNames.TraceState, out var traceState))
{
activity.TraceStateString = traceState;
}

// We expect baggage to be empty by default
// Only very advanced users will be using it in near future, we encourage them to keep baggage small (few items)
string[] baggage = httpContext.Request.Headers.GetCommaSeparatedValues(CorrelationContextHeaderName);
string[] baggage = httpContext.Request.Headers.GetCommaSeparatedValues(HeaderNames.CorrelationContext);
if (baggage.Length > 0)
{
foreach (var item in baggage)
Expand Down
3 changes: 2 additions & 1 deletion src/Hosting/Hosting/src/Internal/WebHost.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
using Microsoft.Extensions.Hosting;
using Microsoft.Extensions.Logging;
using Microsoft.Extensions.StackTrace.Sources;
using Microsoft.Net.Http.Headers;

namespace Microsoft.AspNetCore.Hosting.Internal
{
Expand Down Expand Up @@ -276,7 +277,7 @@ private RequestDelegate BuildApplication()
return context =>
{
context.Response.StatusCode = 500;
context.Response.Headers["Cache-Control"] = "no-cache";
context.Response.Headers[HeaderNames.CacheControl] = "no-cache";
return errorPage.ExecuteAsync(context);
};
}
Expand Down
149 changes: 81 additions & 68 deletions src/Http/Headers/ref/Microsoft.Net.Http.Headers.netcoreapp3.0.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,74 +118,87 @@ public EntityTagHeaderValue(Microsoft.Extensions.Primitives.StringSegment tag, b
}
public static partial class HeaderNames
{
public const string Accept = "Accept";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is a breaking change? These can't be used in attributes anymore?

Copy link
Member

@Tratcher Tratcher Apr 14, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed. Do we have any examples where it's relevant? I have not seen these used in attributes.

Copy link
Member Author

@benaadams benaadams Apr 15, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was one in http2 tests; though that was testing the headers

image

Wouldn't think it was common? e.g. everything in ASP.NET Core repos work and this was the only change.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suppose its a choice:

  • Fast header string matching (extremely common; used in every request, 1258 uses in ASP.NET Core itself)

  • Use in attributes (extremely uncommon, 2 test uses in ASP.NET Core itself and those testing http/2 protocol psudeo headers)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It’s not common no. The only other thing is that I’d like to eventually burn the headers assembly to the ground

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Keeping it would involve duplication, which would get pretty confusing.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To caveat the scope, its source code breaking if used in attributes or other string const initalization; otherwise isn't source code breaking.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh I didn't see the whole change. I think this change is fine then. The few cases I found on the internet were not in attribute cases where a const is required. They were just regular imperative code. Carry on.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We use these quite a bit in case statements for what it's worth...though I guess not anymore. Bleh.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry :-/

public const string AcceptCharset = "Accept-Charset";
public const string AcceptEncoding = "Accept-Encoding";
public const string AcceptLanguage = "Accept-Language";
public const string AcceptRanges = "Accept-Ranges";
public const string AccessControlAllowCredentials = "Access-Control-Allow-Credentials";
public const string AccessControlAllowHeaders = "Access-Control-Allow-Headers";
public const string AccessControlAllowMethods = "Access-Control-Allow-Methods";
public const string AccessControlAllowOrigin = "Access-Control-Allow-Origin";
public const string AccessControlExposeHeaders = "Access-Control-Expose-Headers";
public const string AccessControlMaxAge = "Access-Control-Max-Age";
public const string AccessControlRequestHeaders = "Access-Control-Request-Headers";
public const string AccessControlRequestMethod = "Access-Control-Request-Method";
public const string Age = "Age";
public const string Allow = "Allow";
public const string Authority = ":authority";
public const string Authorization = "Authorization";
public const string CacheControl = "Cache-Control";
public const string Connection = "Connection";
public const string ContentDisposition = "Content-Disposition";
public const string ContentEncoding = "Content-Encoding";
public const string ContentLanguage = "Content-Language";
public const string ContentLength = "Content-Length";
public const string ContentLocation = "Content-Location";
public const string ContentMD5 = "Content-MD5";
public const string ContentRange = "Content-Range";
public const string ContentSecurityPolicy = "Content-Security-Policy";
public const string ContentSecurityPolicyReportOnly = "Content-Security-Policy-Report-Only";
public const string ContentType = "Content-Type";
public const string Cookie = "Cookie";
public const string Date = "Date";
public const string ETag = "ETag";
public const string Expect = "Expect";
public const string Expires = "Expires";
public const string From = "From";
public const string Host = "Host";
public const string IfMatch = "If-Match";
public const string IfModifiedSince = "If-Modified-Since";
public const string IfNoneMatch = "If-None-Match";
public const string IfRange = "If-Range";
public const string IfUnmodifiedSince = "If-Unmodified-Since";
public const string LastModified = "Last-Modified";
public const string Location = "Location";
public const string MaxForwards = "Max-Forwards";
public const string Method = ":method";
public const string Origin = "Origin";
public const string Path = ":path";
public const string Pragma = "Pragma";
public const string ProxyAuthenticate = "Proxy-Authenticate";
public const string ProxyAuthorization = "Proxy-Authorization";
public const string Range = "Range";
public const string Referer = "Referer";
public const string RetryAfter = "Retry-After";
public const string Scheme = ":scheme";
public const string Server = "Server";
public const string SetCookie = "Set-Cookie";
public const string Status = ":status";
public const string StrictTransportSecurity = "Strict-Transport-Security";
public const string TE = "TE";
public const string Trailer = "Trailer";
public const string TransferEncoding = "Transfer-Encoding";
public const string Upgrade = "Upgrade";
public const string UserAgent = "User-Agent";
public const string Vary = "Vary";
public const string Via = "Via";
public const string Warning = "Warning";
public const string WebSocketSubProtocols = "Sec-WebSocket-Protocol";
public const string WWWAuthenticate = "WWW-Authenticate";
public static readonly string Accept;
public static readonly string AcceptCharset;
public static readonly string AcceptEncoding;
public static readonly string AcceptLanguage;
public static readonly string AcceptRanges;
public static readonly string AccessControlAllowCredentials;
public static readonly string AccessControlAllowHeaders;
public static readonly string AccessControlAllowMethods;
public static readonly string AccessControlAllowOrigin;
public static readonly string AccessControlExposeHeaders;
public static readonly string AccessControlMaxAge;
public static readonly string AccessControlRequestHeaders;
public static readonly string AccessControlRequestMethod;
public static readonly string Age;
public static readonly string Allow;
public static readonly string Authority;
public static readonly string Authorization;
public static readonly string CacheControl;
public static readonly string Connection;
public static readonly string ContentDisposition;
public static readonly string ContentEncoding;
public static readonly string ContentLanguage;
public static readonly string ContentLength;
public static readonly string ContentLocation;
public static readonly string ContentMD5;
public static readonly string ContentRange;
public static readonly string ContentSecurityPolicy;
public static readonly string ContentSecurityPolicyReportOnly;
public static readonly string ContentType;
public static readonly string Cookie;
public static readonly string CorrelationContext;
public static readonly string Date;
public static readonly string DNT;
public static readonly string ETag;
public static readonly string Expect;
public static readonly string Expires;
public static readonly string From;
public static readonly string Host;
public static readonly string IfMatch;
public static readonly string IfModifiedSince;
public static readonly string IfNoneMatch;
public static readonly string IfRange;
public static readonly string IfUnmodifiedSince;
public static readonly string KeepAlive;
public static readonly string LastModified;
public static readonly string Location;
public static readonly string MaxForwards;
public static readonly string Method;
public static readonly string Origin;
public static readonly string Path;
public static readonly string Pragma;
public static readonly string ProxyAuthenticate;
public static readonly string ProxyAuthorization;
public static readonly string Range;
public static readonly string Referer;
public static readonly string RequestId;
public static readonly string RetryAfter;
public static readonly string Scheme;
public static readonly string SecWebSocketAccept;
public static readonly string SecWebSocketKey;
public static readonly string SecWebSocketProtocol;
public static readonly string SecWebSocketVersion;
public static readonly string Server;
public static readonly string SetCookie;
public static readonly string Status;
public static readonly string StrictTransportSecurity;
public static readonly string TE;
public static readonly string TraceParent;
public static readonly string TraceState;
public static readonly string Trailer;
public static readonly string TransferEncoding;
public static readonly string Translate;
public static readonly string Upgrade;
public static readonly string UpgradeInsecureRequests;
public static readonly string UserAgent;
public static readonly string Vary;
public static readonly string Via;
public static readonly string Warning;
public static readonly string WebSocketSubProtocols;
public static readonly string WWWAuthenticate;
public static readonly string XFrameOptions;
}
public static partial class HeaderQuality
{
Expand Down
Loading