Closed Tratcher closed 5 years ago
When I wrote the initial PR of the Response Compression middleware, I thought about wildcards and other MIME type filtering problems.
If I can do a suggestion, I propose a class like that:
public class MimeTypeFilter
{
public void Add(string mimeType, bool enable) { ... }
public void Remove(string mimeType) { ... }
public void Clear() { ... }
public bool IsMatch(string mimeType) { ... }
}
Microsoft.AspNetCore.WebUtilities
.IsMatch
(like in Regex
) and not Contains
because the wildcards make MimeTypeFilter
not strictly a 'simple' container.f.Add("text/*", true); f.Add("text/plain", false);
), I see two solutions:
Add()
calls order, and so it checks the rules from first to last. But I think it would not be optimal in terms of performance.Add("*/*", true);
Regex
classes (slower).Add()
has not been called at least one time, IsMatch()
returns False.If you like the idea, I can prepare a PR...
I'll take a look and let you know.
I think the way IIS resolves conflicts is that it selects the most specific match.
Note Nginx does not support wildcards. Apache does: https://varvy.com/pagespeed/enable-compression.html
@Yves57 have you identified other places that would use the same MimeTypeFilter type? It looks generic, but I think the constraints on IsMatch would vary depending on the context.
Recommendation: won't-fix.
Looking at sample configurations for IIS, NGinx, Apache, etc., most include less than 10 mime types, with less than 5 per group (text/*). This isn't like static files with hundreds of entries. Manually listing types the is not a major burden. Nginx doesn't even support wildcards.
@Tratcher No I have currrently no idea of an other scenario.
About the IsMatch
algorithm, I was thinking during night (France time :-) about a more generic idea like that:
public interface IMimeTypeFilter
{
bool IsMatch(string mimeType);
}
public class IisMimeTypeFilter : IMimeTypeFilter
{
public void Add(string mimeType, bool enable) { ... }
public void Remove(string mimeType) { ... }
public void Clear() { ... }
public bool IsMatch(string mimeType)
{
_fullTypes.TryGetValue(....)
or else
_typesOnly.TryGetValue(...)
or else
check "*/*"
}
}
And maybe in the future
public class ApacheMimeTypeFilter : IMimeTypeFilter { ... }
public class MyCustomMimeTypeFilter : IMimeTypeFilter { ... }
I agree that the list seems to be short in real life, but I have seen too that Apache has a very sophisticated filter (larger than MIME types). So I suppose there are use cases?
Complicated customizations can already be achieved by overriding IResponseCompressionProvider.ShouldCompressResponse(HttpContext). We're better off keeping the API simple for the initial version. From there we can look at usage patterns.
Yes, I agree that a simple string collection is probably enough.
@Tratcher Chris so what is the current way of adding mime types of gzip compression in core? I noticed Google's pagespeed complains about my "svg"s not being compressed (a 50%-70% reduction in file size)
Thanks!
@gdoron SVG was discussed in #108 but rejected in the default MIME type list (see ResponseCompressionDefaults.cs But you can set your own list in the the middleware options.
Hi @Tratcher. I raised #311 because I had a specific requirement. Having read this thread, implementing MimeTypeFilter
as @Yves57 described above, will be a much better option. And will obviously solve my issue as well.
The way I see it working is pretty much what @Yves57 said above:
type/subtype
, type/*
and */*
.Enable
and Disable
, Disable
wins.Should I give it a go? I will also preserver the current way of setting mime type list via options, and will mark it as obsolete.
I'll defer to @glennc on this design.
In IIS a dev can specify wildcard mime types when checking if it should compress the response. Right now the middleware can only do exact matches. It would be much simpler if devs could specify wildcards.
Note in IIS it can opt in or out with wildcards, and override with specific values. E.g. text/* can be enabled and text/foo can be disabled. Most specific match wins.
RE: https://www.iis.net/configreference/system.webserver/httpcompression/dynamictypes IIS/Express Defaults: