Skip to content

Add X-Forwarded-For header name #32171

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

Closed
wants to merge 2 commits into from
Closed

Conversation

DenisBalan
Copy link

tldr; add header name X-Forwarded-For as constant in HeaderNames static class.

more info https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/X-Forwarded-For

@ghost ghost added area-runtime community-contribution Indicates that the PR has been added by a community member labels Apr 26, 2021
@DenisBalan DenisBalan requested a review from a team as a code owner April 26, 2021 09:09
@davidfowl
Copy link
Member

Why? We have middleware that already translates this into the appropriate values.

@DenisBalan
Copy link
Author

Why? We have middleware that already translates this into the appropriate values.

This is useful when creating own http calls to other services, to not have magic strings, but reference to one source of truth ie HeaderNames

@@ -370,4 +370,5 @@ static readonly Microsoft.Net.Http.Headers.HeaderNames.WWWAuthenticate -> string
static readonly Microsoft.Net.Http.Headers.HeaderNames.Warning -> string!
static readonly Microsoft.Net.Http.Headers.HeaderNames.WebSocketSubProtocols -> string!
static readonly Microsoft.Net.Http.Headers.HeaderNames.XFrameOptions -> string!
static readonly Microsoft.Net.Http.Headers.HeaderNames.XForwardedFor -> string!
Copy link
Member

Choose a reason for hiding this comment

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

Should be in PublicAPI.Unshipped.txt not Shipped

tldr; add header name `X-Forwarded-For` as constant in `HeaderNames` static class.
@DenisBalan DenisBalan removed the request for review from a team April 28, 2021 16:10
@Tratcher Tratcher added the api-ready-for-review API is ready for formal API review - https://github.com/dotnet/apireviews label Apr 28, 2021
@ghost
Copy link

ghost commented Apr 28, 2021

Thank you for submitting this for API review. This will be reviewed by @dotnet/aspnet-api-review at the next meeting of the ASP.NET Core API Review group. Please ensure you take a look at the API review process documentation and ensure that:

  • The PR contains changes to the reference-assembly that describe the API change. Or, you have included a snippet of reference-assembly-style code that illustrates the API change.
  • The PR describes the impact to users, both positive (useful new APIs) and negative (breaking changes).
  • Someone is assigned to "champion" this change in the meeting, and they understand the impact and design of the change.

@Tratcher
Copy link
Member

@dotnet/aspnet-api-review, I flagged this PR because I want to discuss and document what our policy is for adding constants to a list like this.

@pranavkm pranavkm removed the api-ready-for-review API is ready for formal API review - https://github.com/dotnet/apireviews label May 3, 2021
@Tratcher
Copy link
Member

Tratcher commented May 4, 2021

@DenisBalan we reviewed this and decided to add a written policy for what should and should not be added to this list. See https://github.com/dotnet/aspnetcore/pull/32394/files. Based on that policy I'm going to close this PR. Thanks for bringing this to our attention.

@Tratcher Tratcher closed this May 4, 2021
@amcasey amcasey added area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions and removed area-runtime labels Aug 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions community-contribution Indicates that the PR has been added by a community member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants