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/05 01:31:10 UTC
[GitHub] [apisix] Firstsawyou opened a new pull request #2629: fix: after proxy-mirror plugin mirroring request, http version change…
Firstsawyou opened a new pull request #2629:
URL: https://github.com/apache/apisix/pull/2629
…d from 1.1 to 1.0.
fix #2618
### 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?
* [x] 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] Firstsawyou commented on a change in pull request #2629: fix: after proxy-mirror plugin mirroring request, http version change…
Posted by GitBox <gi...@apache.org>.
Firstsawyou commented on a change in pull request #2629:
URL: https://github.com/apache/apisix/pull/2629#discussion_r517735588
##########
File path: t/plugin/proxy-mirror.t
##########
@@ -32,10 +32,11 @@ add_block_preprocessor(sub {
server_tokens off;
location / {
- content_by_lua_block {
+ content_by_lua_block {
local core = require("apisix.core")
- local headers_tab = ngx.req.get_headers()
+ core.log.info("upstream_http_version: ", ngx.req.http_version())
+ local headers_tab = ngx.req.get_headers()
for k, v in pairs(headers_tab) do
Review comment:
Thank you very much for your suggestions, I will update later.
----------------------------------------------------------------
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 #2629: fix: after proxy-mirror plugin mirroring request, http version change…
Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #2629:
URL: https://github.com/apache/apisix/pull/2629#discussion_r517732526
##########
File path: t/plugin/proxy-mirror.t
##########
@@ -32,10 +32,11 @@ add_block_preprocessor(sub {
server_tokens off;
location / {
- content_by_lua_block {
+ content_by_lua_block {
local core = require("apisix.core")
- local headers_tab = ngx.req.get_headers()
+ core.log.info("upstream_http_version: ", ngx.req.http_version())
+ local headers_tab = ngx.req.get_headers()
for k, v in pairs(headers_tab) do
Review comment:
I run the tests locally. It seems the order of `headers_tab` is unstable?
----------------------------------------------------------------
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] membphis commented on pull request #2629: fix: after proxy-mirror plugin mirroring request, http version change…
Posted by GitBox <gi...@apache.org>.
membphis commented on pull request #2629:
URL: https://github.com/apache/apisix/pull/2629#issuecomment-722168969
@spacewander do you have time to take a look at this PR?
----------------------------------------------------------------
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 #2629: fix: after proxy-mirror plugin mirroring request, http version change…
Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #2629:
URL: https://github.com/apache/apisix/pull/2629#discussion_r517732526
##########
File path: t/plugin/proxy-mirror.t
##########
@@ -32,10 +32,11 @@ add_block_preprocessor(sub {
server_tokens off;
location / {
- content_by_lua_block {
+ content_by_lua_block {
local core = require("apisix.core")
- local headers_tab = ngx.req.get_headers()
+ core.log.info("upstream_http_version: ", ngx.req.http_version())
+ local headers_tab = ngx.req.get_headers()
for k, v in pairs(headers_tab) do
Review comment:
I run the tests locally. It seems the order of `headers_tab` is unstable? We need to sort it to make the test stable.
----------------------------------------------------------------
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 #2629: fix: after proxy-mirror plugin mirroring request, http version change…
Posted by GitBox <gi...@apache.org>.
spacewander merged pull request #2629:
URL: https://github.com/apache/apisix/pull/2629
----------------------------------------------------------------
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 #2629: fix: after proxy-mirror plugin mirroring request, http version change…
Posted by GitBox <gi...@apache.org>.
Firstsawyou commented on a change in pull request #2629:
URL: https://github.com/apache/apisix/pull/2629#discussion_r517770767
##########
File path: t/plugin/proxy-mirror.t
##########
@@ -32,10 +32,11 @@ add_block_preprocessor(sub {
server_tokens off;
location / {
- content_by_lua_block {
+ content_by_lua_block {
local core = require("apisix.core")
- local headers_tab = ngx.req.get_headers()
+ core.log.info("upstream_http_version: ", ngx.req.http_version())
+ local headers_tab = ngx.req.get_headers()
for k, v in pairs(headers_tab) do
Review comment:
hi, @spacewander , I tried it locally. The `headers_tab` printed to the log does not need to be sorted. We need to sort the `_M.uri()` function in `t/lib/server.lua`. I think we can solve this by creating a new pr problem. What do you think?
----------------------------------------------------------------
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 pull request #2629: fix: after proxy-mirror plugin mirroring request, http version change…
Posted by GitBox <gi...@apache.org>.
spacewander commented on pull request #2629:
URL: https://github.com/apache/apisix/pull/2629#issuecomment-722070507
Now we force the proxy HTTP request to be version 1.1.
But it is fine... Since there is no way to dynamically change proxy HTTP version without modifying Nginx core and the majority HTTP request is 1.1.
----------------------------------------------------------------
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 pull request #2629: fix: after proxy-mirror plugin mirroring request, http version change…
Posted by GitBox <gi...@apache.org>.
Firstsawyou commented on pull request #2629:
URL: https://github.com/apache/apisix/pull/2629#issuecomment-722072962
> Now we force the proxy HTTP request to be version 1.1.
> But it is fine... Since there is no way to dynamically change proxy HTTP version without modifying Nginx core and the majority HTTP request is 1.1.
Yes, I tried to dynamically change the proxy HTTP version, but without success.
----------------------------------------------------------------
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 pull request #2629: fix: after proxy-mirror plugin mirroring request, http version change…
Posted by GitBox <gi...@apache.org>.
Firstsawyou commented on pull request #2629:
URL: https://github.com/apache/apisix/pull/2629#issuecomment-722170393
@spacewander ping
----------------------------------------------------------------
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