fix(plugins/response-transformer): support mime type structured syntax suffix #10656
Conversation
vm-001
force-pushed
the
FTI-4959
branch
2 times, most recently
from
425ed72
to
a77d0e7
Compare
vm-001
force-pushed
the
FTI-4959
branch
2 times, most recently
from
76cf4a9
to
daf8caa
Compare
CHANGELOG.md
Outdated
| @@ -175,7 +175,9 @@ | |||
| - **Request Transformer**: honor value of untrusted_lua configuration parameter | |||
| [#10327](https://github.com/Kong/kong/pull/10327) | |||
| - **OAuth2**: fix an issue that OAuth2 token was being cached to nil while access to the wrong service first. | |||
| [#10522](https://github.com/Kong/kong/pull/10522) | |||
| [#10522](https://github.com/Kong/kong/pull/10522) | |||
| - **Response Transformer**: fix an issue that plugin does not transform the response body while upstream returns a Content-Type with +json suffix at subtype. | |||
There was a problem hiding this comment.
| - **Response Transformer**: fix an issue that plugin does not transform the response body while upstream returns a Content-Type with +json suffix at subtype. | |
| - **Response Transformer**: Support the use of structured syntax suffixes when parsing and matching content types. |
| local JSON_MEDIA_TYPES = { | ||
| { type = "application", subtype = "json" }, | ||
| { type = "application", subtype = "*+json" }, | ||
| } |
There was a problem hiding this comment.
Is it really necessary to specify that a subtype is allowed? When parsing a document of a certain media type, its structured subtypes will always be permitted. The subtype information is useful for processors that know how to deal specifically with documents of that subtype. All other processors can always treat all subtype documents as if they were of the main type. Effectively, when matching a media type without a subtype, the subtype information can always be ignored and the additional *+json media type is not required.
There was a problem hiding this comment.
It depends on how we design the includes util function. Here, the includes is not taking application/problem+json as included to application/json, since the last one does not contain the wildcard.
The utils function does not have the knowledge of which specific mime types belong to its “parent” type. It just does the wildcard match.
There was a problem hiding this comment.
The utils function does not have the knowledge of which specific mime types belong to its “parent” type. It just does the wildcard match.
As the function is designed to work with mime types, it should know thatapplication/json includes application/problem+json.
There was a problem hiding this comment.
If that logic is reused by many places, I think it makes sense to come up with another convenient function that can consider that application/json includes application/problem+json.
There was a problem hiding this comment.
I made a commit. Would you please continue to review it?
| local JSON_MEDIA_TYPES = { | ||
| { type = "application", subtype = "json" }, | ||
| { type = "application", subtype = "*+json" }, | ||
| } |
There was a problem hiding this comment.
The utils function does not have the knowledge of which specific mime types belong to its “parent” type. It just does the wildcard match.
As the function is designed to work with mime types, it should know thatapplication/json includes application/problem+json.
…s not contain a suffix and is the suffix of other_subtype
Summary
Based on the RFC(https://www.rfc-editor.org/rfc/rfc6839#section-3.1),
application/problem+jsonis a valid mime type, and is a subset ofapplication/json, can be treated like json.Checklist
Issue reference
FTI-4959