HTTP/2 to upstream #771

hongzhidao · 2025-07-07T03:10:26Z

TODO:
buffering
cache

src/http/modules/ngx_http_proxy_module.h

src/http/ngx_http.h

auto/options

auto/modules

src/http/v2/ngx_http_v2_proxy_module.c

hongzhidao · 2025-07-08T03:07:11Z

Hi @arut,
I left some comments on the first commit, please check them.

src/http/modules/ngx_http_proxy_module.c

arut · 2025-07-08T15:01:00Z

We must avoid sending the Connection header for HTTP/2, as well as for HTTP/3. This is a strictly HTTP/1 header and it makes the request malformed. That's why I added a variable $proxy_internal_connection for this header which evaluates depending on a new context field connection_type and it can be 0. This field is set by the protocol handler. This should probably be a part of the first patch.

ac000

Hi @hongzhidao just some minor things I noticed...

    HTTP/2: ngx_http_v2_proxy_module.
    The module allows to use HTTP/2 protocol for proxying.
    HTTP/2 proxying is enabled by specifying "proxy_http_version 2.0".

    ...

Blank line between the subject and body...

...
diff --git a/src/http/v2/ngx_http_v2_proxy_module.c b/src/http/v2/ngx_http_v2_proxy_module.c
new file mode 100644
index 0000000000..a69ffcc406
--- /dev/null
+++ b/src/http/v2/ngx_http_v2_proxy_module.c

...

+ngx_int_t
+ngx_http_v2_proxy_handler(ngx_http_request_t *r)
+{

...

+#if (NGX_HTTP_SSL)
+        u->ssl = plcf->ssl;
+
+        if (u->ssl) {
+            ngx_str_set(&u->schema, "https://");
+
+        } else {
+            ngx_str_set(&u->schema, "https://");
+        }
+#else
+        ngx_str_set(&u->schema, "https://");
+#endif

Maybe I'm not understanding what the checks for 'ssl' here are for but in
the non-ssl cases shouldn't the schema be 'http://' ? (Otherwise all the
checks seem redundant as we set 'https://' regardless...)

...

+static ngx_int_t
+ngx_http_v2_proxy_body_output_filter(void *data, ngx_chain_t *in)
+{

...

+        /*
+         * We have already got the response and were sending some additional

s/were/we're/

+         * control frames.  Even if there is still something unsent, stop
+         * here anyway.
+         */

...

ngx_http_v2_proxy_parse_frame(ngx_http_request_t *r,
+    ngx_http_v2_proxy_ctx_t *ctx, ngx_buf_t *b)
+{

...

+        /* suppress warning */
+        case ngx_http_v2_proxy_st_payload:
+        case ngx_http_v2_proxy_st_padding:
+            break;

You could just use 'default:' here. Of course you won't get warned if you add
more enum values and don't handle them, but maybe you aren't planning on
adding any more or it's not a concern...

+        }

...

+static ngx_int_t
+ngx_http_v2_proxy_parse_header(ngx_http_request_t *r,
+    ngx_http_v2_proxy_ctx_t *ctx, ngx_buf_t *b)
+{

...

+    if (state < sw_fragment) {
+
+        if (b->last - b->pos < (ssize_t) ctx->rest) {

Or even (ptrdiff_t) to be more explicit about _why_ we're casting...

+            last = b->last;

...

+    if (state == sw_padding) {
+
+        if (b->last - b->pos < (ssize_t) ctx->rest) {

... and here ... and likely a few other places...

+            ctx->rest -= b->last - b->pos;

...

hongzhidao · 2025-07-08T22:54:27Z

Hi @ac000,
Thanks for the catching.
I borrowed most of the code from ngx_http_grpc_module to make it work.

About the scheme setting.

+#if (NGX_HTTP_SSL)
+        u->ssl = plcf->ssl;
+
+        if (u->ssl) {
+            ngx_str_set(&u->schema, "https://");
+
+        } else {
+            ngx_str_set(&u->schema, "https://");
+        }
+#else
+        ngx_str_set(&u->schema, "https://");
+#endif

I feel it should be touched again, I'm looking into @arut implementation about the proxy http3.

I'd like to discuss the others based on ngx_http_grpc_module. And we can keep them consistent.

+static ngx_int_t
+ngx_http_v2_proxy_body_output_filter(void *data, ngx_chain_t *in)
+{

...

+        /*
+         * We have already got the response and were sending some additional

s/were/we're/

+         * control frames.  Even if there is still something unsent, stop
+         * here anyway.
+         */

arut · 2025-07-09T14:08:23Z

I feel it should be touched again, I'm looking into @arut implementation about the proxy http3.

ngx_http_proxy_module already has code for this, namely the ngx_http_proxy_eval() function. You just need to call it, see the v3 proxy. Both http and https schemas should be supported in HTTP/2. HTTP/3 only supports https.

src/http/modules/ngx_http_proxy_module.c

hongzhidao · 2025-07-15T11:29:43Z

ngx_http_proxy_module already has code for this, namely the ngx_http_proxy_eval() function. You just need to call it, see the v3 proxy. Both http and https schemas should be supported in HTTP/2. HTTP/3 only supports https.

Hi @arut, take a look plz, did you mean something like this?

diff --git a/src/http/v2/ngx_http_v2_proxy_module.c b/src/http/v2/ngx_http_v2_proxy_module.c
index 6a4a54a..cc3b4c2 100644
--- a/src/http/v2/ngx_http_v2_proxy_module.c
+++ b/src/http/v2/ngx_http_v2_proxy_module.c
@@ -232,18 +232,9 @@ ngx_http_v2_proxy_handler(ngx_http_request_t *r)

     if (plcf->proxy_lengths == NULL) {
         ctx->host = plcf->host;
-
+        u->schema = plcf->vars.schema;
 #if (NGX_HTTP_SSL)
         u->ssl = plcf->ssl;
-
-        if (u->ssl) {
-            ngx_str_set(&u->schema, "https://");
-
-        } else {
-            ngx_str_set(&u->schema, "https://");
-        }
-#else
-        ngx_str_set(&u->schema, "https://");
 #endif

     } else {
@@ -252,6 +243,10 @@ ngx_http_v2_proxy_handler(ngx_http_r
equest_t *r)
         }
     }

+    if (u->ssl) {
+        ngx_str_set(&u->ssl_alpn_protocol, NGX_HTTP_V2_A
LPN_PROTO);
+    }
+
     u->output.tag = (ngx_buf_tag_t) &ngx_http_v2_proxy_m
odule;

     u->conf = &plcf->upstream;
@@ -266,8 +261,6 @@ ngx_http_v2_proxy_handler(ngx_http_re
quest_t *r)
     u->input_filter = ngx_http_v2_proxy_filter;
     u->input_filter_ctx = ctx;

-    ngx_str_set(&u->ssl_alpn_protocol, NGX_HTTP_V2_ALPN_
PROTO);
-
     r->request_body_no_buffering = 1;

     rc = ngx_http_read_client_request_body(r, ngx_http_u
pstream_init);

hongzhidao · 2025-07-15T11:45:41Z

We must avoid sending the Connection header for HTTP/2, as well as for HTTP/3. This is a strictly HTTP/1 header and it makes the request malformed. That's why I added a variable $proxy_internal_connection for this header which evaluates depending on a new context field connection_type and it can be 0. This field is set by the protocol handler. This should probably be a part of the first patch.

Good to learn, I added it into the proxy refactoring patch.

src/http/modules/ngx_http_proxy_module.c

src/http/ngx_http.h

src/http/modules/ngx_http_proxy_module.c

arut · 2025-07-15T14:28:46Z

src/http/modules/ngx_http_proxy_module.c

+#if (NGX_HTTP_V2)
+
+    if (plcf->http_version == NGX_HTTP_VERSION_20) {
+
+        if (u.family != AF_UNIX) {
+
+            if (u.no_port) {
+                plcf->host = u.host;
+
+            } else {
+                plcf->host.len = u.host.len + 1 + u.port_text.len;
+                plcf->host.data = u.host.data;
+            }
+
+        } else {
+            ngx_str_set(&plcf->host, "localhost");
+        }
+    }
+
+#endif
+


This code is almost identical to what we have in ngx_http_proxy_set_vars(). We can use host from ctx->host_header. In fact, it's used by default if using HTTP/1 headers ngx_http_proxy_headers / ngx_http_proxy_cache_headers instead of grpc-specific ngx_http_grpc_headers, which you are already using.

These header sets are not fully compatible though. For example, the following header is empty in HTTP/1:

{ ngx_string("TE"), ngx_string("$grpc_internal_trailers") },

We may think about adding something similar to ng_http_proxy_module, but do it natively, NOT under HTTP/2 ifdef. I think this deserves a separate patch. And I'm not sure if we want client trailers to be sent by default to upstream. Anyway, this looks like a separate feature we can think about later.

src/http/v2/ngx_http_v2_proxy_module.c

src/http/modules/ngx_http_proxy_module.h

src/http/v2/ngx_http_v2_proxy_module.c

arut · 2025-07-16T06:15:10Z

This commit is prepared for HTTP/2 and HTTP/3 support.
    
    This resolves SSL context sharing issues between sibling locations with
    different HTTP protocol versions. Previously, SSL_CTX was shared between
    locations but ALPN protocol was set at SSL context level, causing conflicts
    when different locations used different HTTP versions.
    
    The ALPN protocol is now set per-connection in
    ngx_http_upstream_ssl_init_connection(), allowing proper protocol negotiation
    for each individual upstream connection regardless of SSL context sharing.

I suggest skipping the paragraph about sibling location, those details don't matter here.

arut · 2025-07-16T06:17:56Z

The refactoring patch has no connection_type assignment now for some reason. This assignment should be there for HTTP/1, but not for HTTP/2.

arut · 2025-07-16T06:20:04Z

Also, listen 8443 ssl http2; is a deprecated way of enabling HTTP/2. The preferred way is to use the http2 on directive.

This commit is prepared for HTTP/2 and HTTP/3 support. The ALPN protocol is now set per-connection in ngx_http_upstream_ssl_init_connection(), allowing proper protocol negotiation for each individual upstream connection regardless of SSL context sharing.

src/http/modules/ngx_http_proxy_module.c

arut · 2025-07-21T14:57:16Z

src/http/v2/ngx_http_v2_proxy_module.c

@@ -3877,3 +4386,4 @@ ngx_http_v2_proxy_finalize_request(ngx_http_request_t *r, ngx_int_t rc)
                   "finalize proxy http2 request");
    return;
 }
+


Trailing empty line here.

arut · 2025-07-22T16:22:18Z

@hongzhidao the grpc module has two specific configuration values

    conf->upstream.pass_trailers = 1;                                            
    conf->upstream.preserve_output = 1;

Regarding the first one, there's a directive proxy_pass_trailers which can enable it in case someone needs to proxy grpc over this module.

The second one is needed for unbuffered bidirectional grpc proxying and probably needs to be enabled only in this case.A possible solution is a new proxy directive like proxy_bidirectional. We will need this for HTTP/3 as well. Also, this behavior seems useful even for HTTP/1. I suggest a new patch for this.

arut · 2025-07-22T17:23:39Z

@hongzhidao the grpc module has two specific configuration values
    conf->upstream.pass_trailers = 1;                                            
    conf->upstream.preserve_output = 1;   
Regarding the first one, there's a directive proxy_pass_trailers which can enable it in case someone needs to proxy grpc over this module.

The second one is needed for unbuffered bidirectional grpc proxying and probably needs to be enabled only in this case.A possible solution is a new proxy directive like proxy_bidirectional. We will need this for HTTP/3 as well. Also, this behavior seems useful even for HTTP/1. I suggest a new patch for this.

Upon closer inspection, it seems like at least part of preserve_output functionality is needed for buffered proxying as well. The v2 module may need to send HTTP/2 control frames to upstream and this flag allows to do that after the request body has already been sent. The best solution now is to move this flag from u->conf to u and set it only for HTTP/2.

The module allows to use HTTP/2 protocol for proxying. HTTP/2 proxying is enabled by specifying "proxy_http_version 2.0". Example: server { listen 8000; location / { proxy_http_version 2.0; proxy_pass https://127.0.0.1:8443; } } server { listen 8443 ssl; http2 on; ssl_certificate certs/example.com.crt; ssl_certificate_key certs/example.com.key; location / { return 200 foo; } }

arut · 2025-07-24T15:51:29Z

@hongzhidao, a few points regarding testing.

Please use modified gRPC tests from https://github.com/nginx/nginx-tests to test this module. You need to substitute grpc-specific directives with proxy directives and add proxy_pass_trailers on for some tests. Looks like these tests should fail because of unset preserve_output flag (see HTTP/2 to upstream #771 (comment)).
Also, try running our regular HTTP/1 proxy tests. Add proxy_http_version 2.0 and http2 on to use this module.

arut · 2025-07-24T15:58:02Z

src/http/v2/ngx_http_v2_proxy_module.c

@@ -2155,6 +2169,501 @@ ngx_http_v2_proxy_non_buffered_filter(void *data, ssize_t bytes)
 }


+static ngx_int_t
+ngx_http_v2_proxy_filter(ngx_event_pipe_t *p, ngx_buf_t *buf)


This function resembles its unbuffered counterpart ngx_http_v2_proxy_non_buffered_filter(), which it of course should. However, the amount of code copied is too large. I suggest moving the common code which handles HTTP/2 internals to a separate function and only leave buffer-related code outside. I'm pretty sure this will make the code much more readable.

arut · 2025-07-24T16:00:37Z

src/http/v2/ngx_http_v2_proxy_module.c

+    u->process_header = ngx_http_v2_proxy_process_header;
+    u->abort_request = ngx_http_v2_proxy_abort_request;
+    u->finalize_request = ngx_http_v2_proxy_finalize_request;
+


There's a part that's missing in this module compared to ngx_http_proxy_module:

+ if (plcf->redirects) { + u->rewrite_redirect = ngx_http_proxy_rewrite_redirect; + } + + if (plcf->cookie_domains || plcf->cookie_paths || plcf->cookie_flags) { + u->rewrite_cookie = ngx_http_proxy_rewrite_cookie; + } +

Also, this line is missing as well:

+ u->accel = 1;

hongzhidao marked this pull request as draft July 7, 2025 03:10

hongzhidao changed the title ~~Http2 upstream~~ HTTP/2 to upstream Jul 7, 2025

Maryna-f5 added this to the nginx-1.29.2 milestone Jul 7, 2025

arut reviewed Jul 7, 2025

View reviewed changes

src/http/modules/ngx_http_proxy_module.h Outdated Show resolved Hide resolved

hongzhidao force-pushed the http2-upstream branch 3 times, most recently from 5dedbca to cc47b74 Compare July 8, 2025 02:53

hongzhidao commented Jul 8, 2025

View reviewed changes

src/http/ngx_http.h Outdated Show resolved Hide resolved

hongzhidao commented Jul 8, 2025

View reviewed changes

auto/options Outdated Show resolved Hide resolved

auto/options Outdated Show resolved Hide resolved

auto/modules Outdated Show resolved Hide resolved

auto/modules Outdated Show resolved Hide resolved

src/http/v2/ngx_http_v2_proxy_module.c Show resolved Hide resolved

hongzhidao force-pushed the http2-upstream branch 2 times, most recently from 66338f9 to 63a769c Compare July 8, 2025 07:14

arut reviewed Jul 8, 2025

View reviewed changes

src/http/modules/ngx_http_proxy_module.c Outdated Show resolved Hide resolved

hongzhidao force-pushed the http2-upstream branch 2 times, most recently from 8e79530 to a1c2cd9 Compare July 8, 2025 22:30

ac000 reviewed Jul 8, 2025

View reviewed changes

arut reviewed Jul 9, 2025

View reviewed changes

src/http/modules/ngx_http_proxy_module.c Outdated Show resolved Hide resolved

hongzhidao force-pushed the http2-upstream branch 2 times, most recently from c32eeeb to 731645c Compare July 15, 2025 11:18

hongzhidao force-pushed the http2-upstream branch from 731645c to 6498c2c Compare July 15, 2025 11:44