Add HttpConnectAttribute

[BrennanConroy]

Background and Motivation

We recently added WebSockets over HTTP/2 support to Kestrel. By default this lights up automatically if your app supports HTTP/2 and you use the browser. One potential problem is that using WebSockets with controllers could potentially stop working because WebSockets over HTTP/2 now uses a CONNECT request instead of a GET request.

Our docs even show using WebSockets from a controller and are now broken in this specific scenario.

Proposed API

namespace Microsoft.AspNetCore.Mvc;

+ public class HttpConnectAttribute : HttpMethodAttribute
+ {
+    public HttpConnectAttribute();

+    public HttpConnectAttribute([StringSyntax("Route")] string template);
+ }

Usage Examples

public class WebSocketController : ControllerBase
{
    [HttpGet("/ws")]
    [HttpConnect("/ws")]
    public async Task Get()
    {
        if (HttpContext.WebSockets.IsWebSocketRequest)
        {
            using var webSocket = await HttpContext.WebSockets.AcceptWebSocketAsync();
            await Echo(webSocket);
        }
        else
        {
            HttpContext.Response.StatusCode = StatusCodes.Status400BadRequest;
        }
    }
}

Alternative Designs

public class WebSocketController : ControllerBase
{
    [WebSocket("/ws")]
    public async Task Get()
    {
        using var webSocket = await HttpContext.WebSockets.AcceptWebSocketAsync();
        await Echo(webSocket);
    }
}

Don't add a new API and add docs to show using a custom HttpMethodAttribute.

Risks

[davidfowl]

Oh very interesting. We should do this for rc2.

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.

[captainsafia]

Triage: at minimum, we'll try to document the workaround. We'll also do the additive API change for RC2.

Note: we'll also need to file a breaking change announcement with this.

[halter73]

I don't see this as critical for .NET 7 considering that it is quite easy to work around this issue by accepting all HTTP methods and checking HttpContext.Request.Method in the action. But if we do this, I think we also need to make sure we've finished the last checkbox on #42101 (comment). Thankfully, it looks like that's also slated for rc2. @Tratcher

[captainsafia]

I don't see this as critical for .NET 7 considering that it is quite easy to work around this issue by accepting all HTTP methods and checking HttpContext.Request.Method in the action

During triage, we had discussed that we would need to document this at a minimum. And if we're going to document the issue and provide a workaround, we might as well make the workaround something a little more bullet-proof/accessible.

It's fair to say this isn't critical but I wanted to provide some context into why this was the case.

But if we do this, I think we also need to make sure we've finished the last checkbox on #42101 (comment).

How is that item related to this?