Skip to content
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

Add HttpContext.Endpoint property #50522

Open
1 task done
JamesNK opened this issue Sep 5, 2023 · 3 comments · May be fixed by #57565
Open
1 task done

Add HttpContext.Endpoint property #50522

JamesNK opened this issue Sep 5, 2023 · 3 comments · May be fixed by #57565
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions help candidate Indicates that the issues may be a good fit for community to help with. Requires work from eng. team

Comments

@JamesNK
Copy link
Member

JamesNK commented Sep 5, 2023

Is there an existing issue for this?

  • I have searched the existing issues

Is your feature request related to a problem? Please describe the problem.

ASP.NET Core includes the GetEndpoint and SetEndpoint extension methods for working with endpoints and HttpContext.

I think endpoints have become such an intrinsic feature to ASP.NET Core that we should consider making it a real property on HttpContext. A property would be easier to use (no need to have the right namespace for the extension method) and feel better to use compared to a method.

Describe the solution you'd like

Add HttpContext.Endpoint property.

Additional context

No response

@dotnet-issue-labeler dotnet-issue-labeler bot added the area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions label Sep 5, 2023
@JamesNK
Copy link
Member Author

JamesNK commented Sep 6, 2023

cc @davidfowl

@davidfowl
Copy link
Member

Sounds reasonable to me

@amcasey amcasey added the api-suggestion Early API idea and discussion, it is NOT ready for implementation label Sep 12, 2023
@Kahbazi Kahbazi added the help candidate Indicates that the issues may be a good fit for community to help with. Requires work from eng. team label Nov 4, 2023
@antonybstack
Copy link

antonybstack commented Dec 4, 2023

How do we feel about removing the private EndpointFeature wrapper in addition to adding the Endpoint property in the HttpContext?
see EndpointHttpContextExtensions.cs

    public static Endpoint? GetEndpoint(this HttpContext context)
    {
        ArgumentNullException.ThrowIfNull(context);

        return context.Features.Get<IEndpointFeature>()?.Endpoint;
    }    

    public static void SetEndpoint(this HttpContext context, Endpoint? endpoint)
    {
        ArgumentNullException.ThrowIfNull(context);

        var feature = context.Features.Get<IEndpointFeature>();

        if (endpoint != null)
        {
            if (feature == null)
            {
                feature = new EndpointFeature();
                context.Features.Set(feature);
   ...
   }

    private sealed class EndpointFeature : IEndpointFeature
    {
        public Endpoint? Endpoint { get; set; }
    }

Note, there currently seems to be an inconsistent pattern of accessing the endpoint value. The public extension methods GetEndpoint() and SetEndpoint() are used, and some internal code uses context.Features.Set<IEndpointFeature> and context.Features.Get<IEndpointFeature>. This can be cleaned up when public abstract Endpoint? Endpoint { get; set; } is present.

Code relying on the presence of the EndpointFeature in the feature list of the HttpContext would experience bugs. If we want to avoid this, we may need to still maintain the endpoint value in the feature set within the concrete classes implementing HttpContext, or something similar to that effect.

Refactoring the dependency on the Endpoint Feature wrapper is feasible but can break existing code. @JamesNK please let me know if I should proceed with removing the IEndpointFeature wrapper or what you advise, and I will create the PR.

@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 Feb 6, 2024
@wtgodbe wtgodbe removed the pending-ci-rerun When assigned to a PR indicates that the CI checks should be rerun label Feb 6, 2024
@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 Feb 6, 2024
@wtgodbe wtgodbe removed the pending-ci-rerun When assigned to a PR indicates that the CI checks should be rerun label Feb 13, 2024
@dotnet dotnet deleted a comment from dotnet-policy-service bot Feb 13, 2024
@dotnet dotnet deleted a comment from dotnet-policy-service bot Feb 13, 2024
This was referenced Aug 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions help candidate Indicates that the issues may be a good fit for community to help with. Requires work from eng. team
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants