Closed cseufert closed 4 years ago
This should close #1
Thanks for your contribution and sorry for the delay of my response. This pull request contain multiple changes, some of them I am not convinced.
with*
syntax.
(new Middlewares\GzipEncoder())->contentType([
'application/json',
'text/html'
]),
If you want to do these changes (keep only the contentType functionality), I'll be happy to merge it. Thanks!
I agree with you about the content-length header.
I am not so sure about the Vary header though, how does another middleware know that the output encoding was changed based on reading the Accept header? Seems awkward to pass that information to another middleware, and have an interdependency to use this middleware
I have updated the configuration to allow either regex or list of content-types
Ok, thanks again for the changes. I have two suggestions:
Vary
header should be optional and enabled with $encoder->vary()
.$encoder->contentType(
'application/json',
'text/*'
),
So, if a content type has a *
, it could be converted as a regexp in this way:
if (strpos($type, '*') !== false) {
$regexp = '#^'.str_replace('*', '.*').'$#'; // #^text/.*$#
preg_match($regexp, $contentType);
}
I have refactored this to a single contentType function, and it now checks the first character, if its a slash it treats the string as a regex, else it does a string comparison.
I think it's a bad idea to have the Vary header as opt-in, and it really does not make sense to have it opt-out either.
Ok, thanks for your contribution!
Middleware not looks at Content-Type to determine if its worth compressing the response, and also adds a Vary: Content-Encoding if its not already present.