Skip to content

Allow maximum published package size to be configured when using IIS#473

Merged
loic-sharma merged 3 commits intoloic-sharma:masterfrom
ApewareLtd:master
Feb 25, 2020
Merged

Allow maximum published package size to be configured when using IIS#473
loic-sharma merged 3 commits intoloic-sharma:masterfrom
ApewareLtd:master

Conversation

@RichardSinden
Copy link
Contributor

Now sets the MaxRequestBodySize for IIS on startup.

  • I've modified the startup code to set the IISServerOptions based on configuration in the appsettings.json file, with a default entry for MaxRequestBodySize with a maximum size of 250MB.
  • I've updated the documentation

Addresses #463

this IServiceCollection services,
IConfiguration configuration)
{
services.ConfigureAndValidate<IISServerOptions>(configuration.GetSection(nameof(IISServerOptions)));
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like to keep the appsettings.json file as light as possible. What do you think of setting the default of 250MB here? I think this should work:

// IIS defaults upload sizes to ~30MB. Increase it to 250MB.
// See https://github.com/aspnet/Announcements/issues/267
services.Configure<IISServerOptions>(iis=>
{
    iis.MaxRequestBodySize = 262144000
});

services.ConfigureAndValidate<IISServerOptions>(configuration.GetSection(nameof(IISServerOptions)));

I believe this should default to 250MB, yet let customers override that value by adding a IISServerOptions option.


"IISServerOptions": {
"MaxRequestBodySize": 262144000
},
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you go with my comment above https://github.com/loic-sharma/BaGet/pull/473/files#r378645236, please remove these lines since they are no longer necessary for the 250MB default.

@loic-sharma
Copy link
Owner

loic-sharma commented Feb 13, 2020

Thank you for figuring this problem out. I'm a bit of an IIS noob so I really appreciate the help! :)

Please let me know what you think of this comment #473 (comment), and I'll merge this fix in!

@RichardSinden
Copy link
Contributor Author

Yep, I agree, I wasn't entirely comfortable with always having config for IIS in the appsettings even when you are not using IIS. Your solution is better and I'll update it when I get the chance later, although I was going to wait and see if there was more information on this comment #463 (comment), in case we don't need to do anything at all.

…MB in code and remove defaults from appsettings.json
@softlion
Copy link

Is this MaxRequestBodySize not enough ?

  .ConfigureKestrel((context, options) =>
    {
        options.Limits.MaxRequestBodySize = ...;
    });

@RichardSinden
Copy link
Contributor Author

Is this MaxRequestBodySize not enough ?

No, this setting doesn't seem to have any effect when hosting in IIS.

@loic-sharma, I've updated the PR as suggested to set a default in code and remove the appsettings.json default section. I've updated the documentation to make reference to the IISServerOptions section in the Microsoft docs.

@loic-sharma
Copy link
Owner

Thanks for the contribution! :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants