Orchardcore: Large file uploads

[deanmarcussen]

Has anyone had any luck getting the MediaController to accept large files?

I get this exception when uploading anything over 128mb

Microsoft.AspNetCore.Diagnostics.DeveloperExceptionPageMiddleware|ERROR|An unhandled exception has occurred while executing the request. System.IO.InvalidDataException: Multipart body length limit 134217728 exceeded.

I've set the OrchardCore.Media.MaxFileSize max file size.

Have tried various options including

      <security>
        <requestFiltering>
          <!-- This will handle requests up to 2GB -->
          <requestLimits maxAllowedContentLength="2072576000" />
        </requestFiltering>
      </security>      
    </system.webServer>

and

          services.Configure<FormOptions>(x =>
            {
                x.ValueLengthLimit = Int32.MaxValue;
                x.MultipartBodyLengthLimit = Int64.MaxValue; // In case of multipart
            });

with no luck

[deanmarcussen]

Figured it out.

If you need to set Microsoft.AspNetCore.Http.Features options you need to configure them in the OrchardCore.Cms.Web project or other startup project or they don't make it in

[jtkech]

I let you grab more infos here #3263 and the referenced #3282 where the usage of the [MediaSizeLimit] attribute has been removed.

• As i remember the original problem was to use

  // Forcing AntiForgery Token Validation on by default, it's only in Razor Pages by default
  options.Filters.Add(typeof(AutoValidateAntiforgeryTokenAttribute));

  That add a filter but with an order of 0, see here. But normally the order of AutoValidateAntiforgeryTokenAttribute is 1000 see here and it was intended to run after our attribute whose order is 900 as for the RequestFormLimitsAttribute see here and here for its related filter.

• So, maybe we could re-use our MediaSizeLimit attribute for the media upload action and then use.

  options.Filters.Add(typeof(AutoValidateAntiforgeryTokenAttribute), 1000);

  For testing you could just do the above change, then use the RequestFormLimitsAttribute for the media upload action

  Or re-use your FormOptions configuration and then also use.

  options.Filters.Add(typeof(AutoValidateAntiforgeryTokenAttribute), 1000);
  options.Filters.Add(typeof(RequestFormLimitsAttribute), 900);

The advantage of reusing our MediaSizeLimit attribute would be to be able to reuse e.g the media MaxRequestBodySize config value.

[deanmarcussen]

Hmm interesting reading.

I will try it with the obsolete filter and see if it possible to get FormOptions set before it becomes readonly.

Fortunately this one is easy to repro you just need a 130mb file :)

[jtkech]

Yes, but don't forget to use this in the mvc sartup, with the Order property

options.Filters.Add(typeof(AutoValidateAntiforgeryTokenAttribute), 1000);

And in MediaSizeLimitAttribute.cs fix the test line 75 by

if (maxRequestBodySizeFeature != null && !maxRequestBodySizeFeature.IsReadOnly)

[jtkech]

So i reopen it in case it is worth to try something.

[deanmarcussen]

@jtkech Initial tests look good.

The site I needed large uploads on went live yesterday so hopefully today I get a bit of downtime and a chance to tinker with the attribute to see it working in all scenarios

[jtkech]

Cool 👍

[deanmarcussen]

@jtkech ok, it took a bit of figuring out, to repro the original issue on https://github.com/OrchardCMS/OrchardCore/issues/3263, to be sure that the changes you suggested fixed it, mostly because I tend to use IISExpress or IIS.

Both IIS and IISExpress are InProcess hosted by default, and IHttpMaxRequestBodySizeFeature is a kestrel only feature, so under IIS & IISExpress it is always null, so the filter does not try and set it at all.

It's only after I shift to OutOfProcess hosting that IHttpMaxRequestBodySizeFeature is not null.

Then I can always repro the exception that was reported on https://github.com/OrchardCMS/OrchardCore/issues/3263

2019-07-20 15:51:06.9717|blog|0HLOD46GM86S4:00000005||Microsoft.AspNetCore.Diagnostics.DeveloperExceptionPageMiddleware|ERROR|An unhandled exception has occurred while executing the request. System.InvalidOperationException: The maximum request body size cannot be modified after the app has already started reading from the request body.
   at Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http.HttpProtocol.Microsoft.AspNetCore.Http.Features.IHttpMaxRequestBodySizeFeature.set_MaxRequestBodySize(Nullable`1 value)
   at OrchardCore.Media.Services.MediaSizeLimitAttribute.InternalMediaSizeFilter.OnAuthorization(AuthorizationFilterContext context) in E:\Sites\OrchardCoreV3\src\OrchardCore.Modules\OrchardCore.Media\Services\MediaSizeLimitAttribute.cs:line 77

By changing the filter order as you suggest to
options.Filters.Add(typeof(AutoValidateAntiforgeryTokenAttribute), 1000);

and fixing the MediaSizeLimitAttribute.cs
if (maxRequestBodySizeFeature != null && !maxRequestBodySizeFeature.IsReadOnly)

It fixes the above exception on small files.

If I then upload large files (200mb) it will work if I add a larger MaxRequestBodySize to the appsettings.json with OutProcess hosting (still need to increase IIS limits to match), or if I go back to InProcess hosting the MaxFileSize and IFormFeature FormOptions also works correctly.

So I'll see about reverting the changes in the MediaController and PR with the MediaSizeLimitAttribute instead.

The downside is the error reported back to the browser is just a 404

Both MaxRequestBodySize and MultipartBodyLengthLimit are in bytes, so the correct values should always be the same. So I don't see a need for both a MaxFileSize and MaxRequestBodySize appsettings.json configuration.

[jtkech]

Okay.

The downside is the error reported back to the browser is just a 404

So I don't see a need for both a MaxFileSize and MaxRequestBodySize configuration

Hmm, maybe in the attribute implementation we could have just one config value, e.g our MaxRequestBodySize, for both FormOptions.MultipartBodyLengthLimit and IHttpMaxRequestBodySizeFeature.MaxRequestBodySize, and that would be an upper limit for security.

Then still use our MaxFileSize, intended to be lower, in the media controller to not return a 404 (if the file is still lower than MaxRequestBodySize).

[deanmarcussen]

Hmm, maybe in the attribute implementation we could have just one config value, e.g our MaxRequestBodySize, for both FormOptions.MultipartBodyLengthLimit and IHttpMaxRequestBodySizeFeature.MaxRequestBodySize, and that would be an upper limit for security.

Then still use our MaxFileSize, intended to be lower, in the media controller to not return a 404 (if the file is still lower than MaxRequestBodySize).

So my mistake, it's may or may not be a 404 returned, it depends again (no one size fits all!) on whether is hosted by IIS, IIS Express, or Kestrel. IIS gives a 413 (Request Entity Too Large), or IISExpress gives a 404.13, and I suspect earlier versions of IIS will also give a 404.13, whereas Kestrel just throws an Exception and returns a 500.

I think an upper limit for security might defeat the security purpose of MaxRequestBodySize, and would still not give a user friendly error running InProcess on IIS, as IIS will still block it anyway before it hits the server / attribute.

If a user friendly error is wanted, it might be better to just check the file size in javascript before attempting the upload this.files[0].size works with HTML5 browsers. That way the controller remains secured, and the user sees a valid error.

Alternatively the media app could handle a 413 response, and display an error, and we could have a custom UseExceptionHandler to catch the exceptions that Kestrel will throw and return a 413 as well.

[jtkech]

Okay but i'm lost ;) So i let you decide, here only some suggestions.

• Anyway we need options.Filters.Add(typeof(AutoValidateAntiforgeryTokenAttribute), 1000); with the order parameter for this filter that we use globally, so that it doesn't conflict with the usage of another action attribute as e.g RequestSizeLimitAttribute or RequestFormLimitsAttribute.

• First if InProcess, as i understand if we are above the default limits, only doing a check in the controller action would not work. So, anyway we still need the MediaSizeLimitAttribute that needs to be fixed => !maxRequestBodySizeFeature.IsReadOnly. And then maybe remove the controller action checking.

  Okay, i understand that it is better to keep the 2 config values. Hmm, i thought that the default value of MaxRequestBodySize would be higher than MultipartBodyLengthLimit, but here the default values from the aspnetcore source, notice that by default this is the opposite.

  // Matches the default maxAllowedContentLength in IIS (~28.6 MB)
  // https://www.iis.net/configreference/system.webserver/security/requestfiltering/requestlimits#005
  private long? _maxRequestBodySize = 30000000;
  
  /// <summary>
  /// A limit for the length of each multipart body. Forms sections that exceed this limit will throw an
  /// <see cref="InvalidDataException"/> when parsed.
  /// </summary>
  public const long DefaultMultipartBodyLengthLimit = 1024 * 1024 * 128; // = 134 217 728‬ bytes
  public long MultipartBodyLengthLimit { get; set; } = DefaultMultipartBodyLengthLimit;
  

  Note: As i remember in 3.0 with upgraded connection _maxRequestBodySize will be unlimited by default, but i think it will be the same if using OutOfProcess (not only using Kestrel).

• If a user friendly error is wanted, it might be better to just check the file size in javascript

  Looks like a good and the only one solution, but as you want and / or in another PR.

• Then if OutOfProcess as i understand we also need to do some config at the application level, e.g for the maxAllowedContentLength in IIS as commented above in the aspnetcore source code. So, here we can't do anything, we can just comment how to do this in some documentation. Unless if we check the file size in javascript, but again as you want and / or in another PR.