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 2020/11/10 13:43:42 UTC
[GitHub] [apisix] Firstsawyou opened a new pull request #2696: [WIP]: sort the request headers output by the _M.uri and _M.old_uri function in t/lib/server.lua
Firstsawyou opened a new pull request #2696:
URL: https://github.com/apache/apisix/pull/2696
fix #2636
### What this PR does / why we need it:
<!--- Why is this change required? What problem does it solve? -->
<!--- If it fixes an open issue, please link to the issue here. -->
### Pre-submission checklist:
* [x] Did you explain what problem does this PR solve? Or what new features have been added?
* [ ] Have you added corresponding test cases?
* [ ] Have you modified the corresponding document?
* [x] Is this PR backward compatible?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] tokers commented on a change in pull request #2696: chore: sort the request headers output by the _M.uri and _M.old_uri function in t/lib/server.lua
Posted by GitBox <gi...@apache.org>.
tokers commented on a change in pull request #2696:
URL: https://github.com/apache/apisix/pull/2696#discussion_r521023803
##########
File path: t/lib/server.lua
##########
@@ -101,8 +101,15 @@ function _M.uri()
-- ngx.sleep(1)
ngx.say("uri: ", ngx.var.uri)
local headers = ngx.req.get_headers()
- for k, v in pairs(headers) do
- ngx.say(k, ": ", v)
+
+ local keys = {}
+ for k, _ in pairs(headers) do
Review comment:
Just ignore the second dumb variable if you don't need it.
```lua
for k in pairs(headers) do
......
end
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] nic-chen commented on a change in pull request #2696: chore: sort the request headers output by the _M.uri and _M.old_uri function in t/lib/server.lua
Posted by GitBox <gi...@apache.org>.
nic-chen commented on a change in pull request #2696:
URL: https://github.com/apache/apisix/pull/2696#discussion_r521729518
##########
File path: t/plugin/hmac-auth.t
##########
@@ -1246,16 +1246,16 @@ location /t {
GET /t
--- response_body
uri: /uri
-host: 127.0.0.1
+content-length: 52
content-type: application/x-www-form-urlencoded
-x-real-ip: 127.0.0.1
+host: 127.0.0.1
user-agent: lua-resty-http/0.14 (Lua) ngx_lua/10017
Review comment:
a little confuse, why `user-agent` is in this place ?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander commented on a change in pull request #2696: chore: sort the request headers output by the _M.uri and _M.old_uri function in t/lib/server.lua
Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #2696:
URL: https://github.com/apache/apisix/pull/2696#discussion_r521774023
##########
File path: t/plugin/hmac-auth.t
##########
@@ -1246,16 +1246,16 @@ location /t {
GET /t
--- response_body
uri: /uri
-host: 127.0.0.1
+content-length: 52
content-type: application/x-www-form-urlencoded
-x-real-ip: 127.0.0.1
+host: 127.0.0.1
user-agent: lua-resty-http/0.14 (Lua) ngx_lua/10017
Review comment:
@Firstsawyou
Can we omit the `user-agent`? Since this field will change across different versions.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] Firstsawyou commented on a change in pull request #2696: chore: sort the request headers output by the _M.uri and _M.old_uri function in t/lib/server.lua
Posted by GitBox <gi...@apache.org>.
Firstsawyou commented on a change in pull request #2696:
URL: https://github.com/apache/apisix/pull/2696#discussion_r521041163
##########
File path: t/lib/server.lua
##########
@@ -101,8 +101,15 @@ function _M.uri()
-- ngx.sleep(1)
ngx.say("uri: ", ngx.var.uri)
local headers = ngx.req.get_headers()
- for k, v in pairs(headers) do
- ngx.say(k, ": ", v)
+
+ local keys = {}
+ for k, _ in pairs(headers) do
Review comment:
fixed.
##########
File path: t/lib/server.lua
##########
@@ -101,8 +101,15 @@ function _M.uri()
-- ngx.sleep(1)
ngx.say("uri: ", ngx.var.uri)
local headers = ngx.req.get_headers()
- for k, v in pairs(headers) do
- ngx.say(k, ": ", v)
+
+ local keys = {}
+ for k, _ in pairs(headers) do
+ table.insert(keys, k)
+ end
+ table.sort(keys)
+
+ for _, key in pairs(keys) do
Review comment:
fixed.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander merged pull request #2696: chore: sort the request headers output by the _M.uri and _M.old_uri function in t/lib/server.lua
Posted by GitBox <gi...@apache.org>.
spacewander merged pull request #2696:
URL: https://github.com/apache/apisix/pull/2696
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] Firstsawyou commented on a change in pull request #2696: chore: sort the request headers output by the _M.uri and _M.old_uri function in t/lib/server.lua
Posted by GitBox <gi...@apache.org>.
Firstsawyou commented on a change in pull request #2696:
URL: https://github.com/apache/apisix/pull/2696#discussion_r521759543
##########
File path: t/plugin/hmac-auth.t
##########
@@ -1246,16 +1246,16 @@ location /t {
GET /t
--- response_body
uri: /uri
-host: 127.0.0.1
+content-length: 52
content-type: application/x-www-form-urlencoded
-x-real-ip: 127.0.0.1
+host: 127.0.0.1
user-agent: lua-resty-http/0.14 (Lua) ngx_lua/10017
Review comment:
When requesting the URL of `/uri`, all request headers will be output. So it is normal to have the request header `user-agent`.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] nic-chen commented on a change in pull request #2696: chore: sort the request headers output by the _M.uri and _M.old_uri function in t/lib/server.lua
Posted by GitBox <gi...@apache.org>.
nic-chen commented on a change in pull request #2696:
URL: https://github.com/apache/apisix/pull/2696#discussion_r521816482
##########
File path: t/plugin/hmac-auth.t
##########
@@ -1246,16 +1246,16 @@ location /t {
GET /t
--- response_body
uri: /uri
-host: 127.0.0.1
+content-length: 52
content-type: application/x-www-form-urlencoded
-x-real-ip: 127.0.0.1
+host: 127.0.0.1
user-agent: lua-resty-http/0.14 (Lua) ngx_lua/10017
Review comment:
> When requesting the URL of `/uri`, all request headers will be output. So it is normal to have the request header `user-agent`.
I mean it is not sorted in order.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] tokers commented on a change in pull request #2696: chore: sort the request headers output by the _M.uri and _M.old_uri function in t/lib/server.lua
Posted by GitBox <gi...@apache.org>.
tokers commented on a change in pull request #2696:
URL: https://github.com/apache/apisix/pull/2696#discussion_r521024212
##########
File path: t/lib/server.lua
##########
@@ -101,8 +101,15 @@ function _M.uri()
-- ngx.sleep(1)
ngx.say("uri: ", ngx.var.uri)
local headers = ngx.req.get_headers()
- for k, v in pairs(headers) do
- ngx.say(k, ": ", v)
+
+ local keys = {}
+ for k, _ in pairs(headers) do
+ table.insert(keys, k)
+ end
+ table.sort(keys)
+
+ for _, key in pairs(keys) do
Review comment:
Use `ipairs` here since `keys` is an array-like table.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] Firstsawyou commented on a change in pull request #2696: chore: sort the request headers output by the _M.uri and _M.old_uri function in t/lib/server.lua
Posted by GitBox <gi...@apache.org>.
Firstsawyou commented on a change in pull request #2696:
URL: https://github.com/apache/apisix/pull/2696#discussion_r521035559
##########
File path: t/lib/server.lua
##########
@@ -101,8 +101,15 @@ function _M.uri()
-- ngx.sleep(1)
ngx.say("uri: ", ngx.var.uri)
local headers = ngx.req.get_headers()
- for k, v in pairs(headers) do
- ngx.say(k, ": ", v)
+
+ local keys = {}
+ for k, _ in pairs(headers) do
Review comment:
nice.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] Firstsawyou commented on a change in pull request #2696: chore: sort the request headers output by the _M.uri and _M.old_uri function in t/lib/server.lua
Posted by GitBox <gi...@apache.org>.
Firstsawyou commented on a change in pull request #2696:
URL: https://github.com/apache/apisix/pull/2696#discussion_r521828845
##########
File path: t/plugin/hmac-auth.t
##########
@@ -1246,16 +1246,16 @@ location /t {
GET /t
--- response_body
uri: /uri
-host: 127.0.0.1
+content-length: 52
content-type: application/x-www-form-urlencoded
-x-real-ip: 127.0.0.1
+host: 127.0.0.1
user-agent: lua-resty-http/0.14 (Lua) ngx_lua/10017
Review comment:
> > When requesting the URL of `/uri`, all request headers will be output. So it is normal to have the request header `user-agent`.
>
> I mean it is not sorted in order.
It has been sorted, `user-agent` is after `host`. ^ _ ^
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org