You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@apisix.apache.org by "kingluo (via GitHub)" <gi...@apache.org> on 2023/06/15 08:13:26 UTC
[GitHub] [apisix] kingluo opened a new pull request, #9669: fix(body-transformer): xml2lua: replace empty table with empty string
kingluo opened a new pull request, #9669:
URL: https://github.com/apache/apisix/pull/9669
### Description
<!-- Please include a summary of the change and which issue is fixed. -->
<!-- Please also include relevant motivation and context. -->
Fixes #9625
### Checklist
- [x] I have explained the need for this PR and the problem it solves
- [x] I have explained the changes or the new features added to this PR
- [x] I have added tests corresponding to this change
- [ ] I have updated the documentation to reflect this change
- [x] I have verified that this change is backward compatible (If not, please discuss on the [APISIX mailing list](https://github.com/apache/apisix/tree/master#community) first)
<!--
Note
1. Mark the PR as draft until it's ready to be reviewed.
2. Always add/update tests for any changes unless you have a good reason.
3. Always update the documentation to reflect the changes made in the PR.
4. Make a new commit to resolve conversations instead of `push -f`.
5. To resolve merge conflicts, merge master instead of rebasing.
6. Use "request review" to notify the reviewer after making changes.
7. Only a reviewer can mark a conversation as resolved.
-->
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] monkeyDluffy6017 commented on a diff in pull request #9669: fix(body-transformer): xml2lua: replace empty table with empty string
Posted by "monkeyDluffy6017 (via GitHub)" <gi...@apache.org>.
monkeyDluffy6017 commented on code in PR #9669:
URL: https://github.com/apache/apisix/pull/9669#discussion_r1233805304
##########
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:
It will return 500 if there are no `content-type` and `input_format`
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] kingluo commented on a diff in pull request #9669: fix(body-transformer): xml2lua: replace empty table with empty string
Posted by "kingluo (via GitHub)" <gi...@apache.org>.
kingluo commented on code in PR #9669:
URL: https://github.com/apache/apisix/pull/9669#discussion_r1233802765
##########
t/plugin/body-transformer.t:
##########
@@ -821,3 +821,75 @@ location /demo {
assert(data.raw_body == '{"result": "hello world"}')
}
}
+
+
+
+=== TEST 12: empty xml value should be rendered as empty string
+--- config
+location /demo {
+ content_by_lua_block {
+ ngx.print([[
+<SOAP-ENV:Envelope xmlns:SOAP-ENV="http://schemas.xmlsoap.org/soap/envelope/" xmlns:xsd="http://www.w3.org/2001/XMLSchema" xmlns:xrd="http://x-road.eu/xsd/xroad.xsd" xmlns:prod="http://rr.x-road.eu/producer" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmlns:id="http://x-road.eu/xsd/identifiers" xmlns:repr="http://x-road.eu/xsd/representation.xsd" xmlns:SOAP-ENC="http://schemas.xmlsoap.org/soap/encoding/">
+ <SOAP-ENV:Body>
+ <prod:RR58isikEpiletResponse>
+ <request><Isikukood>33333333333</Isikukood></request>
+ <response>
+ <Isikukood>33333333333</Isikukood>
+ <KOVKood></KOVKood>
+ </response>
+ </prod:RR58isikEpiletResponse>
+ </SOAP-ENV:Body>
+</SOAP-ENV:Envelope>
+ ]])
+ }
+}
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin")
+
+ local rsp_template = ngx.encode_base64[[
+{ "KOVKood":"{{Envelope.Body.RR58isikEpiletResponse.response.KOVKood}}" }
+ ]]
+
+ local code, body = t.test('/apisix/admin/routes/1',
+ ngx.HTTP_PUT,
+ string.format([[{
+ "uri": "/ws",
+ "plugins": {
+ "proxy-rewrite": {
+ "uri": "/demo"
+ },
+ "body-transformer": {
+ "response": {
+ "input_format": "xml",
+ "template": "%s"
+ }
+ }
+ },
+ "upstream": {
+ "type": "roundrobin",
+ "nodes": {
+ "127.0.0.1:%d": 1
+ }
+ }
+ }]], rsp_template, ngx.var.server_port)
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ return
+ end
+ ngx.sleep(0.5)
+
+ local core = require("apisix.core")
+ local http = require("resty.http")
+ local uri = "http://127.0.0.1:" .. ngx.var.server_port .. "/ws"
+ local opt = {method = "GET"}
+ local httpc = http.new()
+ local res = httpc:request_uri(uri, opt)
+ assert(res.status == 200)
+ local data1 = core.json.decode(res.body)
+ local data2 = core.json.decode[[{"KOVKood":""}]]
+ assert(core.json.stably_encode(data1) == core.json.stably_encode(data2))
+ }
+ }
Review Comment:
What's your opinion? Why make this change? Please describe it. Don't leave a long code block without any description. I can not figure what's the difference.
##########
t/plugin/body-transformer.t:
##########
@@ -821,3 +821,75 @@ location /demo {
assert(data.raw_body == '{"result": "hello world"}')
}
}
+
+
+
+=== TEST 12: empty xml value should be rendered as empty string
+--- config
+location /demo {
+ content_by_lua_block {
+ ngx.print([[
+<SOAP-ENV:Envelope xmlns:SOAP-ENV="http://schemas.xmlsoap.org/soap/envelope/" xmlns:xsd="http://www.w3.org/2001/XMLSchema" xmlns:xrd="http://x-road.eu/xsd/xroad.xsd" xmlns:prod="http://rr.x-road.eu/producer" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmlns:id="http://x-road.eu/xsd/identifiers" xmlns:repr="http://x-road.eu/xsd/representation.xsd" xmlns:SOAP-ENC="http://schemas.xmlsoap.org/soap/encoding/">
+ <SOAP-ENV:Body>
+ <prod:RR58isikEpiletResponse>
+ <request><Isikukood>33333333333</Isikukood></request>
+ <response>
+ <Isikukood>33333333333</Isikukood>
+ <KOVKood></KOVKood>
+ </response>
+ </prod:RR58isikEpiletResponse>
+ </SOAP-ENV:Body>
+</SOAP-ENV:Envelope>
+ ]])
+ }
+}
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin")
+
+ local rsp_template = ngx.encode_base64[[
+{ "KOVKood":"{{Envelope.Body.RR58isikEpiletResponse.response.KOVKood}}" }
+ ]]
+
+ local code, body = t.test('/apisix/admin/routes/1',
+ ngx.HTTP_PUT,
+ string.format([[{
+ "uri": "/ws",
+ "plugins": {
+ "proxy-rewrite": {
+ "uri": "/demo"
+ },
+ "body-transformer": {
+ "response": {
+ "input_format": "xml",
+ "template": "%s"
+ }
+ }
+ },
+ "upstream": {
+ "type": "roundrobin",
+ "nodes": {
+ "127.0.0.1:%d": 1
+ }
+ }
+ }]], rsp_template, ngx.var.server_port)
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ return
+ end
+ ngx.sleep(0.5)
+
+ local core = require("apisix.core")
+ local http = require("resty.http")
+ local uri = "http://127.0.0.1:" .. ngx.var.server_port .. "/ws"
+ local opt = {method = "GET"}
+ local httpc = http.new()
+ local res = httpc:request_uri(uri, opt)
+ assert(res.status == 200)
+ local data1 = core.json.decode(res.body)
+ local data2 = core.json.decode[[{"KOVKood":""}]]
+ assert(core.json.stably_encode(data1) == core.json.stably_encode(data2))
+ }
+ }
Review Comment:
What's your opinion? Why make this change? Please describe it. Don't leave a long code block without any description. I can not figure out what's the difference.
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] monkeyDluffy6017 commented on a diff in pull request #9669: fix(body-transformer): xml2lua: replace empty table with empty string
Posted by "monkeyDluffy6017 (via GitHub)" <gi...@apache.org>.
monkeyDluffy6017 commented on code in PR #9669:
URL: https://github.com/apache/apisix/pull/9669#discussion_r1233894569
##########
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 `conf[typ].input_format` is set by `set_input_format`, so if there is no `content_type` and `input_format`, the `conf[typ].input_format` will be nil, if the `format` is nil, the `out` will get wrong.
When i remove the parameter `input_format` of test 12 which you just added, i get this error:
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] monkeyDluffy6017 commented on a diff in pull request #9669: fix(body-transformer): xml2lua: replace empty table with empty string
Posted by "monkeyDluffy6017 (via GitHub)" <gi...@apache.org>.
monkeyDluffy6017 commented on code in PR #9669:
URL: https://github.com/apache/apisix/pull/9669#discussion_r1233808628
##########
t/plugin/body-transformer.t:
##########
@@ -821,3 +821,75 @@ location /demo {
assert(data.raw_body == '{"result": "hello world"}')
}
}
+
+
+
+=== TEST 12: empty xml value should be rendered as empty string
+--- config
+location /demo {
+ content_by_lua_block {
+ ngx.print([[
+<SOAP-ENV:Envelope xmlns:SOAP-ENV="http://schemas.xmlsoap.org/soap/envelope/" xmlns:xsd="http://www.w3.org/2001/XMLSchema" xmlns:xrd="http://x-road.eu/xsd/xroad.xsd" xmlns:prod="http://rr.x-road.eu/producer" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmlns:id="http://x-road.eu/xsd/identifiers" xmlns:repr="http://x-road.eu/xsd/representation.xsd" xmlns:SOAP-ENC="http://schemas.xmlsoap.org/soap/encoding/">
+ <SOAP-ENV:Body>
+ <prod:RR58isikEpiletResponse>
+ <request><Isikukood>33333333333</Isikukood></request>
+ <response>
+ <Isikukood>33333333333</Isikukood>
+ <KOVKood></KOVKood>
+ </response>
+ </prod:RR58isikEpiletResponse>
+ </SOAP-ENV:Body>
+</SOAP-ENV:Envelope>
+ ]])
+ }
+}
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin")
+
+ local rsp_template = ngx.encode_base64[[
+{ "KOVKood":"{{Envelope.Body.RR58isikEpiletResponse.response.KOVKood}}" }
+ ]]
+
+ local code, body = t.test('/apisix/admin/routes/1',
+ ngx.HTTP_PUT,
+ string.format([[{
+ "uri": "/ws",
+ "plugins": {
+ "proxy-rewrite": {
+ "uri": "/demo"
+ },
+ "body-transformer": {
+ "response": {
+ "input_format": "xml",
+ "template": "%s"
+ }
+ }
+ },
+ "upstream": {
+ "type": "roundrobin",
+ "nodes": {
+ "127.0.0.1:%d": 1
+ }
+ }
+ }]], rsp_template, ngx.var.server_port)
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ return
+ end
+ ngx.sleep(0.5)
+
+ local core = require("apisix.core")
+ local http = require("resty.http")
+ local uri = "http://127.0.0.1:" .. ngx.var.server_port .. "/ws"
+ local opt = {method = "GET"}
+ local httpc = http.new()
+ local res = httpc:request_uri(uri, opt)
+ assert(res.status == 200)
+ local data1 = core.json.decode(res.body)
+ local data2 = core.json.decode[[{"KOVKood":""}]]
+ assert(core.json.stably_encode(data1) == core.json.stably_encode(data2))
+ }
+ }
Review Comment:
I format the code for you, just copy and paste it
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] kingluo commented on a diff in pull request #9669: fix(body-transformer): xml2lua: replace empty table with empty string
Posted by "kingluo (via GitHub)" <gi...@apache.org>.
kingluo commented on code in PR #9669:
URL: https://github.com/apache/apisix/pull/9669#discussion_r1233816020
##########
t/plugin/body-transformer.t:
##########
@@ -821,3 +821,75 @@ location /demo {
assert(data.raw_body == '{"result": "hello world"}')
}
}
+
+
+
+=== TEST 12: empty xml value should be rendered as empty string
+--- config
+location /demo {
+ content_by_lua_block {
+ ngx.print([[
+<SOAP-ENV:Envelope xmlns:SOAP-ENV="http://schemas.xmlsoap.org/soap/envelope/" xmlns:xsd="http://www.w3.org/2001/XMLSchema" xmlns:xrd="http://x-road.eu/xsd/xroad.xsd" xmlns:prod="http://rr.x-road.eu/producer" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmlns:id="http://x-road.eu/xsd/identifiers" xmlns:repr="http://x-road.eu/xsd/representation.xsd" xmlns:SOAP-ENC="http://schemas.xmlsoap.org/soap/encoding/">
+ <SOAP-ENV:Body>
+ <prod:RR58isikEpiletResponse>
+ <request><Isikukood>33333333333</Isikukood></request>
+ <response>
+ <Isikukood>33333333333</Isikukood>
+ <KOVKood></KOVKood>
+ </response>
+ </prod:RR58isikEpiletResponse>
+ </SOAP-ENV:Body>
+</SOAP-ENV:Envelope>
+ ]])
+ }
+}
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin")
+
+ local rsp_template = ngx.encode_base64[[
+{ "KOVKood":"{{Envelope.Body.RR58isikEpiletResponse.response.KOVKood}}" }
+ ]]
+
+ local code, body = t.test('/apisix/admin/routes/1',
+ ngx.HTTP_PUT,
+ string.format([[{
+ "uri": "/ws",
+ "plugins": {
+ "proxy-rewrite": {
+ "uri": "/demo"
+ },
+ "body-transformer": {
+ "response": {
+ "input_format": "xml",
+ "template": "%s"
+ }
+ }
+ },
+ "upstream": {
+ "type": "roundrobin",
+ "nodes": {
+ "127.0.0.1:%d": 1
+ }
+ }
+ }]], rsp_template, ngx.var.server_port)
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ return
+ end
+ ngx.sleep(0.5)
+
+ local core = require("apisix.core")
+ local http = require("resty.http")
+ local uri = "http://127.0.0.1:" .. ngx.var.server_port .. "/ws"
+ local opt = {method = "GET"}
+ local httpc = http.new()
+ local res = httpc:request_uri(uri, opt)
+ assert(res.status == 200)
+ local data1 = core.json.decode(res.body)
+ local data2 = core.json.decode[[{"KOVKood":""}]]
+ assert(core.json.stably_encode(data1) == core.json.stably_encode(data2))
+ }
+ }
Review Comment:
ok. Please describe the code change next time so that anyone could get a brief.
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] monkeyDluffy6017 commented on a diff in pull request #9669: fix(body-transformer): xml2lua: replace empty table with empty string
Posted by "monkeyDluffy6017 (via GitHub)" <gi...@apache.org>.
monkeyDluffy6017 commented on code in PR #9669:
URL: https://github.com/apache/apisix/pull/9669#discussion_r1233894569
##########
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 `conf[typ].input_format` is set by `set_input_format`, so if there is no `content_type` and `input_format`, the `conf[typ].input_format` will be nil, if the `format` is nil, the `out` will get wrong.
When i remove the parameter `input_format` of test 12 which you just added, i get this error:
I think it's better to handle this case, we need one of the `content_type` and `input_format`
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] kingluo commented on a diff in pull request #9669: fix(body-transformer): xml2lua: replace empty table with empty string
Posted by "kingluo (via GitHub)" <gi...@apache.org>.
kingluo commented on code in PR #9669:
URL: https://github.com/apache/apisix/pull/9669#discussion_r1233801309
##########
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 format is optional! Notice the document for the requirement before you make a suggestion.
Here it's just a warning log to remind the user.
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] kingluo commented on a diff in pull request #9669: fix(body-transformer): xml2lua: replace empty table with empty string
Posted by "kingluo (via GitHub)" <gi...@apache.org>.
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] monkeyDluffy6017 commented on a diff in pull request #9669: fix(body-transformer): xml2lua: replace empty table with empty string
Posted by "monkeyDluffy6017 (via GitHub)" <gi...@apache.org>.
monkeyDluffy6017 commented on code in PR #9669:
URL: https://github.com/apache/apisix/pull/9669#discussion_r1233894569
##########
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 `conf[typ].input_format` is set by `set_input_format`, so if there is no `content_type` and `input_format`, the `conf[typ].input_format` will be nil, if the `format` is nil, the `out` will get wrong.
When i remove the parameter of test 12 which you just added, i get this error:
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] kingluo commented on a diff in pull request #9669: fix(body-transformer): xml2lua: replace empty table with empty string
Posted by "kingluo (via GitHub)" <gi...@apache.org>.
kingluo commented on code in PR #9669:
URL: https://github.com/apache/apisix/pull/9669#discussion_r1233802765
##########
t/plugin/body-transformer.t:
##########
@@ -821,3 +821,75 @@ location /demo {
assert(data.raw_body == '{"result": "hello world"}')
}
}
+
+
+
+=== TEST 12: empty xml value should be rendered as empty string
+--- config
+location /demo {
+ content_by_lua_block {
+ ngx.print([[
+<SOAP-ENV:Envelope xmlns:SOAP-ENV="http://schemas.xmlsoap.org/soap/envelope/" xmlns:xsd="http://www.w3.org/2001/XMLSchema" xmlns:xrd="http://x-road.eu/xsd/xroad.xsd" xmlns:prod="http://rr.x-road.eu/producer" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmlns:id="http://x-road.eu/xsd/identifiers" xmlns:repr="http://x-road.eu/xsd/representation.xsd" xmlns:SOAP-ENC="http://schemas.xmlsoap.org/soap/encoding/">
+ <SOAP-ENV:Body>
+ <prod:RR58isikEpiletResponse>
+ <request><Isikukood>33333333333</Isikukood></request>
+ <response>
+ <Isikukood>33333333333</Isikukood>
+ <KOVKood></KOVKood>
+ </response>
+ </prod:RR58isikEpiletResponse>
+ </SOAP-ENV:Body>
+</SOAP-ENV:Envelope>
+ ]])
+ }
+}
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin")
+
+ local rsp_template = ngx.encode_base64[[
+{ "KOVKood":"{{Envelope.Body.RR58isikEpiletResponse.response.KOVKood}}" }
+ ]]
+
+ local code, body = t.test('/apisix/admin/routes/1',
+ ngx.HTTP_PUT,
+ string.format([[{
+ "uri": "/ws",
+ "plugins": {
+ "proxy-rewrite": {
+ "uri": "/demo"
+ },
+ "body-transformer": {
+ "response": {
+ "input_format": "xml",
+ "template": "%s"
+ }
+ }
+ },
+ "upstream": {
+ "type": "roundrobin",
+ "nodes": {
+ "127.0.0.1:%d": 1
+ }
+ }
+ }]], rsp_template, ngx.var.server_port)
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ return
+ end
+ ngx.sleep(0.5)
+
+ local core = require("apisix.core")
+ local http = require("resty.http")
+ local uri = "http://127.0.0.1:" .. ngx.var.server_port .. "/ws"
+ local opt = {method = "GET"}
+ local httpc = http.new()
+ local res = httpc:request_uri(uri, opt)
+ assert(res.status == 200)
+ local data1 = core.json.decode(res.body)
+ local data2 = core.json.decode[[{"KOVKood":""}]]
+ assert(core.json.stably_encode(data1) == core.json.stably_encode(data2))
+ }
+ }
Review Comment:
What's your opinion? Why make this change? Please describe it.
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] kingluo commented on a diff in pull request #9669: fix(body-transformer): xml2lua: replace empty table with empty string
Posted by "kingluo (via GitHub)" <gi...@apache.org>.
kingluo commented on code in PR #9669:
URL: https://github.com/apache/apisix/pull/9669#discussion_r1233810597
##########
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:
Check this test case, where's 500? https://github.com/apache/apisix/blob/e19a46663f6cbf2d2a91027b2887cfdbebf70dbc/t/plugin/body-transformer.t#L458
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] kingluo commented on a diff in pull request #9669: fix(body-transformer): xml2lua: replace empty table with empty string
Posted by "kingluo (via GitHub)" <gi...@apache.org>.
kingluo commented on code in PR #9669:
URL: https://github.com/apache/apisix/pull/9669#discussion_r1233802765
##########
t/plugin/body-transformer.t:
##########
@@ -821,3 +821,75 @@ location /demo {
assert(data.raw_body == '{"result": "hello world"}')
}
}
+
+
+
+=== TEST 12: empty xml value should be rendered as empty string
+--- config
+location /demo {
+ content_by_lua_block {
+ ngx.print([[
+<SOAP-ENV:Envelope xmlns:SOAP-ENV="http://schemas.xmlsoap.org/soap/envelope/" xmlns:xsd="http://www.w3.org/2001/XMLSchema" xmlns:xrd="http://x-road.eu/xsd/xroad.xsd" xmlns:prod="http://rr.x-road.eu/producer" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmlns:id="http://x-road.eu/xsd/identifiers" xmlns:repr="http://x-road.eu/xsd/representation.xsd" xmlns:SOAP-ENC="http://schemas.xmlsoap.org/soap/encoding/">
+ <SOAP-ENV:Body>
+ <prod:RR58isikEpiletResponse>
+ <request><Isikukood>33333333333</Isikukood></request>
+ <response>
+ <Isikukood>33333333333</Isikukood>
+ <KOVKood></KOVKood>
+ </response>
+ </prod:RR58isikEpiletResponse>
+ </SOAP-ENV:Body>
+</SOAP-ENV:Envelope>
+ ]])
+ }
+}
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin")
+
+ local rsp_template = ngx.encode_base64[[
+{ "KOVKood":"{{Envelope.Body.RR58isikEpiletResponse.response.KOVKood}}" }
+ ]]
+
+ local code, body = t.test('/apisix/admin/routes/1',
+ ngx.HTTP_PUT,
+ string.format([[{
+ "uri": "/ws",
+ "plugins": {
+ "proxy-rewrite": {
+ "uri": "/demo"
+ },
+ "body-transformer": {
+ "response": {
+ "input_format": "xml",
+ "template": "%s"
+ }
+ }
+ },
+ "upstream": {
+ "type": "roundrobin",
+ "nodes": {
+ "127.0.0.1:%d": 1
+ }
+ }
+ }]], rsp_template, ngx.var.server_port)
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ return
+ end
+ ngx.sleep(0.5)
+
+ local core = require("apisix.core")
+ local http = require("resty.http")
+ local uri = "http://127.0.0.1:" .. ngx.var.server_port .. "/ws"
+ local opt = {method = "GET"}
+ local httpc = http.new()
+ local res = httpc:request_uri(uri, opt)
+ assert(res.status == 200)
+ local data1 = core.json.decode(res.body)
+ local data2 = core.json.decode[[{"KOVKood":""}]]
+ assert(core.json.stably_encode(data1) == core.json.stably_encode(data2))
+ }
+ }
Review Comment:
What's your opinion? Why make this change? Please describe it. Don't leave a long code block without any description.
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] monkeyDluffy6017 commented on a diff in pull request #9669: fix(body-transformer): xml2lua: replace empty table with empty string
Posted by "monkeyDluffy6017 (via GitHub)" <gi...@apache.org>.
monkeyDluffy6017 commented on code in PR #9669:
URL: https://github.com/apache/apisix/pull/9669#discussion_r1233894569
##########
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 `conf[typ].input_format` is set by `set_input_format`, so if there is no `content_type` and `input_format`, the `conf[typ].input_format` will be nil, if the `format` is nil, the `out` will get wrong.
When i remove the parameter `input_format` of test 12 which you just added, i get this error:
I think it's better to handle this situation
##########
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 `conf[typ].input_format` is set by `set_input_format`, so if there is no `content_type` and `input_format`, the `conf[typ].input_format` will be nil, if the `format` is nil, the `out` will get wrong.
When i remove the parameter `input_format` of test 12 which you just added, i get this error:
I think it's better to handle this case
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] leslie-tsang commented on a diff in pull request #9669: fix(body-transformer): xml2lua: replace empty table with empty string
Posted by "leslie-tsang (via GitHub)" <gi...@apache.org>.
leslie-tsang commented on code in PR #9669:
URL: https://github.com/apache/apisix/pull/9669#discussion_r1233427173
##########
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
+ ngx.log(ngx.WARN, "no input format to parse ", typ, " body")
Review Comment:
Please use `core.log.error` instead
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] monkeyDluffy6017 merged pull request #9669: fix(body-transformer): xml2lua: replace empty table with empty string
Posted by "monkeyDluffy6017 (via GitHub)" <gi...@apache.org>.
monkeyDluffy6017 merged PR #9669:
URL: https://github.com/apache/apisix/pull/9669
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] monkeyDluffy6017 commented on a diff in pull request #9669: fix(body-transformer): xml2lua: replace empty table with empty string
Posted by "monkeyDluffy6017 (via GitHub)" <gi...@apache.org>.
monkeyDluffy6017 commented on code in PR #9669:
URL: https://github.com/apache/apisix/pull/9669#discussion_r1233776729
##########
t/plugin/body-transformer.t:
##########
@@ -821,3 +821,75 @@ location /demo {
assert(data.raw_body == '{"result": "hello world"}')
}
}
+
+
+
+=== TEST 12: empty xml value should be rendered as empty string
+--- config
+location /demo {
+ content_by_lua_block {
+ ngx.print([[
+<SOAP-ENV:Envelope xmlns:SOAP-ENV="http://schemas.xmlsoap.org/soap/envelope/" xmlns:xsd="http://www.w3.org/2001/XMLSchema" xmlns:xrd="http://x-road.eu/xsd/xroad.xsd" xmlns:prod="http://rr.x-road.eu/producer" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmlns:id="http://x-road.eu/xsd/identifiers" xmlns:repr="http://x-road.eu/xsd/representation.xsd" xmlns:SOAP-ENC="http://schemas.xmlsoap.org/soap/encoding/">
+ <SOAP-ENV:Body>
+ <prod:RR58isikEpiletResponse>
+ <request><Isikukood>33333333333</Isikukood></request>
+ <response>
+ <Isikukood>33333333333</Isikukood>
+ <KOVKood></KOVKood>
+ </response>
+ </prod:RR58isikEpiletResponse>
+ </SOAP-ENV:Body>
+</SOAP-ENV:Envelope>
+ ]])
+ }
+}
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin")
+
+ local rsp_template = ngx.encode_base64[[
+{ "KOVKood":"{{Envelope.Body.RR58isikEpiletResponse.response.KOVKood}}" }
+ ]]
+
+ local code, body = t.test('/apisix/admin/routes/1',
+ ngx.HTTP_PUT,
+ string.format([[{
+ "uri": "/ws",
+ "plugins": {
+ "proxy-rewrite": {
+ "uri": "/demo"
+ },
+ "body-transformer": {
+ "response": {
+ "input_format": "xml",
+ "template": "%s"
+ }
+ }
+ },
+ "upstream": {
+ "type": "roundrobin",
+ "nodes": {
+ "127.0.0.1:%d": 1
+ }
+ }
+ }]], rsp_template, ngx.var.server_port)
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ return
+ end
+ ngx.sleep(0.5)
+
+ local core = require("apisix.core")
+ local http = require("resty.http")
+ local uri = "http://127.0.0.1:" .. ngx.var.server_port .. "/ws"
+ local opt = {method = "GET"}
+ local httpc = http.new()
+ local res = httpc:request_uri(uri, opt)
+ assert(res.status == 200)
+ local data1 = core.json.decode(res.body)
+ local data2 = core.json.decode[[{"KOVKood":""}]]
+ assert(core.json.stably_encode(data1) == core.json.stably_encode(data2))
+ }
+ }
Review Comment:
```suggestion
location /demo {
content_by_lua_block {
ngx.print([[
<SOAP-ENV:Envelope xmlns:SOAP-ENV="http://schemas.xmlsoap.org/soap/envelope/" xmlns:xsd="http://www.w3.org/2001/XMLSchema" xmlns:xrd="http://x-road.eu/xsd/xroad.xsd" xmlns:prod="http://rr.x-road.eu/producer" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmlns:id="http://x-road.eu/xsd/identifiers" xmlns:repr="http://x-road.eu/xsd/representation.xsd" xmlns:SOAP-ENC="http://schemas.xmlsoap.org/soap/encoding/">
<SOAP-ENV:Body>
<prod:RR58isikEpiletResponse>
<request><Isikukood>33333333333</Isikukood></request>
<response>
<Isikukood>33333333333</Isikukood>
<KOVKood></KOVKood>
</response>
</prod:RR58isikEpiletResponse>
</SOAP-ENV:Body>
</SOAP-ENV:Envelope>
]])
}
}
location /t {
content_by_lua_block {
local t = require("lib.test_admin")
local rsp_template = ngx.encode_base64[[
{ "KOVKood":"{{Envelope.Body.RR58isikEpiletResponse.response.KOVKood}}" }
]]
local code, body = t.test('/apisix/admin/routes/1',
ngx.HTTP_PUT,
string.format([[{
"uri": "/ws",
"plugins": {
"proxy-rewrite": {
"uri": "/demo"
},
"body-transformer": {
"response": {
"template": "%s"
}
}
},
"upstream": {
"type": "roundrobin",
"nodes": {
"127.0.0.1:%d": 1
}
}
}]], rsp_template, ngx.var.server_port)
)
if code >= 300 then
ngx.status = code
return
end
ngx.sleep(0.5)
local core = require("apisix.core")
local http = require("resty.http")
local uri = "http://127.0.0.1:" .. ngx.var.server_port .. "/ws"
local opt = {method = "GET"}
local httpc = http.new()
local res = httpc:request_uri(uri, opt)
assert(res.status == 200)
local data1 = core.json.decode(res.body)
local data2 = core.json.decode[[{"KOVKood":""}]]
assert(core.json.stably_encode(data1) == core.json.stably_encode(data2))
}
}
```
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] monkeyDluffy6017 commented on a diff in pull request #9669: fix(body-transformer): xml2lua: replace empty table with empty string
Posted by "monkeyDluffy6017 (via GitHub)" <gi...@apache.org>.
monkeyDluffy6017 commented on code in PR #9669:
URL: https://github.com/apache/apisix/pull/9669#discussion_r1233777969
##########
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:
It should return 400 if the `format` is nil
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] kingluo commented on a diff in pull request #9669: fix(body-transformer): xml2lua: replace empty table with empty string
Posted by "kingluo (via GitHub)" <gi...@apache.org>.
kingluo commented on code in PR #9669:
URL: https://github.com/apache/apisix/pull/9669#discussion_r1233810597
##########
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:
Check https://github.com/apache/apisix/blob/e19a46663f6cbf2d2a91027b2887cfdbebf70dbc/t/plugin/body-transformer.t#L458
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org