Skip to content

Include a warning about port spoofing in xml #54971

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

Jump to bottom
Closed
wants to merge 7 commits into from
Closed

Include a warning about port spoofing in xml #54971

wants to merge 7 commits into from

Conversation

bseptember
Copy link
Contributor

Include a warning about port spoofing in xml

  • You've read the Contributor Guide and Code of Conduct.
  • You've included unit or integration tests for your change, where applicable.
  • You've included inline docs for your change, where applicable.
  • There's an open issue for the PR that you are making. If you'd like to propose a new feature or change, please open an issue to discuss the change or find an existing issue.

Summary of the changes

Added remark which describes the warning as per https://learn.microsoft.com/en-us/aspnet/core/host-and-deploy/health-checks?view=aspnetcore-7.0#require-host

Description

Update API remarks for RoutingEndpointConventionBuilderExtensions.RequireHost to warn about host spoofing

Fixes #48818 (documentation changed)

@bseptember bseptember requested a review from javiercn as a code owner April 5, 2024 13:59
@ghost ghost added the area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions label Apr 5, 2024
@dotnet-policy-service dotnet-policy-service bot added the community-contribution Indicates that the PR has been added by a community member label Apr 5, 2024
@bseptember @martincostello
Docs: De-localize and de-version links
65675fd 
Co-authored-by: Martin Costello <martin@martincostello.com>
@amcasey
Copy link
Member

amcasey commented Apr 5, 2024

Seems like something @captainsafia would have thoughts on.

Also, do we know why those crefs aren't binding?

captainsafia
captainsafia reviewed Apr 5, 2024
View reviewed changes
Copy link
Member

@captainsafia captainsafia left a comment

Choose a reason for hiding this comment

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

Also tagging @Rick-Anderson and @tdykstra for doc review.

tdykstra
tdykstra reviewed Apr 6, 2024
View reviewed changes
bseptember and others added 3 commits April 6, 2024 08:19
@bseptember @tdykstra
Docs: fix to plural sentence
14c9d81 
Co-authored-by: Tom Dykstra <tdykstra@microsoft.com>
@bseptember @tdykstra
Docs: prompting a more immediate response to the issue
94e53d1 
Co-authored-by: Tom Dykstra <tdykstra@microsoft.com>
@bseptember
Fix: use list tag and add directive
ca6657d
@captainsafia captainsafia enabled auto-merge (squash) April 8, 2024 16:38
@captainsafia
Copy link
Member

@dotnet/aspnet-build CI status seems off on this PR. Can I get help merging since it's just a docs change?

halter73
halter73 requested changes Apr 8, 2024
View reviewed changes
@captainsafia captainsafia disabled auto-merge April 8, 2024 22:32
@bseptember @halter73
Fix: Enhanced security measures to prevent host and port spoofing
f693081 
Co-authored-by: Stephen Halter <halter73@gmail.com>
@bseptember bseptember requested a review from halter73 April 11, 2024 10:10
Rick-Anderson
Rick-Anderson suggested changes Apr 12, 2024
View reviewed changes
@bseptember @Rick-Anderson
docs: change in the structure and approach to explaining the safeguar…
d732ff9 
…ds against host and port spoofing presents the information in a more continuous narrative

Co-authored-by: Rick Anderson <3605364+Rick-Anderson@users.noreply.github.com>
@dotnet-policy-service dotnet-policy-service bot added the pending-ci-rerun When assigned to a PR indicates that the CI checks should be rerun label Apr 23, 2024
@captainsafia
Copy link
Member

@bseptember It looks like there is an XML syntax issue on the PR. Would you be able to fix it? Otherwise, I can make a modification in this branch.

captainsafia
captainsafia reviewed Jun 28, 2024
View reviewed changes
src/Http/Routing/src/Builder/RoutingEndpointConventionBuilderExtensions.cs
/// </description></item>
/// </list>
/// </remarks>
/// </remarks>
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
/// </remarks>

@mkArtakMSFT
Copy link
Contributor

Closing this PR, as we haven't received a response from the author for quite a while.
@bseptember feel free to reopen and resolve the remaining minor issues so that we can move forward with this PR.

@dotnet-policy-service dotnet-policy-service bot added this to the 10.0-preview1 milestone Dec 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@martincostello martincostello martincostello left review comments

@tdykstra tdykstra tdykstra left review comments

@captainsafia captainsafia captainsafia approved these changes

@gewarren gewarren gewarren left review comments

@halter73 halter73 halter73 approved these changes

@Rick-Anderson Rick-Anderson Rick-Anderson approved these changes

@javiercn javiercn Awaiting requested review from javiercn

Assignees

@adityamandaleeka adityamandaleeka

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 pending-ci-rerun When assigned to a PR indicates that the CI checks should be rerun
Projects
None yet
Milestone
10.0-preview1
Development

Successfully merging this pull request may close these issues.

Update API remarks for RoutingEndpointConventionBuilderExtensions.RequireHost to warn about host spoofing
10 participants
@bseptember @amcasey @captainsafia @mkArtakMSFT @halter73 @martincostello @tdykstra @Rick-Anderson @gewarren @adityamandaleeka