dotnet / AspNetCore.Docs

Documentation for ASP.NET Core
https://docs.microsoft.com/aspnet/core
Creative Commons Attribution 4.0 International
12.55k stars 25.3k forks source link

Dynamic port binding solution does not make sense to me #31861

Open epsitec opened 7 months ago

epsitec commented 7 months ago

EDIT by @Rick-Anderson

Dynamic port binding

End of @Rick-Anderson edit

Description

The documentation (https://learn.microsoft.com/en-us/aspnet/core/fundamentals/servers/kestrel/endpoints?view=aspnetcore-8.0#dynamic-port-binding) suggests to use app.Run(async (context) => { ...}) which does not make sense to me.

This is setting up a request delegate as a middleware!? Do I miss something or is the proposed solution incorrect or incomplete? Trying the solution makes the app terminate immediately.

I'd rather see this as a proposed solution:

app.Start ();

var server = app.Services.GetRequiredService<IServer> ();
var serverAddressFeature = server.Features.Get<IServerAddressesFeature> ();

if (serverAddressFeature is not null)
{
    var listenAddresses = string.Join(", ", serverAddressFeature.Addresses);

    // ...
}

Page URL

https://learn.microsoft.com/en-us/aspnet/core/fundamentals/servers/kestrel/endpoints?view=aspnetcore-8.0

Content source URL

https://github.com/dotnet/AspNetCore.Docs/blob/main/aspnetcore/fundamentals/servers/kestrel/endpoints.md

Document ID

fd9cc0c2-3ea3-f437-e0ac-8789fb96faf7

Article author

@tdykstra

Rick-Anderson commented 7 months ago

@halter73 can you suggest better code? I tried the following

using Microsoft.AspNetCore.Hosting.Server.Features;

var builder = WebApplication.CreateBuilder(args);
var app = builder.Build();

app.MapGet("/", (HttpContext context) => {
    var serverAddressesFeature = context.Features.Get<IServerAddressesFeature>();
    if (serverAddressesFeature != null)
    {
        var listenAddresses = string.Join(", ", serverAddressesFeature.Addresses);
        return $"Server listening on: {listenAddresses}";
    }
    return "IServerAddressesFeature not available.";
});

app.Run();

displays IServerAddressesFeature not available.

Setting the port to 0 yields : System.InvalidOperationException: Dynamic port binding is not supported when binding to localhost. You must either bind to 127.0.0.1:0 or [::1]:0, or both. Shouldn't the doc point that out?

It doesn't make sense to to set the port to 0 on Azure, so I'm guessing that it's not commonly used.

@epsitec a PR would be welcome.

halter73 commented 6 months ago

I think the code suggested by @epsitec has is right and the documentation is wrong. IServerAddressesFeature is a server-only feature available on IServer.Features and not on HttpContext.Features or ConnectionContext.Features. If you update the sample to the following, it should work.

using Microsoft.AspNetCore.Hosting.Server;
using Microsoft.AspNetCore.Hosting.Server.Features;
using Microsoft.AspNetCore.Http.Features;

var builder = WebApplication.CreateBuilder(args);
var app = builder.Build();

var serverAddressesFeature = app.Services.GetRequiredService<IServer>()
                                .Features.GetRequiredFeature<IServerAddressesFeature>();

app.MapGet("/", () => $"Server listening on: {string.Join(", ", serverAddressesFeature.Addresses)}");

app.Run();

You could also do something like the following to avoid reallocating the string each request:

var listenAddresses = "";

app.MapGet("/", () => $"Server listening on: {listenAddresses}");

app.Start();
listenAddresses = string.Join(", ", serverAddressesFeature.Addresses);
app.WaitForShutdown();

But then you'd have a small race where a request could be processed before listenAddress is set. That too could be avoided by calling into a method that caches the string, but I don't think it's worth the complexity for asmple.

Setting the port to 0 yields : System.InvalidOperationException: Dynamic port binding is not supported when binding to localhost. You must either bind to 127.0.0.1:0 or [::1]:0, or both. Shouldn't the doc point that out?

This probably worth pointing out. I hope the exception is already pretty clear (I wrote it 😅), but we should recommend using 127.0.0.1:0 if you're doing dynamic port binding on localhost.