Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HTTP/2 with location.capture() re-enable #2243

Closed
oowl opened this issue Oct 19, 2023 · 8 comments
Closed

HTTP/2 with location.capture() re-enable #2243

oowl opened this issue Oct 19, 2023 · 8 comments

Comments

@oowl
Copy link
Contributor

oowl commented Oct 19, 2023

I have tried to reproduce #1195 (comment) mentioned bug by revert 3078ca6 commit

user root;
worker_rlimit_core 500M;

events {
    worker_connections 1024;
}

http {
    log_format compression '$remote_addr - $remote_user [$time_local] '
                        '"$request" $status $bytes_sent '
                        '"$http_referer" "$http_user_agent" "$gzip_ratio"';
    access_log logs/access.log compression;
            proxy_cache_path /tmp/nginx/cache keys_zone=api_key:10m;
    server {
        listen 8080 http2;
        location /main2 {
            content_by_lua '
                local res2 = ngx.location.capture("/internal/api",  {
                    method = ngx.HTTP_POST,
                    body = body,
                    args = {hello = "world"}
                })
                ngx.say(res2.body)
            ';
        }
        location /foo {
            echo -n world;
        }
        location /bar {
            echo -n ' people';
        }
        location = /internal/api {
            internal;
            proxy_method GET;
            proxy_cache_methods GET HEAD POST;
            proxy_cache_valid any 5m;
            proxy_cache api_key;
            proxy_cache_key api_$args;
            proxy_pass https://httpbin.org/anything;
        }
    }
}
curl --http2-prior-knowledge 127.0.0.1:8080/main2/api\?a=1  -H "accept: application/json" -X POST -vv
*   Trying 127.0.0.1:8080...
* Connected to 127.0.0.1 (127.0.0.1) port 8080
* [HTTP/2] [1] OPENED stream for http://127.0.0.1:8080/main2/api?a=1
* [HTTP/2] [1] [:method: POST]
* [HTTP/2] [1] [:scheme: http]
* [HTTP/2] [1] [:authority: 127.0.0.1:8080]
* [HTTP/2] [1] [:path: /main2/api?a=1]
* [HTTP/2] [1] [user-agent: curl/8.4.0]
* [HTTP/2] [1] [accept: application/json]
> POST /main2/api?a=1 HTTP/2
> Host: 127.0.0.1:8080
> User-Agent: curl/8.4.0
> accept: application/json
> 
< HTTP/2 200 
< server: openresty/1.21.4.2
< date: Thu, 19 Oct 2023 15:32:30 GMT
< content-type: text/plain
< 
{
  "args": {
    "hello": "world"
  }, 
  "data": "", 
  "files": {}, 
  "form": {}, 
  "headers": {
    "Accept": "application/json", 
    "Host": "httpbin.org", 
    "User-Agent": "curl/8.4.0", 
    "X-Amzn-Trace-Id": "Root=1-65314b87-0647d43721dc60d564c46b16"
  }, 
  "json": null, 
  "method": "GET", 
  "url": "https://httpbin.org/anything?hello=world"
}


curl --http2-prior-knowledge 127.0.0.1:8080/main2/api\?a=1  -H "accept: application/json" -X POST -vv
*   Trying 127.0.0.1:8080...
* Connected to 127.0.0.1 (127.0.0.1) port 8080
* [HTTP/2] [1] OPENED stream for http://127.0.0.1:8080/main2/api?a=1
* [HTTP/2] [1] [:method: POST]
* [HTTP/2] [1] [:scheme: http]
* [HTTP/2] [1] [:authority: 127.0.0.1:8080]
* [HTTP/2] [1] [:path: /main2/api?a=1]
* [HTTP/2] [1] [user-agent: curl/8.4.0]
* [HTTP/2] [1] [accept: application/json]
> POST /main2/api?a=1 HTTP/2
> Host: 127.0.0.1:8080
> User-Agent: curl/8.4.0
> accept: application/json
> 
< HTTP/2 200 
< server: openresty/1.21.4.2
< date: Thu, 19 Oct 2023 15:36:08 GMT
< content-type: text/plain
< 
{
  "args": {
    "hello": "world"
  }, 
  "data": "", 
  "files": {}, 
  "form": {}, 
  "headers": {
    "Accept": "application/json", 
    "Host": "httpbin.org", 
    "User-Agent": "curl/8.4.0", 
    "X-Amzn-Trace-Id": "Root=1-65314ce8-504043c862ceaf6a48dbe6ce"
  }, 
  "json": null, 
  "method": "GET", 
  "url": "https://httpbin.org/anything?hello=world"
}

I can not reproduce this problem, Just wanted to know if it's possible to revert the 3078ca6 commit to enable HTTP2 location.capture() support.

@oowl
Copy link
Contributor Author

oowl commented Oct 19, 2023

If anyone can help me to reproduce this problem, I would be very willing to dig it up and fix it.

@oowl oowl changed the title HTTP/2 with location.capture() renable HTTP/2 with location.capture() re-enable Oct 19, 2023
@oowl
Copy link
Contributor Author

oowl commented Jul 9, 2024

Hi team, for a long time, there has been no progress on this issue. However, some of our code relies on this API for cache-related work. This means that for a long time, our caching functionality hasn't worked over HTTP2 because of this hard-coded limitation. Our service does not involve any internal proxy_cache features, so it shouldn't be affected. Therefore, I believe we should remove this hard-coded limit because the impact of not being able to use it at all is greater than the potential bugs that might arise.
cc @zhuizhuhaomeng @agentzh

@oowl
Copy link
Contributor Author

oowl commented Jul 9, 2024

also cc @wangchunpeng

@Navinnair
Copy link

Navinnair commented Aug 12, 2024

Hi @oowl

Thank you for your ongoing work on this module. I wanted to ask if there are any plans to address the HTTP/2 issue that has been discussed. Specifically, is there any intention to revert the changes mentioned, or will there be a different fix implemented?

I understand that maintaining such a project requires a lot of time and effort, and I truly appreciate all that you do. Any updates or insights you could share would be highly appreciated.

Thanks in advance!

@oowl
Copy link
Contributor Author

oowl commented Aug 21, 2024

Hi @oowl

Thank you for your ongoing work on this module. I wanted to ask if there are any plans to address the HTTP/2 issue that has been discussed. Specifically, is there any intention to revert the changes mentioned, or will there be a different fix implemented?

I understand that maintaining such a project requires a lot of time and effort, and I truly appreciate all that you do. Any updates or insights you could share would be highly appreciated.

Thanks in advance!

Sorry, I am not a maintainer of Openresty, so I can not answer your problem.

@splitice
Copy link

splitice commented Oct 15, 2024

For 2 different applications we have had the restriction code removed for multiple years without issue. AFAIK there isnt any major issues with http2 and request capture.

@bjoe2k4
Copy link
Contributor

bjoe2k4 commented Oct 15, 2024

I can second this, removed the restriction since it was introduced, never had a problem since.

@oowl
Copy link
Contributor Author

oowl commented Nov 6, 2024

Hey guys, http2 location.capture API is re-enable in master now. So close this issue now. @bjoe2k4 @splitice @Navinnair

@oowl oowl closed this as completed Nov 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants