You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@apisix.apache.org by GitBox <gi...@apache.org> on 2022/04/22 05:41:59 UTC
[GitHub] [apisix] soulbird opened a new pull request, #6908: fix: upstream nodes cant't include ipv6 addr
soulbird opened a new pull request, #6908:
URL: https://github.com/apache/apisix/pull/6908
### Description
Make a simple modification to make the upstream nodes host support ipv6
Fixes #6905
### Checklist
- [ ] I have explained the need for this PR and the problem it solves
- [ ] I have explained the changes or the new features added to this PR
- [ ] I have added tests corresponding to this change
- [ ] I have updated the documentation to reflect this change
- [ ] 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)
--
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] soulbird commented on a diff in pull request #6908: fix: upstream nodes cant't include ipv6 addr
Posted by GitBox <gi...@apache.org>.
soulbird commented on code in PR #6908:
URL: https://github.com/apache/apisix/pull/6908#discussion_r857051484
##########
apisix/schema_def.lua:
##########
@@ -37,7 +37,7 @@ local id_schema = {
}
}
-local host_def_pat = "^\\*?[0-9a-zA-Z-._]+$"
+local host_def_pat = "^\\*?[0-9a-zA-Z-._\\[\\]:]+$"
Review Comment:
> I would prefer to a simple regex check as a complex check is error-prone (may reject valid input)
So I'm leaning towards a more conservative way of fixing it, like this currently. @zhendongcmss
##########
t/plugin/referer-restriction.t:
##########
@@ -173,7 +173,6 @@ hello world
local cases = {
"x.*",
"~y.xn",
- "::1",
Review Comment:
In fact, referer is a URL, and the current shcema check is not suitable. Maybe we need another PR to fix this
--
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] spacewander merged pull request #6908: fix: upstream nodes cant't include ipv6 addr
Posted by GitBox <gi...@apache.org>.
spacewander merged PR #6908:
URL: https://github.com/apache/apisix/pull/6908
--
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] zhendongcmss commented on a diff in pull request #6908: fix: upstream nodes cant't include ipv6 addr
Posted by GitBox <gi...@apache.org>.
zhendongcmss commented on code in PR #6908:
URL: https://github.com/apache/apisix/pull/6908#discussion_r856935951
##########
apisix/schema_def.lua:
##########
@@ -37,7 +37,7 @@ local id_schema = {
}
}
-local host_def_pat = "^\\*?[0-9a-zA-Z-._]+$"
+local host_def_pat = "^\\*?[0-9a-zA-Z-._\\[\\]:]+$"
Review Comment:
My point that it is hard to use the same regular exprssion to valitate ipv4 and ipv6. Can we add a function to do it to judge the ip class then using different regular expression.
--
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] zhendongcmss commented on a diff in pull request #6908: fix: upstream nodes cant't include ipv6 addr
Posted by GitBox <gi...@apache.org>.
zhendongcmss commented on code in PR #6908:
URL: https://github.com/apache/apisix/pull/6908#discussion_r856935951
##########
apisix/schema_def.lua:
##########
@@ -37,7 +37,7 @@ local id_schema = {
}
}
-local host_def_pat = "^\\*?[0-9a-zA-Z-._]+$"
+local host_def_pat = "^\\*?[0-9a-zA-Z-._\\[\\]:]+$"
Review Comment:
My point that it is hard to use the same regular exprssion to valitate ipv4 and ipv6.
--
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] membphis commented on a diff in pull request #6908: fix: upstream nodes cant't include ipv6 addr
Posted by GitBox <gi...@apache.org>.
membphis commented on code in PR #6908:
URL: https://github.com/apache/apisix/pull/6908#discussion_r855870455
##########
apisix/schema_def.lua:
##########
@@ -37,7 +37,7 @@ local id_schema = {
}
}
-local host_def_pat = "^\\*?[0-9a-zA-Z-._]+$"
+local host_def_pat = "^\\*?[0-9a-zA-Z-._\\[\\]:]+$"
Review Comment:
IPv6 schema: https://github.com/apache/apisix/blob/master/apisix/schema_def.lua#L56
--
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] membphis commented on a diff in pull request #6908: fix: upstream nodes cant't include ipv6 addr
Posted by GitBox <gi...@apache.org>.
membphis commented on code in PR #6908:
URL: https://github.com/apache/apisix/pull/6908#discussion_r855870455
##########
apisix/schema_def.lua:
##########
@@ -37,7 +37,7 @@ local id_schema = {
}
}
-local host_def_pat = "^\\*?[0-9a-zA-Z-._]+$"
+local host_def_pat = "^\\*?[0-9a-zA-Z-._\\[\\]:]+$"
Review Comment:
IPv6 schema: https://github.com/apache/apisix/blob/master/apisix/schema_def.lua#L56
can we reuse 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] spacewander commented on a diff in pull request #6908: fix: upstream nodes cant't include ipv6 addr
Posted by GitBox <gi...@apache.org>.
spacewander commented on code in PR #6908:
URL: https://github.com/apache/apisix/pull/6908#discussion_r856146635
##########
t/plugin/referer-restriction.t:
##########
@@ -173,7 +173,6 @@ hello world
local cases = {
"x.*",
"~y.xn",
- "::1",
Review Comment:
The `::1` is an invalid referer.
--
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] soulbird commented on a diff in pull request #6908: fix: upstream nodes cant't include ipv6 addr
Posted by GitBox <gi...@apache.org>.
soulbird commented on code in PR #6908:
URL: https://github.com/apache/apisix/pull/6908#discussion_r857296810
##########
t/admin/upstream-array-nodes.t:
##########
@@ -450,3 +450,38 @@ GET /t
{"error_msg":"invalid configuration: property \"nodes\" validation failed: object matches none of the required"}
--- no_error_log
[error]
+
+
+
+=== TEST 14: nodes host include ipv6 addr
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/routes/1',
+ ngx.HTTP_PUT,
+ [[{
+ "upstream": {
+ "nodes": [
+ {
+ "host":"[::1]",
Review Comment:
It will not affect object's node, see here: https://github.com/apache/apisix/blob/master/apisix/schema_def.lua#L308
--
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] soulbird commented on a diff in pull request #6908: fix: upstream nodes cant't include ipv6 addr
Posted by GitBox <gi...@apache.org>.
soulbird commented on code in PR #6908:
URL: https://github.com/apache/apisix/pull/6908#discussion_r855930590
##########
apisix/schema_def.lua:
##########
@@ -37,7 +37,7 @@ local id_schema = {
}
}
-local host_def_pat = "^\\*?[0-9a-zA-Z-._]+$"
+local host_def_pat = "^\\*?[0-9a-zA-Z-._\\[\\]:]+$"
Review Comment:
> IPv6 schema: https://github.com/apache/apisix/blob/master/apisix/schema_def.lua#L56
>
> can we reuse it?
Let my try
--
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] spacewander commented on a diff in pull request #6908: fix: upstream nodes cant't include ipv6 addr
Posted by GitBox <gi...@apache.org>.
spacewander commented on code in PR #6908:
URL: https://github.com/apache/apisix/pull/6908#discussion_r855824886
##########
t/APISIX.pm:
##########
@@ -200,7 +200,7 @@ if ($version =~ m/\/apisix-nginx-module/) {
$a6_ngx_directives = <<_EOC_;
apisix_delay_client_max_body_check on;
apisix_mirror_on_demand on;
- wasm_vm wasmtime;
+ #wasm_vm wasmtime;
Review Comment:
Why remove that?
##########
apisix/schema_def.lua:
##########
@@ -37,7 +37,7 @@ local id_schema = {
}
}
-local host_def_pat = "^\\*?[0-9a-zA-Z-._]+$"
+local host_def_pat = "^\\*?[0-9a-zA-Z-._\\[\\]:]+$"
Review Comment:
I would prefer to a simple regex check as a complex check is error-prone (may reject valid input)
--
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] zhendongcmss commented on a diff in pull request #6908: fix: upstream nodes cant't include ipv6 addr
Posted by GitBox <gi...@apache.org>.
zhendongcmss commented on code in PR #6908:
URL: https://github.com/apache/apisix/pull/6908#discussion_r855819705
##########
apisix/schema_def.lua:
##########
@@ -37,7 +37,7 @@ local id_schema = {
}
}
-local host_def_pat = "^\\*?[0-9a-zA-Z-._]+$"
+local host_def_pat = "^\\*?[0-9a-zA-Z-._\\[\\]:]+$"
Review Comment:
Can validate ipv4/ipv6 more detail ?
--
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] bzp2010 commented on a diff in pull request #6908: fix: upstream nodes cant't include ipv6 addr
Posted by GitBox <gi...@apache.org>.
bzp2010 commented on code in PR #6908:
URL: https://github.com/apache/apisix/pull/6908#discussion_r857216426
##########
t/admin/upstream-array-nodes.t:
##########
@@ -450,3 +450,38 @@ GET /t
{"error_msg":"invalid configuration: property \"nodes\" validation failed: object matches none of the required"}
--- no_error_log
[error]
+
+
+
+=== TEST 14: nodes host include ipv6 addr
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/routes/1',
+ ngx.HTTP_PUT,
+ [[{
+ "upstream": {
+ "nodes": [
+ {
+ "host":"[::1]",
Review Comment:
Is it valid only when using object's node? What happens if I use an array pattern like `[::1]:8080`?
--
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