kingluo commented on code in PR #9669:
URL: https://github.com/apache/apisix/pull/9669#discussion_r1233938449
##########
apisix/plugins/body-transformer.lua:
##########
@@ -123,6 +128,8 @@ local function transform(conf, body, typ, ctx)
core.log.error(err, ", body=", body)
return nil, 400, err
end
+ else
+ core.log.warn("no input format to parse ", typ, " body")
Review Comment:
The input_format nil case is for manual parsing by the user via template
code block, which is the expected case in the proposal! For example, for the
time being, we don't support yaml parsing, then what if the user needs to parse
himself? We should enable him to do this via this plugin, but not 500! We
cannot handle all content types in the world. And you should not assume the
content type exists, especially for unknown and unmanaged upstream websites.
Since we cannot support all types to parse the body, what's the meaning to
ensure content_type or input_format exists? After all, for unknown types, the
user is in charge of parsing via the template.
In TEST 12, the XML format is expected, so it's expected to accept
input_format as XML. The user should ensure the template matches the actual
input content. Think that what if the user composes an invalid template even
for the correct type and content, it will still raise errors for non-exist
fields! Then how to handle this case? That is, you cannot avoid such errors you
show only via configuration sanity check!
**And, the most important is, please do not argue the proposal and
requirement in this PR, if you're in doubt, you can propose your opinion via
the mailing list and describe your change request there!**
**This PR is for empty xml value bugfix only!**
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
