feat(plugins/proxy-cache): add wildcard and parameter match support for content_type

[vm-001]

Summary

Add wildcard and parameter match support for content_type

Checklist

• The Pull Request has tests
• There's an entry in the CHANGELOG
• There is a user-facing docs PR against https://github.com/Kong/docs.konghq.com - PUT DOCS PR HERE

Issue reference

FTI-1131

[vm-001]

sister PR: #10030

[windmgc]

LGTM on the code wise, this needs to get merged after #10030

[hbagdi]

@vm-001 Could you please rebase?

[hanshuebner]

Does this PR overlap with #10374, @samugi ?

[samugi]

Does this PR overlap with #10374, @samugi ?

it will not create conflicts @hanshuebner because this PR is creating a new dedicated kong.tools.mime_type module and is only using it in the proxy cache plugin, it does not affect what we currently have in the pdk (moved in utils with #10374). However... we will want to unify things and only do this one way, it should be simple once we have this mime_type module to point to it from whatever function needs it in utils etc. Perhaps that should be a separate PR once these 2 are merged. What do you think?

EDIT: tracked in KAG-789

[hanshuebner]

Unification would be good. We don't want to have to implementations of basically the same thing. Please create a KAG ticket for visibility.

[fffonion]

put a block on this PR as the original perf (#10030 (comment)) doesn't seem to prove that lpeg is faster than ngx.re

[vm-001]

put a block on this PR as the original perf (#10030 (comment)) doesn't seem to prove that lpeg is faster than ngx.re

@fffonion The log of that test is reversed(due to the typo), I've just corrected that.

Here is the output from my computer:

case ngxreg: time used  : 519.99998092651 ms
case lpeg: time used  : 273.00000190735 ms

[hanshuebner]

put a block on this PR as the original perf (#10030 (comment)) doesn't seem to prove that lpeg is faster than ngx.re

@fffonion The log of that test is reversed(due to the typo), I've just corrected that.

Here is the output from my computer:

case ngxreg: time used  : 519.99998092651 ms
case lpeg: time used  : 273.00000190735 ms

The reason why lpeg is faster in this example program is that the regular expression to parse the parameters is evaluated multiple times, once for each parameter provided. If the whole matching is done by one regular expression evaluation, it is faster than lpeg. Let's not try to be too cheap when making arguments about performance.

The regular expression based version of the code also is easier to understand and reason about than the lpeg based version because it uses less named local state. Also, regular expressions are much more widely known than lpeg, which is a plus in terms of review- and maintainability.

[vm-001]

put a block on this PR as the original perf (#10030 (comment)) doesn't seem to prove that lpeg is faster than ngx.re

@fffonion The log of that test is reversed(due to the typo), I've just corrected that.
Here is the output from my computer:

case ngxreg: time used  : 519.99998092651 ms
case lpeg: time used  : 273.00000190735 ms

The reason why lpeg is faster in this example program is that the regular expression to parse the parameters is evaluated multiple times, once for each parameter provided. If the whole matching is done by one regular expression evaluation, it is faster than lpeg. Let's not try to be too cheap when making arguments about performance.

The regular expression based version of the code also is easier to understand and reason about than the lpeg based version because it uses less named local state. Also, regular expressions are much more widely known than lpeg, which is a plus in terms of review- and maintainability.

Writing a single static regular expression able to match a content type(with multiple parameters) is almost impossible. If an example content type with parameters here is to be considered cheap. Then Feel free to remove (parameters/merge_params) and do another test.

- local media_type = (types/format_types) * (parameters/merge_params) * P(-1)
+ local media_type = (types/format_types) * P(-1)

The Regular expression is indeed easy to maintain compared to Lpeg, but the media type is something RFC and hasn't been changed for a decade. That's why I think using Lpeg for this scenario is acceptable.

[hanshuebner]

Writing a single static regular expression able to match a content type(with multiple parameters) is almost impossible.

I'm not sure what you mean by that. How about

(?<type>.+) \/ (?<subtype>[^ ;]+) (?:;\s*([^= ]+)=([^; ]+))*?

The performance benefit of the lpeg version is too small to warrant an implementation that is opaque and can't be reviewed unless one learns a non-standard parsing library. There is much more to be gained spending time learning how to use regular expressions.

[vm-001]

Writing a single static regular expression able to match a content type(with multiple parameters) is almost impossible.

I'm not sure what you mean by that. How about

(?<type>.+) \/ (?<subtype>[^ ;]+) (?:;\s*([^= ]+)=([^; ]+))*?

The performance benefit of the lpeg version is too small to warrant an implementation that is opaque and can't be reviewed unless one learns a non-standard parsing library. There is much more to be gained spending time learning how to use regular expressions.

It's not working. Missing "charset = UTF-8"

local inspect = require("inspect")
local r = ngx.re.match("application/json;charset=UTF-8;Foo=Bar", [[(?<type>.+)\/(?<subtype>[^ ;]+)(?:;\s*([^= ]+)=([^; ]+))*]])
print(inspect(r))

--[[
{ "application", "json", "Foo", "Bar",
  [0] = "application/json;charset=UTF-8;Foo=Bar",
  subtype = "json",
  type = "application"
}
--]]

[hanshuebner]

That's true, and you're probably right in claiming that it is difficult to make it work in one regular expression. However, the overhead lies completely in the invocation of the regular expression parser, so reducing the number of regular expression evaluations is the key to improving performance. Regular expressions are easier to review and more universal than lpeg, and they're also faster to write, so the use of an obscure parsing library does not seem to be warranted, in particular given that the performance argument can be made both ways.