Unexpected overriding the default MaxRequestBodySize value for Kestrel

[wtct]

Which Umbraco version are you using?

13.9.2

Bug summary

We have noticed that in the ConfigureKestrelServerOptions class the MaxRequestBodySize is being overridden from 28.6 MB to 50 MB once the Umbraco:CMS:Runtime:MaxRequestLength is not set in the appsettings.json file.

Therefore, it can cause unexpected behavior especially the documentation is referring 28.6 MB here:

https://docs.umbraco.com/umbraco-cms/reference/configuration/maximumuploadsizesettings#using-kestrel

I was trying to point this out in the #20343, but a separate issue were suggested by @AndyButland.

You can find more details in our article under the Documentation inconsistency heading.

In my opinion it would be better to not touch at all the MaxRequestBodySize option if the Umbraco:CMS:Runtime:MaxRequestLength is not set.

I hope it makes sense.

cc: @piotrbach

Specifics

No response

Steps to reproduce

1. Make sure that the Umbraco:CMS:Runtime:MaxRequestLength is not set in the appsettings.json.
2. Try to upload larger file than 28.6 MB e.g. 30 MB once Umbraco is hosted on Kestrel.
3. Ensure that file is being uploaded without any issue.

Expected result / actual result

Expected result:

You should not be able to upload a file larger that 28.6MB in Umbraco hosted on Kestrel if the Umbraco:CMS:Runtime:MaxRequestLength is not set in the appsettings.json as described in documentation:

https://docs.umbraco.com/umbraco-cms/reference/configuration/maximumuploadsizesettings#using-kestrel

[github-actions]

Hi there @wtct!

Firstly, a big thank you for raising this issue. Every piece of feedback we receive helps us to make Umbraco better.

We really appreciate your patience while we wait for our team to have a look at this but we wanted to let you know that we see this and share with you the plan for what comes next.

• We'll assess whether this issue relates to something that has already been fixed in a later version of the release that it has been raised for.
• If it's a bug, is it related to a release that we are actively supporting or is it related to a release that's in the end-of-life or security-only phase?
• We'll replicate the issue to ensure that the problem is as described.
• We'll decide whether the behavior is an issue or if the behavior is intended.

We wish we could work with everyone directly and assess your issue immediately but we're in the fortunate position of having lots of contributions to work with and only a few humans who are able to do it. We are making progress though and in the meantime, we will keep you in the loop and let you know when we have any questions.

Thanks, from your friendly Umbraco GitHub bot 🤖 🙂

[AndyButland]

Thanks @wtct. I've had a look through the history of this, and seems it was deliberate, to align settings with Umbraco 8 as best we could - see #11802

I also feel we can't change it in a minor, as it could break existing setups that are depending on the higher value.

We could change it in a major, e.g. 17, but it could be easily missed and again an upgraded project gets a change in behaviour they may not expect.

So overall I think we should leave this as is, but clearly we should document it correctly. I'll do that and close this issue as, for the reasons noted above, I don't think it makes sense to change behaviour in the CMS.

[AndyButland]

Docs PR is here: umbraco/UmbracoDocs#7465

[wtct]

Hi @AndyButland, I have reviewed the history before raising this issue. Basically, it's about the default value for Kestrel to be the same as for IIS. However, the default value for IIS is 28.6 MB as you can confirm here:

https://learn.microsoft.com/en-us/iis/configuration/system.webserver/security/requestfiltering/requestlimits/#attributes

I'm not sure why it has been set in the Umbraco to 50 MB, so it looks as a mistake.

I understand that it could be recognized as a breaking change for some project, because it will change the limit of uploading. However, relying on the default value which is not aligned with the IIS is rather not a good option.

Moreover, it has been documented as 28.6MB, so probably no one is aware of the real limit. The 28.6 MB is expected, but it's not.

It the documentation you can see a specific information as below:

If you want to upload files larger than 28.6MB, update this value in the appsettings.json file.

So it will not change anything in the particular projects which are configured as suggested in the documentation.

Besides, once included in the minor release, it will just require setting the Umbraco:CMS:Runtime:MaxRequestLength option explicitly together with the upgrade if the limit around 50 MB in a specific project was crucial even no one was aware of that.

cc: @piotrbach

[AndyButland]

I don't think it was a mistake - see #11802 (comment) - where the discussion was about making sure that the default matched what was in Umbraco 8, not necessarily the IIS default.

Don't forget that even for minor releases, people do have them run automatically on Umbraco Cloud, so we have to be super-careful about anything that could be considered a breaking change. Hence I feel we could only change this in the next major, for 17.

I'll tag @nikolajlauridsen and @Zeegaan for their view - just as they were in that original discussion and may recall a bit more than was commented online - please let me know what you think gentlemen. Maybe we should change it for 17. But even then I don't know we need to, so long as we correct the documentation, which I've now raised a PR to do so.

[nikolajlauridsen]

Must admit, my memory from three years ago is a little hazy 🤣 but I just had a chat with @Zeegaan and we're pretty sure it was done to align as much as possible with V8, so if you migrated from V8 IIS to V9 Kestrel, that setting would remain the same.

I think your approach is right @AndyButland in fixing this in the documentation. I mean this is not the hill I want to die on, but I think we should ask ourself, what do we gain from making this breaking change in V17? I don't think this would provide any tangible value. However our docs should of course be accurate 😄

[AndyButland]

OK, thanks @nikolajlauridsen. Then I think we'll leave it as is and just make sure our documentation isn't misleading people.