search

dotnet / aspnetcore

ASP.NET Core is a cross-platform .NET framework for building modern cloud-based web applications on Windows, Mac, or Linux.
https://asp.net
MIT License
35.19k stars 9.93k forks source link

Read additional KestrelServerOptions from config #4765

Open Tratcher opened 6 years ago

Tratcher commented 6 years ago

The new configuration APIs will read endpoints with their certificates and the default certificate. https://github.com/aspnet/KestrelHttpServer/pull/2186

Here are other KestrelServerOptions to consider reading from config (Unsorted):

Today these must all be set in code in parallel to the configuration.

RehanSaeed commented 6 years ago

If anyone wants to set limits from config today, it can be done with a few extra lines of code:

.UseKestrel(
    (builderContext, options) =>
    {
        options.AddServerHeader = false;
        // Configure Kestrel from appsettings.json.
        options.Configure(builderContext.Configuration.GetSection("Kestrel"));

        // Configuring Limits from appsettings.json is not supported.
        // So we manually copy them from config.
        // See https://github.com/aspnet/KestrelHttpServer/issues/2216
        var kestrelOptions = builderContext.Configuration.GetSection<KestrelServerOptions>("Kestrel");
        foreach (var property in typeof(KestrelServerLimits).GetProperties())
        {
            var value = property.GetValue(kestrelOptions.Limits);
            property.SetValue(options.Limits, value);
        }
    })
  "Kestrel": {
    // Set stricter default limits to defend against various types of attacks.
    // See https://docs.microsoft.com/en-us/aspnet/core/fundamentals/servers/kestrel?view=aspnetcore-2.1&tabs=aspnetcore2x#how-to-use-kestrel-in-aspnet-core-apps
    // And https://docs.microsoft.com/en-us/dotnet/api/microsoft.aspnetcore.server.kestrel.core.kestrelserverlimits?view=aspnetcore-2.1
    "Limits": {
      "MaxConcurrentConnections": 100, // Default is null (i.e. no maximum)
      "MaxConcurrentUpgradedConnections": 100, // Default is null (i.e. no maximum)
      "MaxRequestBodySize": 10240, // 10240 = 10KB. Default is 30MB. Use [RequestSizeLimit(100000000)] attribute to use more.
      "MaxRequestHeaderCount": 50 // Default is 100
    }
  },
Tratcher commented 6 years ago

options.Configure(builderContext.Configuration.GetSection(nameof(ApplicationOptions.Kestrel))); shouldn't be needed in that example if you're calling CreateDefaultBuilder first, it's cumulative.

RehanSaeed commented 6 years ago

You're right. I'm not using it because it's too much magic for my taste but other peoples mileage may vary.

stukselbax commented 5 years ago

Any progress on issue? Will you plan to add this to WebHost.CreateDefaultBuilder() method?

Tratcher commented 5 years ago

Is there any specific fields you're interested in? We haven't seen much demand.

stukselbax commented 5 years ago

Currently we tweaked two properties:

MinRequestBodyDataRate
MaxRequestBodySize

It is reasonable to allow configure all listed settings. Who knows where it applicable? For example, if some microservice is published, there is practically no options for maintainers of running service to change those options, if circumstances require it.

stukselbax commented 5 years ago

BTW, I could not find generic method .GetSection<KestrelServerOptions>("Kestrel"), so we end up with the following (checked with 2.1.1):

UseKestrel((builderContext, options) =>
{
    var kestrelSection = builderContext.Configuration.GetSection(KestrelSectionName);
    options.Configure(kestrelSection);

    var kestrelOptions = kestrelSection.Get<KestrelServerOptions>();
    if (kestrelOptions != null)
    {
        options.AddServerHeader = kestrelOptions.AddServerHeader;
        options.AllowSynchronousIO = kestrelOptions.AllowSynchronousIO;
        options.ApplicationSchedulingMode = kestrelOptions.ApplicationSchedulingMode;
        foreach (var property in typeof(KestrelServerLimits).GetProperties())
        {
            var value = property.GetValue(kestrelOptions.Limits);
            property.SetValue(options.Limits, value);
        }
    }
});
ffMathy commented 5 years ago

I could really use this. Currently on Azure App Service with ASP .NET Core 2.2, you will get a 500.30 error on startup if you try to modify AddServerHeader to false.

Quite critical in my opinion.

Tratcher commented 5 years ago

@ffMathy do you have more information on that? It's not clear why setting AddServerHeader would be a problem or how this feature would address that.

mikkelblanne commented 4 years ago

+1. I was expecting this to work with all KestrelServerLimits properties, so I just added a Limits config section. It took me a while to figure out that it didn't work as expected. My concrete need was to set MaxRequestHeadersTotalSize (internal microservice use case).

RehanSaeed commented 4 years ago

The Kestrel documentation suggests that we can now set options on KestrelServerOptions from configuration. Can it do all options? Can this issue be closed?

RehanSaeed commented 4 years ago

As an aside, are there any plans to add some validation attributes to the KestrelServerOptions?

Tratcher commented 4 years ago

Those kestrel docs show how to read most options using the standard Options binder. We know that works for primitives like bool and int, but we should check how well it handles enums, MinDataRate, etc. before closing this.

stukselbax commented 4 years ago

Since MinDataRate has no settable properties - they will not be filled from configuration, because

The binder binds values to all of the public read/write properties of the type provided.

At lease properties for MinDataRate must have set;er. Also I don't really know if it should have public parameterless constructor.

Also I didn't find any notable Enum on KestrelServerOptions object graph (3.1).

Even @RehanSaeed mentioned that we can now set options on KestrelServerOptions from configuration, it requires to add some code to the application in order this to work. Why don't include it inside CreateDefaultBuilder(...)?

Tratcher commented 4 years ago

Kestrel has a custom config binder that's primarily used for endpoints. Additional complex settings like MinDataRate should be added there, not in CreateDefaultBuilder.

drauch commented 3 years ago

What is the status of

The Kestrel documentation suggests that we can now set options on KestrelServerOptions from configuration. Can it do all options? Can this issue be closed?

It looks like in the 5.0 docs this has been removed again and we're not able to use appsettings-configuration anymore and we need to use the workaround again. Why is that the case? Is there a solution (except the Reflection-based-workaround posted above)?

Best regards, D.R.

Tratcher commented 3 years ago

The 5.0 docs were reorganized. Check here for a example showing KestrelServerOptions from config: https://docs.microsoft.com/en-us/aspnet/core/fundamentals/servers/kestrel/options?view=aspnetcore-5.0

drauch commented 3 years ago

Could you also elaborate a tiny bit on why this is not done automatically by the default webhost builder? Why would I not want to have these options configurable?

Tratcher commented 3 years ago

Many of the properties on KestrelServerOptions are things that you would identify a fixed value for during development and are less likely to modify in production. However, the configuration for what endpoints to bind to is something that's expected to be determined and modified in the production environment.

drauch commented 3 years ago

Hmm, less likely for somebody ... very likely for others. No other options are "forbidden to be configured" (to my knowledge), this is a speciality of the KestrelServerOptions, and I don't see a good reason for deviating from the normal, expected behavior. Why not still allow them to be overridden for all the ops guys out there for which max request body size needs to be configurable? Why adding this extra step exactly for this one Options class? Maybe I'm mistaken, but this sounds like you did it for security reasons or some other distinct reason to go "out of the way" to forbid configuration of those options?

Tratcher commented 3 years ago

This issue tracks just that, reading more from config by default. My explanation above was the historical reasoning for why we enabled endpoint config first.

Why adding this extra step exactly for this one Options class?

I don't quite follow, most Options types are not bound to config by default, you do that binding in Startup.ConfigureServices or similar. Logging is the main exception I can think of where the binding is always done for you.

grounzero commented 3 years ago

Are we able to set HttpsConnectionAdapterOptions.SslProtocols via appsettings.json? The docs only contain code examples.

Also, can UseConnectionLogging() be toggled via appsettings.json too?

Tratcher commented 3 years ago

Are we able to set HttpsConnectionAdapterOptions.SslProtocols via appsettings.json? The docs only contain code examples.

Yes in 5.0. The docs for that are still in progress. See https://github.com/dotnet/aspnetcore/issues/26246.

Also, can UseConnectionLogging() be toggled via appsettings.json too?

Not today, and I don't know of any issue tracking it. Is this one that you commonly enable in production? We've mainly seen it used in development environments.

The trick with UseConnectionLogging is that there are multiple places in the pipeline it could be put and you'd get different results. E.g. it could be put before or after HTTPS decryption. Both are valid for investigating different scenarios.

shirhatti commented 2 years ago

Bumping into .NET 7 to reconsider

ghost commented 2 years ago

Thanks for contacting us.

We're moving this issue to the .NET 8 Planning milestone for future evaluation / consideration. We would like to keep this around to collect more feedback, which can help us with prioritizing this work. We will re-evaluate this issue, during our next planning meeting(s). If we later determine, that the issue has no community involvement, or it's very rare and low-impact issue, we will close it - so that the team can focus on more important and high impact issues. To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

deepeshhmehta commented 1 year ago

In Program.CS

  .UseKestrel((builderContext, options) =>
  {
      var kestrelSection = builderContext.Configuration.GetSection("Kestrel");
      options.Configure(kestrelSection);

      var kestrelOptions = kestrelSection.Get<KestrelServerOptions>();
      if (kestrelOptions is not null)
      {
          options.AddServerHeader = kestrelOptions.AddServerHeader;
          options.AllowSynchronousIO = kestrelOptions.AllowSynchronousIO;
          foreach (var property in typeof(KestrelServerLimits).GetProperties())
          {
              var value = property.GetValue(kestrelOptions.Limits);
             // many properties in KestrelServerLimits are not Settable 
              if (value is not null && property.GetSetMethod() is not null)
              {
                  property.SetValue(options.Limits, value);
              }
          }
      }
  })

In appsettings.json

"Kestrel": {
    "Limits": {
      "MaxResponseBufferSize": 2048,
      "MaxRequestBufferSize": 1048576,
      "MaxRequestLineSize": 8192,
      "MaxRequestHeadersTotalSize": 32768,
      "MaxRequestHeaderCount": 100,
      "MaxRequestBodySize": 27262946,
      "KeepAliveTimeout": "0.0:5:0",
      "RequestHeadersTimeout": "0.0:5:0",
      "MaxConcurrentConnections": null,
      "MaxConcurrentUpgradedConnections": null
    },
    "AddServerHeader": true,
    "AllowResponseHeaderCompression": true,
    "AllowSynchronousIO": false,
    "AllowAlternateSchemes": false,
    "DisableStringReuse": false,
    "ConfigurationLoader": null

  }

Feel free to add more custom values and handle them using a switch case in your logic.

Another improvement I will be doing as the second iteration is to create an extention function for webBuilder and move all the logiv away from Program.cs

I will post the logic I create in the extention here, @microsoft can feel free to finetune it and ship it as an extention in .net8

IanKemp commented 1 year ago

Can someone from Microsoft please, please, please explain to me why the MSDN docs say:

By default, Kestrel configuration is loaded from the Kestrel section using a preconfigured set of configuration providers.

yet this literally does not happen and we still, in 2023, have to use the reflection code posted here for Kestrel options to be read from configuration?

niko-la-petrovic commented 1 year ago

Would be nice if the official docs remarked more accurately on the current state of this.

hez2010 commented 10 months ago

A simplified version without needs of doing reflection by yourself:

builder.WebHost.ConfigureKestrel(options =>
{
    var kestrelSection = builder.Configuration.GetSection("Kestrel");
    options.Configure(kestrelSection);
    kestrelSection.Bind(options);
});
pinkfloydx33 commented 10 months ago

These docs include this example:

Kestrel options can also be set using a configuration provider. For example, the File Configuration Provider can load Kestrel configuration from an appsettings.json or appsettings.{Environment}.json file:

{
  "Kestrel": {
    "Limits": {
      "MaxConcurrentConnections": 100,
      "MaxConcurrentUpgradedConnections": 100
    },
    "DisableStringReuse": true
  }
}

Yet none of these settings actually bind by default. I just wasted a bunch of time trying to set Max Request Body/Line size settings from configuration which of course wasn't working. I tried the example above exactly and verified that even those don't load. I chased down the code in this repo and it looks like most configuration loading is still w/r/t ListenOptions and endpoints

Ghostbird commented 7 months ago

I'd love to see this too. Today a customer ran into a maximum file upload size issue. If this had been working, I could have seamlessly fixed this in a minute. Now I have to deploy a new production version of the API.

UPDATE: The code by @hez2010 works very nicely. Now we don't need to a code change if we need to alter these parameters again.

I made one change, I used the overload of ConfigureKestrel that injects the WebHostBuilderContext so it can be used fluently. Our program code looks like this:

Host
.CreateDefaultBuilder(args)
.ConfigureWebHostDefaults((webBuilder) => webBuilder
  .UseStartup<Foo>()
  .ConfigureKestrel((context, options) =>
  {
    var kestrelSection = context.Configuration.GetSection("Kestrel");
    options.Configure(kestrelSection);
    kestrelSection.Bind(options);
  })
)
.Build()
.Run();
houbi56 commented 3 months ago

Are we able to set HttpsConnectionAdapterOptions.SslProtocols via appsettings.json? The docs only contain code examples.

Yes in 5.0. The docs for that are still in progress. See #26246.

Also, can UseConnectionLogging() be toggled via appsettings.json too?

Not today, and I don't know of any issue tracking it. Is this one that you commonly enable in production? We've mainly seen it used in development environments.

The trick with UseConnectionLogging is that there are multiple places in the pipeline it could be put and you'd get different results. E.g. it could be put before or after HTTPS decryption. Both are valid for investigating different scenarios.

This is an important scenario for us when deploying apps in customers environment. Specifically, enabling connection logging/tracing in a running app. It seems that the configuration was developed code first instead of configuration first. Options only configurable through code requires a recompile and deployment.

Could we see a change of direction here for NET 9 where appsettings is a first class citizen?

hez2010 commented 3 months ago

Today a customer ran into a maximum file upload size issue

As for file uploading (through multipart-form), it is limited by another options called FormOptions, which need to be configured separately.

Ghostbird commented 3 months ago

Today a customer ran into a maximum file upload size issue

As for file uploading (through multipart-form), it is limited by another options called FormOptions, which need to be configured separately.

Good to know. We didn't run into this in our case, since it's a ReST-ish POST to our web API where the file upload data is the request body itself and the necessary metadata is part of the URI.