-
Notifications
You must be signed in to change notification settings - Fork 2.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(gzip): support special * to match any type #4817
Conversation
apisix/plugins/gzip.lua
Outdated
}, | ||
}, | ||
{ | ||
type = "string", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think enum = {"*"}
is enough?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@spacewander Yes, I think it's simpler to use {"*"}
, this pr only try to follow nginx's config strictly.
If that not import, I will change it to {"*"}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
misunderstood you firstly, now i got it.
t/plugin/gzip.t
Outdated
@@ -436,6 +436,53 @@ POST /echo | |||
012345678 | |||
--- more_headers | |||
Accept-Encoding: gzip | |||
Content-Type: text/html |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Better to use a special type to verify that any type can be matched.
What this PR does / why we need it:
right now, type of types attr is array, and we dont't have a method to support the special
gzip_types *
config in Nginx Core. So maybe we can change type of types to support single string, and support * too.Pre-submission checklist: