Potential StackOverflow & Wasted CPU Cycles

[speige]

In file https://github.com/AnderssonPeter/CompressedStaticFiles/blob/master/src/CompressedStaticFiles/CompressedStaticFileMiddleware.cs

These lines have a potential issue:

 var originalPrepareResponse = staticFileOptions.Value.OnPrepareResponse;
            staticFileOptions.Value.OnPrepareResponse = ctx =>
            {
                originalPrepareResponse(ctx);

The above code essentially "monkey-patches" the OnPrepareResponse. By keeping a reference to the original & adding some extra code.

The problem with this approach is that .net core instantiates a new Middleware instance on each request. However, the options.value passed to the middleware constructor is usually a singleton (depends on the programmer).

This means the 1st request will work fine, but the 2nd request will monkey-patch the already monkey-patched method. The double-monkey-patched method still works, it just wastes cpu time running the extra code 2x. With every request this overhead gets worse until a stack overflow exception eventually happens and the app pool resets.

One potential solution would be to have the constructor to CompressedStaticFileMiddleware.cs clone the original StaticFileOptions before monkey-patching.

[speige]

I believe a workaround would be to call

app.Use(async (context, next) => ...)

instead of

app.UseCompressedStaticFiles( ... )

and always instantiate a new instance of the Options parameter.

[Tratcher]

No, middleware are not recreated per request, only mvc does that with controllers. Did you actually experience this issue or was it just a theory?

[speige]

I was actually experiencing this, however I guess I was using the middleware incorrectly.

causes stack overflow:

            StaticFileOptions wwwrootStaticFileOptions = new StaticFileOptions()
            {
                FileProvider = new PhysicalFileProvider(_globalSettings.WWWROOT_PHYSICAL_PATH),
                RequestPath = new PathString(""),
                OnPrepareResponse = x => AddExpiresHeaders(x.Context, true)
            };
            app.Use(async (context, next) => await new CompressedStaticFileMiddleware(request => next(), _environment, Options.Create(wwwrootStaticFileOptions), app.ApplicationServices.GetService<ILoggerFactory>()).Invoke(context));

doesn't cause stack overflow:

            app.UseCompressedStaticFiles(wwwrootStaticFileOptions);

I'm not sure why I wasn't doing .Use instead of .UseMiddleware (or the even simpler extension method).

My mistake.

[Tratcher]

Ah, that Use pattern would recreate the middleware per request, it executes that func every request. UseMiddleware avoids this.