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
bseptember wants to merge 7 commits into from
Closed

Include a warning about port spoofing in xml #54971

bseptember wants to merge 7 commits into from

Conversation

@bseptember
Copy link
Contributor

@bseptember bseptember commented Apr 5, 2024

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
@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
@captainsafia captainsafia enabled auto-merge (squash) April 8, 2024 16:38
@captainsafia
Copy link
Member

captainsafia commented Apr 8, 2024

@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 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 <[email protected]>
@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

captainsafia commented Apr 25, 2024

@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

@captainsafia captainsafia Jun 28, 2024

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

mkArtakMSFT commented Dec 19, 2024

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
@divyeshio divyeshio mentioned this pull request Aug 7, 2025
Merged
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tdykstra tdykstra tdykstra left review comments

@captainsafia captainsafia captainsafia approved these changes

@gewarren gewarren gewarren left review comments

@halter73 halter73 halter73 approved these changes

@javiercn javiercn Awaiting requested review from javiercn

+2 more reviewers

@martincostello martincostello martincostello left review comments

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

Reviewers whose approvals may not affect merge requirements

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